| Message ID | 20240615-md-s390-drivers-s390-cio-v1-1-8fc9584e8595@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/cio: add missing MODULE_DESCRIPTION() macros | expand |
On 6/16/24 05:56, Jeff Johnson wrote: > With ARCH=s390, make allmodconfig && make W=1 C=1 reports: > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/s390/cio/ccwgroup.o > WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/s390/cio/vfio_ccw.o > > Add the missing invocations of the MODULE_DESCRIPTION() macro. > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/s390/cio/ccwgroup.c | 1 + > drivers/s390/cio/vfio_ccw_drv.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/s390/cio/ccwgroup.c b/drivers/s390/cio/ccwgroup.c > index b72f672a7720..a741e5012fce 100644 > --- a/drivers/s390/cio/ccwgroup.c > +++ b/drivers/s390/cio/ccwgroup.c > @@ -550,4 +550,5 @@ void ccwgroup_remove_ccwdev(struct ccw_device *cdev) > put_device(&gdev->dev); > } > EXPORT_SYMBOL(ccwgroup_remove_ccwdev); > +MODULE_DESCRIPTION("CCW group bus driver"); the name of the bus here is "ccwgroup" bus without a space. Otherwise this change in ccwgroup.c looks good to me. Thank you for the patch. With the correction mentioned above, Reviewed-by: Vineeth Vijayan <vneethv@linux.ibm.com> > MODULE_LICENSE("GPL"); > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 8ad49030a7bf..49da348355b4 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -488,4 +488,5 @@ static void __exit vfio_ccw_sch_exit(void) > module_init(vfio_ccw_sch_init); > module_exit(vfio_ccw_sch_exit); > > +MODULE_DESCRIPTION("VFIO based Physical Subchannel device driver"); Halil/Mathew/Eric, Could you please comment on this ? > MODULE_LICENSE("GPL v2"); > > --- > base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670 > change-id: 20240615-md-s390-drivers-s390-cio-3598abb802ad >
On Tue, 2024-06-18 at 14:52 +0200, Vineeth Vijayan wrote: > > > On 6/16/24 05:56, Jeff Johnson wrote: > > With ARCH=s390, make allmodconfig && make W=1 C=1 reports: > > WARNING: modpost: missing MODULE_DESCRIPTION() in > > drivers/s390/cio/ccwgroup.o > > WARNING: modpost: missing MODULE_DESCRIPTION() in > > drivers/s390/cio/vfio_ccw.o > > > > Add the missing invocations of the MODULE_DESCRIPTION() macro. > > > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> > > --- > > drivers/s390/cio/ccwgroup.c | 1 + > > drivers/s390/cio/vfio_ccw_drv.c | 1 + > > 2 files changed, 2 insertions(+) > > > > diff --git a/drivers/s390/cio/ccwgroup.c > > b/drivers/s390/cio/ccwgroup.c > > index b72f672a7720..a741e5012fce 100644 > > --- a/drivers/s390/cio/ccwgroup.c > > +++ b/drivers/s390/cio/ccwgroup.c > > @@ -550,4 +550,5 @@ void ccwgroup_remove_ccwdev(struct ccw_device > > *cdev) > > put_device(&gdev->dev); > > } > > EXPORT_SYMBOL(ccwgroup_remove_ccwdev); > > +MODULE_DESCRIPTION("CCW group bus driver"); > > the name of the bus here is "ccwgroup" bus without a space. > Otherwise this change in ccwgroup.c looks good to me. > Thank you for the patch. > > With the correction mentioned above, > Reviewed-by: Vineeth Vijayan <vneethv@linux.ibm.com> > > > > MODULE_LICENSE("GPL"); > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c > > b/drivers/s390/cio/vfio_ccw_drv.c > > index 8ad49030a7bf..49da348355b4 100644 > > --- a/drivers/s390/cio/vfio_ccw_drv.c > > +++ b/drivers/s390/cio/vfio_ccw_drv.c > > @@ -488,4 +488,5 @@ static void __exit vfio_ccw_sch_exit(void) > > module_init(vfio_ccw_sch_init); > > module_exit(vfio_ccw_sch_exit); > > > > +MODULE_DESCRIPTION("VFIO based Physical Subchannel device > > driver"); > > Halil/Mathew/Eric, > Could you please comment on this ? That's what is in the prologue, and is fine. Reviewed-by: Eric Farman <farman@linux.ibm.com> > > > MODULE_LICENSE("GPL v2"); > > > > --- > > base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670 > > change-id: 20240615-md-s390-drivers-s390-cio-3598abb802ad > >
On Tue, 18 Jun 2024 16:11:33 -0400 Eric Farman <farman@linux.ibm.com> wrote: > > > +MODULE_DESCRIPTION("VFIO based Physical Subchannel device > > > driver"); > > > > Halil/Mathew/Eric, > > Could you please comment on this ? > > That's what is in the prologue, and is fine. Eric can you explain it to me why is the attribute "physical" appropriate here? I did a quick grep for "Physical Subchannel" only turned up hits in vfio-ccw. My best guess is that "physical" was somehow intended to mean the opposite of "virtual". But actually it does not matter if our underlying subchannel is emulated or not, at least AFAIU. Regards, Halil
On Wed, 2024-06-19 at 12:32 +0200, Halil Pasic wrote: > On Tue, 18 Jun 2024 16:11:33 -0400 > Eric Farman <farman@linux.ibm.com> wrote: > > > > > +MODULE_DESCRIPTION("VFIO based Physical Subchannel device > > > > driver"); > > > > > > Halil/Mathew/Eric, > > > Could you please comment on this ? > > > > That's what is in the prologue, and is fine. > > Eric can you explain it to me why is the attribute "physical" > appropriate > here? I did a quick grep for "Physical Subchannel" only turned up > hits > in vfio-ccw. One hit, in the prologue comment of this module. "Physical device" adds three to the tally, but only one of those is in vfio-ccw so we should expand your query regarding "physical" vs "emulated" vs "virtual" in the context of, say, tape devices. > > My best guess is that "physical" was somehow intended to mean the > opposite of "virtual". But actually it does not matter if our > underlying > subchannel is emulated or not, at least AFAIU. I also believe this was intended to mean "not virtual," regardless of whether there's emulation taking place underneath. That point is moot since I don't see that information being surfaced, such that the driver can only work with "physical" subchannels. I'm fine with removing it if it bothers you, but I don't see it as an issue. Thanks, Eric > > Regards, > Halil
On 6/19/2024 7:00 AM, Eric Farman wrote: > On Wed, 2024-06-19 at 12:32 +0200, Halil Pasic wrote: >> On Tue, 18 Jun 2024 16:11:33 -0400 >> Eric Farman <farman@linux.ibm.com> wrote: >> >>>>> +MODULE_DESCRIPTION("VFIO based Physical Subchannel device >>>>> driver"); >>>> >>>> Halil/Mathew/Eric, >>>> Could you please comment on this ? >>> >>> That's what is in the prologue, and is fine. >> >> Eric can you explain it to me why is the attribute "physical" >> appropriate >> here? I did a quick grep for "Physical Subchannel" only turned up >> hits >> in vfio-ccw. > > One hit, in the prologue comment of this module. "Physical device" adds > three to the tally, but only one of those is in vfio-ccw so we should > expand your query regarding "physical" vs "emulated" vs "virtual" in > the context of, say, tape devices. > >> >> My best guess is that "physical" was somehow intended to mean the >> opposite of "virtual". But actually it does not matter if our >> underlying >> subchannel is emulated or not, at least AFAIU. > > I also believe this was intended to mean "not virtual," regardless of > whether there's emulation taking place underneath. That point is moot > since I don't see that information being surfaced, such that the driver > can only work with "physical" subchannels. > > I'm fine with removing it if it bothers you, but I don't see it as an > issue. Since I'm not the domain expert here I just copied what was in the prologue. If someone can supply a suitable description, I'll update the patch to use it :) I'm hoping to have these issued cleaned up tree-wide before the 6.11 merge window. /jeff
diff --git a/drivers/s390/cio/ccwgroup.c b/drivers/s390/cio/ccwgroup.c index b72f672a7720..a741e5012fce 100644 --- a/drivers/s390/cio/ccwgroup.c +++ b/drivers/s390/cio/ccwgroup.c @@ -550,4 +550,5 @@ void ccwgroup_remove_ccwdev(struct ccw_device *cdev) put_device(&gdev->dev); } EXPORT_SYMBOL(ccwgroup_remove_ccwdev); +MODULE_DESCRIPTION("CCW group bus driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 8ad49030a7bf..49da348355b4 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -488,4 +488,5 @@ static void __exit vfio_ccw_sch_exit(void) module_init(vfio_ccw_sch_init); module_exit(vfio_ccw_sch_exit); +MODULE_DESCRIPTION("VFIO based Physical Subchannel device driver"); MODULE_LICENSE("GPL v2");
With ARCH=s390, make allmodconfig && make W=1 C=1 reports: WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/s390/cio/ccwgroup.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/s390/cio/vfio_ccw.o Add the missing invocations of the MODULE_DESCRIPTION() macro. Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> --- drivers/s390/cio/ccwgroup.c | 1 + drivers/s390/cio/vfio_ccw_drv.c | 1 + 2 files changed, 2 insertions(+) --- base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670 change-id: 20240615-md-s390-drivers-s390-cio-3598abb802ad