| Message ID | 20220407192913.345411-4-sumit.saxena@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | mpi3mr: add BSG interface support for controller management | expand |
On 4/7/22 12:29, Sumit Saxena wrote: > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h b/include/uapi/scsi/mpi3mr/mpi30_cnfg.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h > rename to include/uapi/scsi/mpi3mr/mpi30_cnfg.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_image.h b/include/uapi/scsi/mpi3mr/mpi30_image.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_image.h > rename to include/uapi/scsi/mpi3mr/mpi30_image.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_init.h b/include/uapi/scsi/mpi3mr/mpi30_init.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_init.h > rename to include/uapi/scsi/mpi3mr/mpi30_init.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h b/include/uapi/scsi/mpi3mr/mpi30_ioc.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_ioc.h > rename to include/uapi/scsi/mpi3mr/mpi30_ioc.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_pci.h b/include/uapi/scsi/mpi3mr/mpi30_pci.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_pci.h > rename to include/uapi/scsi/mpi3mr/mpi30_pci.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_sas.h b/include/uapi/scsi/mpi3mr/mpi30_sas.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_sas.h > rename to include/uapi/scsi/mpi3mr/mpi30_sas.h > diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_transport.h b/include/uapi/scsi/mpi3mr/mpi30_transport.h > similarity index 100% > rename from drivers/scsi/mpi3mr/mpi/mpi30_transport.h > rename to include/uapi/scsi/mpi3mr/mpi30_transport.h Please only move the definitions that are useful for user space into include/uapi instead of moving all definitions, including those that are only used inside the kernel. Thanks, Bart.
On Thu, Apr 07, 2022 at 03:29:08PM -0400, Sumit Saxena wrote: > MPI headers are used by user space applications so > it makes sense to move them to uapi/scsi/mpi3mr. I think this is a horrible idea. These headers are a huge and a bit of a mess, and no we need to provide uapi guarantees for them.
On 4/7/22 22:13, Christoph Hellwig wrote: > On Thu, Apr 07, 2022 at 03:29:08PM -0400, Sumit Saxena wrote: >> MPI headers are used by user space applications so >> it makes sense to move them to uapi/scsi/mpi3mr. > > I think this is a horrible idea. These headers are a huge and a bit of > a mess, and no we need to provide uapi guarantees for them. Hi Christoph, Although I agree with the above: my understanding is that this patch series adds a new NVMe pass-through mechanism but without adding the necessary declarations under include/uapi. That is why I asked recently to move the necessary declarations into the include/uapi directory. Bart.
Bart, We can submit another patchset where we will move only structures which are used both by applications and drivers into the include/uapi and keep the remaining ones which are not used in the app into the older mpi3mr/mpi folder. However, I have a generic question on why we need to move the headers into uapi because the driver provides the transfer mechanism already in the uapi through app.h and the information transferred in through that is blob from the driver perspective and that goes to the controller directly and processed by the controller, only for specific cases like NVMe encapsulated command, to set up the DMA address the driver parse through the command. Wouldn't it make sense to keep all of the controller/firmware related structures along with the driver and expose only the transport mechanism in the uapi? Thanks Sathya On Fri, Apr 8, 2022 at 8:37 AM Bart Van Assche <bvanassche@acm.org> wrote: > > On 4/7/22 22:13, Christoph Hellwig wrote: > > On Thu, Apr 07, 2022 at 03:29:08PM -0400, Sumit Saxena wrote: > >> MPI headers are used by user space applications so > >> it makes sense to move them to uapi/scsi/mpi3mr. > > > > I think this is a horrible idea. These headers are a huge and a bit of > > a mess, and no we need to provide uapi guarantees for them. > > Hi Christoph, > > Although I agree with the above: my understanding is that this patch > series adds a new NVMe pass-through mechanism but without adding the > necessary declarations under include/uapi. That is why I asked recently > to move the necessary declarations into the include/uapi directory. > > Bart.
On 4/8/22 07:45, Sathya Prakash Veerichetty wrote: > We can submit another patchset where we will move only structures > which are used both by applications and drivers into the include/uapi > and keep the remaining ones which are not used in the app into the > older mpi3mr/mpi folder. However, I have a generic question on why > we need to move the headers into uapi because the driver provides the > transfer mechanism already in the uapi through app.h and the > information transferred in through that is blob from the driver > perspective and that goes to the controller directly and processed by > the controller, only for specific cases like NVMe encapsulated > command, to set up the DMA address the driver parse through the > command. Wouldn't it make sense to keep all of the > controller/firmware related structures along with the driver and > expose only the transport mechanism in the uapi? Hi Sathya, Please reply below the original email instead of above. From https://en.wikipedia.org/wiki/Posting_style: A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? Regarding the question in your email: the convention in the Linux kernel is to define data structures that are needed by user space applications (and only those data structures) under the include/uapi directory. These data structures are not only exported to user space apps but also used by the kernel code that implements the user space API. This guarantees the correctness of these header files. I can't find the app.h header file anywhere in the kernel tree. Hence my request to move the user space data structures and constant definitions into a header file under the include/uapi/ directory. Thanks, Bart.
diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h index 877b0925dbc5..7538571e61a6 100644 --- a/drivers/scsi/mpi3mr/mpi3mr.h +++ b/drivers/scsi/mpi3mr/mpi3mr.h @@ -39,13 +39,13 @@ #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> -#include "mpi/mpi30_transport.h" -#include "mpi/mpi30_cnfg.h" -#include "mpi/mpi30_image.h" -#include "mpi/mpi30_init.h" -#include "mpi/mpi30_ioc.h" -#include "mpi/mpi30_sas.h" -#include "mpi/mpi30_pci.h" +#include <uapi/scsi/mpi3mr/mpi30_transport.h> +#include <uapi/scsi/mpi3mr/mpi30_cnfg.h> +#include <uapi/scsi/mpi3mr/mpi30_image.h> +#include <uapi/scsi/mpi3mr/mpi30_init.h> +#include <uapi/scsi/mpi3mr/mpi30_ioc.h> +#include <uapi/scsi/mpi3mr/mpi30_sas.h> +#include <uapi/scsi/mpi3mr/mpi30_pci.h> #include "mpi3mr_debug.h" /* Global list and lock for storing multiple adapters managed by the driver */ diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h b/include/uapi/scsi/mpi3mr/mpi30_cnfg.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_cnfg.h rename to include/uapi/scsi/mpi3mr/mpi30_cnfg.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_image.h b/include/uapi/scsi/mpi3mr/mpi30_image.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_image.h rename to include/uapi/scsi/mpi3mr/mpi30_image.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_init.h b/include/uapi/scsi/mpi3mr/mpi30_init.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_init.h rename to include/uapi/scsi/mpi3mr/mpi30_init.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_ioc.h b/include/uapi/scsi/mpi3mr/mpi30_ioc.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_ioc.h rename to include/uapi/scsi/mpi3mr/mpi30_ioc.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_pci.h b/include/uapi/scsi/mpi3mr/mpi30_pci.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_pci.h rename to include/uapi/scsi/mpi3mr/mpi30_pci.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_sas.h b/include/uapi/scsi/mpi3mr/mpi30_sas.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_sas.h rename to include/uapi/scsi/mpi3mr/mpi30_sas.h diff --git a/drivers/scsi/mpi3mr/mpi/mpi30_transport.h b/include/uapi/scsi/mpi3mr/mpi30_transport.h similarity index 100% rename from drivers/scsi/mpi3mr/mpi/mpi30_transport.h rename to include/uapi/scsi/mpi3mr/mpi30_transport.h
MPI headers are used by user space applications so it makes sense to move them to uapi/scsi/mpi3mr. Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com> --- drivers/scsi/mpi3mr/mpi3mr.h | 14 +++++++------- .../mpi => include/uapi/scsi/mpi3mr}/mpi30_cnfg.h | 0 .../mpi => include/uapi/scsi/mpi3mr}/mpi30_image.h | 0 .../mpi => include/uapi/scsi/mpi3mr}/mpi30_init.h | 0 .../mpi => include/uapi/scsi/mpi3mr}/mpi30_ioc.h | 0 .../mpi => include/uapi/scsi/mpi3mr}/mpi30_pci.h | 0 .../mpi => include/uapi/scsi/mpi3mr}/mpi30_sas.h | 0 .../uapi/scsi/mpi3mr}/mpi30_transport.h | 0 8 files changed, 7 insertions(+), 7 deletions(-) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_cnfg.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_image.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_init.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_ioc.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_pci.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_sas.h (100%) rename {drivers/scsi/mpi3mr/mpi => include/uapi/scsi/mpi3mr}/mpi30_transport.h (100%)