Thread (15 messages) 15 messages, 3 authors, 2016-03-10

Re: [PATCH v7] scsi: ufs: add ioctl interface for query request

From: Greg KH <gregkh@linuxfoundation.org>
Date: 2016-03-10 18:58:59
Also in: linux-arm-msm, linux-scsi, lkml

On Thu, Mar 10, 2016 at 06:48:54PM -0000, ygardi@codeaurora.org wrote:
quoted
On Thu, Mar 10, 2016 at 04:29:55PM -0000, ygardi@codeaurora.org wrote:
quoted
quoted
On Thu, Mar 10, 2016 at 03:52:54PM -0000, ygardi@codeaurora.org wrote:
quoted
quoted
On Wed, Mar 09, 2016 at 08:52:59PM -0000, ygardi@codeaurora.org
wrote:
quoted
quoted
quoted
quoted
quoted
On Wed, Mar 09, 2016 at 07:09:49PM -0000, ygardi@codeaurora.org
wrote:
quoted
quoted
quoted
quoted
quoted
On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote:
quoted
This patch exposes the ioctl interface for UFS driver via
SCSI
quoted
quoted
quoted
quoted
device
quoted
quoted
quoted
quoted
ioctl interface. As of now UFS driver would provide the
ioctl
quoted
quoted
for
quoted
quoted
quoted
quoted
query
quoted
quoted
interface to connected UFS device.

Reviewed-by: Subhash Jadavani <redacted>
Signed-off-by: Dolev Raviv <redacted>
Signed-off-by: Gilad Broner <redacted>
Signed-off-by: Yaniv Gardi <redacted>
What tool is going to use this ioctl?  Why does userspcae
want
quoted
quoted
to
quoted
quoted
do
quoted
quoted
quoted
something "special" with UFS devices?  Shouldn't they just be
treated
quoted
quoted
quoted
like any other normal block device?
Any userspace application can be a tool.
We already implemented and used a user space application, that
sent
quoted
quoted
quoted
quoted
queries to the UFS devices in order to get information and
descriptors.
quoted
But do you want to do with that information?  Why does userspace
care?
quoted
quoted
quoted
i don't really understand the subtext of your question -
as ANY ioctl cb, we decided to implement the ioctl callback of
this
quoted
quoted
scsi
quoted
quoted
device in order to get information like UNIT DESC, DEVICE DESC,
FLAGs,
quoted
quoted
ATTRIBUTES.
When dealing with UFS devices, one should be able to read the
characteristics of the device. why ? well, why not ?
Why aren't those characteristics just exported as sysfs attributes
under
quoted
control by the UFS controller driver?  Why do you need/want an
ioctl
quoted
quoted
for
quoted
this?
Hi greg k-h,

in our code, we used the IOCTL during runtime, in order to determine
some
information about the RPMB well known lun.
with the rpmb lun ID we could then go to /dev/sgX and issue
UFS_IOCTL_QUERY to this lun and get the data -
reading the QUERY_DESC_IDN_GEOMETRY descriptor and reading the
QUERY_DESC_IDN_UNIT descriptor.

this was crucial to the work we do in RPMB.
What is RPMB?

And again, why not just use sysfs attributes on your host controller
device?  Why does this have to be a custom ioctl?

greg k-h
RPMB is spcial Logical Unit in the UFS device. you can read about it in
the UFS spec. Greg, you are insisting on sysfs, but i can't implement it
now, as i don't have the Hardware anymore, or the time.
If you don't have the hardware, how are you testing this patch?
This patch is already tested and verified, and also was much helpful
during development.
Also, this patch is a few months old and was tested when previous version
were uploaded.
Currently HW is not available.
quoted
And if you don't have the hardware I guess you don't need this change :)
quoted
This is a tested and verified code that was accepted and reviewed
already,
so i'm not sure what is wrong with this solution, not to say, it's
already
implemented, tested and verified.
hope you are help us push it.
That's a horrible reason to merge a patch that someone else is going to
have to support for 20+ years with an api that doesn't make much sense.

If you don't have the hardware, then this isn't needed.  But if you do,
Even if currently HW is not available it doesn't mean this is not needed
in the future again.
Great, if in the future, you need this again, please resubmit it.
Adding code to the kernel for no real user is a maintaince burden on
others.  Please don't do that.  Especially for an API that is now
required to be supported for forever.
quoted
then please look into using sysfs for this, as I think that should be
the correct interface here, again, not some random ioctl.
Why sysfs makes more sense than this one ?
Why doesn't it?  Why would an ioctl make sense to get simple attributes
from a host controller?  Why not use the interface that makes it trivial
to use from userspace that other drivers already use in this type of
situation.

In other words, why do you have to have this ioctl?  What requires this
to be the way the API works?

But again, as you don't need this code, let's just drop it.  Feel free
to revisit it sometime in the future if you ever get hardware and need
this again.

thanks,

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help