[v1] s390/cio: add missing MODULE_DESCRIPTION() macros

[PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Jeff Johnson 1 year, 6 months ago

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");
 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");

---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240615-md-s390-drivers-s390-cio-3598abb802ad

Re: [PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Vineeth Vijayan 1 year, 6 months ago


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
>

Re: [PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Eric Farman 1 year, 6 months ago

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
> >

Re: [PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Halil Pasic 1 year, 5 months ago

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

Re: [PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Eric Farman 1 year, 5 months ago

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

Re: [PATCH] s390/cio: add missing MODULE_DESCRIPTION() macros

Posted by Jeff Johnson 1 year, 5 months ago

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