| Message ID | 20240904000020.1686611-4-dave.jiang@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Pull out mailbox bits to be independent of cxl_dev_state | expand |
On 9/4/24 00:59, Dave Jiang wrote: > Group all cxl related kernel headers into include/linux/cxl directory. I tried to do this in the RFC for Type2 support. I was told to just move the required bits for accel drivers able to do CXL initialization, What has changed since then? > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > MAINTAINERS | 2 -- > drivers/acpi/apei/einj-cxl.c | 2 +- > drivers/acpi/apei/ghes.c | 2 +- > drivers/cxl/core/port.c | 2 +- > drivers/cxl/cxlmem.h | 2 +- > include/linux/{einj-cxl.h => cxl/einj.h} | 0 > include/linux/{cxl-event.h => cxl/event.h} | 0 > 7 files changed, 4 insertions(+), 6 deletions(-) > rename include/linux/{einj-cxl.h => cxl/einj.h} (100%) > rename include/linux/{cxl-event.h => cxl/event.h} (100%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index bf59c003da76..45a56af42a79 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5620,8 +5620,6 @@ L: linux-cxl@vger.kernel.org > S: Maintained > F: Documentation/driver-api/cxl > F: drivers/cxl/ > -F: include/linux/einj-cxl.h > -F: include/linux/cxl-event.h > F: include/linux/cxl/ > F: include/uapi/linux/cxl_mem.h > F: tools/testing/cxl/ > diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c > index 8b8be0c90709..54ef0eef18fe 100644 > --- a/drivers/acpi/apei/einj-cxl.c > +++ b/drivers/acpi/apei/einj-cxl.c > @@ -7,7 +7,7 @@ > * > * Author: Ben Cheatham <benjamin.cheatham@amd.com> > */ > -#include <linux/einj-cxl.h> > +#include <linux/cxl/einj.h> > #include <linux/seq_file.h> > #include <linux/pci.h> > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index 623cc0cb4a65..55ec68aa417a 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -27,7 +27,7 @@ > #include <linux/timer.h> > #include <linux/cper.h> > #include <linux/cleanup.h> > -#include <linux/cxl-event.h> > +#include <linux/cxl/event.h> > #include <linux/platform_device.h> > #include <linux/mutex.h> > #include <linux/ratelimit.h> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 1d5007e3795a..6506dc2a71b1 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -3,7 +3,7 @@ > #include <linux/platform_device.h> > #include <linux/memregion.h> > #include <linux/workqueue.h> > -#include <linux/einj-cxl.h> > +#include <linux/cxl/einj.h> > #include <linux/debugfs.h> > #include <linux/device.h> > #include <linux/module.h> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 579d5c647189..838f8f16e7dd 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -6,7 +6,7 @@ > #include <linux/pci.h> > #include <linux/cdev.h> > #include <linux/uuid.h> > -#include <linux/cxl-event.h> > +#include <linux/cxl/event.h> > #include <linux/node.h> > #include <linux/cxl/mailbox.h> > #include "cxl.h" > diff --git a/include/linux/einj-cxl.h b/include/linux/cxl/einj.h > similarity index 100% > rename from include/linux/einj-cxl.h > rename to include/linux/cxl/einj.h > diff --git a/include/linux/cxl-event.h b/include/linux/cxl/event.h > similarity index 100% > rename from include/linux/cxl-event.h > rename to include/linux/cxl/event.h
On 9/3/24 11:52 PM, Alejandro Lucero Palau wrote: > > On 9/4/24 00:59, Dave Jiang wrote: >> Group all cxl related kernel headers into include/linux/cxl directory. > > > I tried to do this in the RFC for Type2 support. > > I was told to just move the required bits for accel drivers able to do CXL initialization, > > What has changed since then? I guess I wasn't there for that discussion. But this make sense to me and Alison seems to agree. The number of cxl related bits are expanding and we probably should group them into its own directory now. I guess we can see if Dan and/or Jonathan objects. > > > >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> MAINTAINERS | 2 -- >> drivers/acpi/apei/einj-cxl.c | 2 +- >> drivers/acpi/apei/ghes.c | 2 +- >> drivers/cxl/core/port.c | 2 +- >> drivers/cxl/cxlmem.h | 2 +- >> include/linux/{einj-cxl.h => cxl/einj.h} | 0 >> include/linux/{cxl-event.h => cxl/event.h} | 0 >> 7 files changed, 4 insertions(+), 6 deletions(-) >> rename include/linux/{einj-cxl.h => cxl/einj.h} (100%) >> rename include/linux/{cxl-event.h => cxl/event.h} (100%) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index bf59c003da76..45a56af42a79 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -5620,8 +5620,6 @@ L: linux-cxl@vger.kernel.org >> S: Maintained >> F: Documentation/driver-api/cxl >> F: drivers/cxl/ >> -F: include/linux/einj-cxl.h >> -F: include/linux/cxl-event.h >> F: include/linux/cxl/ >> F: include/uapi/linux/cxl_mem.h >> F: tools/testing/cxl/ >> diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c >> index 8b8be0c90709..54ef0eef18fe 100644 >> --- a/drivers/acpi/apei/einj-cxl.c >> +++ b/drivers/acpi/apei/einj-cxl.c >> @@ -7,7 +7,7 @@ >> * >> * Author: Ben Cheatham <benjamin.cheatham@amd.com> >> */ >> -#include <linux/einj-cxl.h> >> +#include <linux/cxl/einj.h> >> #include <linux/seq_file.h> >> #include <linux/pci.h> >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >> index 623cc0cb4a65..55ec68aa417a 100644 >> --- a/drivers/acpi/apei/ghes.c >> +++ b/drivers/acpi/apei/ghes.c >> @@ -27,7 +27,7 @@ >> #include <linux/timer.h> >> #include <linux/cper.h> >> #include <linux/cleanup.h> >> -#include <linux/cxl-event.h> >> +#include <linux/cxl/event.h> >> #include <linux/platform_device.h> >> #include <linux/mutex.h> >> #include <linux/ratelimit.h> >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >> index 1d5007e3795a..6506dc2a71b1 100644 >> --- a/drivers/cxl/core/port.c >> +++ b/drivers/cxl/core/port.c >> @@ -3,7 +3,7 @@ >> #include <linux/platform_device.h> >> #include <linux/memregion.h> >> #include <linux/workqueue.h> >> -#include <linux/einj-cxl.h> >> +#include <linux/cxl/einj.h> >> #include <linux/debugfs.h> >> #include <linux/device.h> >> #include <linux/module.h> >> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >> index 579d5c647189..838f8f16e7dd 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -6,7 +6,7 @@ >> #include <linux/pci.h> >> #include <linux/cdev.h> >> #include <linux/uuid.h> >> -#include <linux/cxl-event.h> >> +#include <linux/cxl/event.h> >> #include <linux/node.h> >> #include <linux/cxl/mailbox.h> >> #include "cxl.h" >> diff --git a/include/linux/einj-cxl.h b/include/linux/cxl/einj.h >> similarity index 100% >> rename from include/linux/einj-cxl.h >> rename to include/linux/cxl/einj.h >> diff --git a/include/linux/cxl-event.h b/include/linux/cxl/event.h >> similarity index 100% >> rename from include/linux/cxl-event.h >> rename to include/linux/cxl/event.h >
Alejandro Lucero Palau wrote: > > On 9/4/24 00:59, Dave Jiang wrote: > > Group all cxl related kernel headers into include/linux/cxl directory. > > > I tried to do this in the RFC for Type2 support. > > I was told to just move the required bits for accel drivers able to do > CXL initialization, > > What has changed since then? I saw this subject line fly by and had the same thought, but now that I come to read it there is a significant difference. This is not "move cxl headers to new linux/cxl/ directory" this is "rename existing cxl headers in include/linux/ to include/linux/cxl/". Compare that to the proposal to move all drivers/cxl/*.h headers to include/linux/. Now, I do not think it makes sense to place these cross-driver cxl headers under include/linux/cxl/, just place them under include/cxl/. However, I do not understand the motivation. It's not like a new header is being added and that is the final straw that merits moving the existing headers. Most include/$subsys/ directories have more than 2 header files. So what is the pain motivating this move?
On 9/5/24 03:55, Dan Williams wrote: > Alejandro Lucero Palau wrote: >> On 9/4/24 00:59, Dave Jiang wrote: >>> Group all cxl related kernel headers into include/linux/cxl directory. >> >> I tried to do this in the RFC for Type2 support. >> >> I was told to just move the required bits for accel drivers able to do >> CXL initialization, >> >> What has changed since then? > I saw this subject line fly by and had the same thought, but now that I > come to read it there is a significant difference. > > This is not "move cxl headers to new linux/cxl/ directory" this is > "rename existing cxl headers in include/linux/ to include/linux/cxl/". > > Compare that to the proposal to move all drivers/cxl/*.h headers to > include/linux/. Right. My bad. I did just pay attention to the commit message. > > Now, I do not think it makes sense to place these cross-driver cxl headers under > include/linux/cxl/, just place them under include/cxl/. Not sure you are telling this to me or to Dave. FWIW: I have almost ready v3 and I'm adding headers to include/linux/cxl after this directory was created after v2. As I understand it, include/linux implies headers to be used by kernel code, with headers to just include those to be used by user space. The headers added in v3 are for accel drivers so it makes sense to me to be include/linux/cxl. > However, I do not understand the motivation. It's not like a new header > is being added and that is the final straw that merits moving the > existing headers. Most include/$subsys/ directories have more than 2 > header files. > > So what is the pain motivating this move?
On 9/4/24 11:51 PM, Alejandro Lucero Palau wrote: > > On 9/5/24 03:55, Dan Williams wrote: >> Alejandro Lucero Palau wrote: >>> On 9/4/24 00:59, Dave Jiang wrote: >>>> Group all cxl related kernel headers into include/linux/cxl directory. >>> >>> I tried to do this in the RFC for Type2 support. >>> >>> I was told to just move the required bits for accel drivers able to do >>> CXL initialization, >>> >>> What has changed since then? >> I saw this subject line fly by and had the same thought, but now that I >> come to read it there is a significant difference. >> >> This is not "move cxl headers to new linux/cxl/ directory" this is >> "rename existing cxl headers in include/linux/ to include/linux/cxl/". >> >> Compare that to the proposal to move all drivers/cxl/*.h headers to >> include/linux/. > > > Right. My bad. I did just pay attention to the commit message. > > >> >> Now, I do not think it makes sense to place these cross-driver cxl headers under >> include/linux/cxl/, just place them under include/cxl/. > > > Not sure you are telling this to me or to Dave. I think he's telling me. I chatted with Dan offline. I'm going to send v4 and move stuff to include/cxl/. Sorry about the churn. > > FWIW: > > I have almost ready v3 and I'm adding headers to include/linux/cxl after this directory was created after v2. > > As I understand it, include/linux implies headers to be used by kernel code, with headers to just include those to be used by user space. > > The headers added in v3 are for accel drivers so it makes sense to me to be include/linux/cxl. > > >> However, I do not understand the motivation. It's not like a new header >> is being added and that is the final straw that merits moving the >> existing headers. Most include/$subsys/ directories have more than 2 >> header files. >> >> So what is the pain motivating this move?
diff --git a/MAINTAINERS b/MAINTAINERS index bf59c003da76..45a56af42a79 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5620,8 +5620,6 @@ L: linux-cxl@vger.kernel.org S: Maintained F: Documentation/driver-api/cxl F: drivers/cxl/ -F: include/linux/einj-cxl.h -F: include/linux/cxl-event.h F: include/linux/cxl/ F: include/uapi/linux/cxl_mem.h F: tools/testing/cxl/ diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c index 8b8be0c90709..54ef0eef18fe 100644 --- a/drivers/acpi/apei/einj-cxl.c +++ b/drivers/acpi/apei/einj-cxl.c @@ -7,7 +7,7 @@ * * Author: Ben Cheatham <benjamin.cheatham@amd.com> */ -#include <linux/einj-cxl.h> +#include <linux/cxl/einj.h> #include <linux/seq_file.h> #include <linux/pci.h> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 623cc0cb4a65..55ec68aa417a 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -27,7 +27,7 @@ #include <linux/timer.h> #include <linux/cper.h> #include <linux/cleanup.h> -#include <linux/cxl-event.h> +#include <linux/cxl/event.h> #include <linux/platform_device.h> #include <linux/mutex.h> #include <linux/ratelimit.h> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 1d5007e3795a..6506dc2a71b1 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -3,7 +3,7 @@ #include <linux/platform_device.h> #include <linux/memregion.h> #include <linux/workqueue.h> -#include <linux/einj-cxl.h> +#include <linux/cxl/einj.h> #include <linux/debugfs.h> #include <linux/device.h> #include <linux/module.h> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 579d5c647189..838f8f16e7dd 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -6,7 +6,7 @@ #include <linux/pci.h> #include <linux/cdev.h> #include <linux/uuid.h> -#include <linux/cxl-event.h> +#include <linux/cxl/event.h> #include <linux/node.h> #include <linux/cxl/mailbox.h> #include "cxl.h" diff --git a/include/linux/einj-cxl.h b/include/linux/cxl/einj.h similarity index 100% rename from include/linux/einj-cxl.h rename to include/linux/cxl/einj.h diff --git a/include/linux/cxl-event.h b/include/linux/cxl/event.h similarity index 100% rename from include/linux/cxl-event.h rename to include/linux/cxl/event.h
Group all cxl related kernel headers into include/linux/cxl directory. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- MAINTAINERS | 2 -- drivers/acpi/apei/einj-cxl.c | 2 +- drivers/acpi/apei/ghes.c | 2 +- drivers/cxl/core/port.c | 2 +- drivers/cxl/cxlmem.h | 2 +- include/linux/{einj-cxl.h => cxl/einj.h} | 0 include/linux/{cxl-event.h => cxl/event.h} | 0 7 files changed, 4 insertions(+), 6 deletions(-) rename include/linux/{einj-cxl.h => cxl/einj.h} (100%) rename include/linux/{cxl-event.h => cxl/event.h} (100%)