[v1,2/9] pwm: lpss: Move exported symbols to PWM_LPSS namespace

Avoid unnecessary pollution of the global symbol namespace by
moving library functions in to a specific namespace and import
that into the drivers that make use of the functions.

For more info: https://lwn.net/Articles/760045/

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/pwm-lpss-pci.c      | 1 +
 drivers/pwm/pwm-lpss-platform.c | 1 +
 drivers/pwm/pwm-lpss.c          | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

Hello Andy,

On Tue, Sep 06, 2022 at 10:57:28PM +0300, Andy Shevchenko wrote:
> Avoid unnecessary pollution of the global symbol namespace by
> moving library functions in to a specific namespace and import
> that into the drivers that make use of the functions.
> 
> For more info: https://lwn.net/Articles/760045/
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pwm/pwm-lpss-pci.c      | 1 +
>  drivers/pwm/pwm-lpss-platform.c | 1 +
>  drivers/pwm/pwm-lpss.c          | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c
> index 75b778e839b3..9f2c666b95ec 100644
> --- a/drivers/pwm/pwm-lpss-pci.c
> +++ b/drivers/pwm/pwm-lpss-pci.c
> @@ -92,3 +92,4 @@ module_pci_driver(pwm_lpss_driver_pci);
>  
>  MODULE_DESCRIPTION("PWM PCI driver for Intel LPSS");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(PWM_LPSS);
> diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c
> index fcd80cca2f6d..96fde1b2b967 100644
> --- a/drivers/pwm/pwm-lpss-platform.c
> +++ b/drivers/pwm/pwm-lpss-platform.c
> @@ -87,4 +87,5 @@ module_platform_driver(pwm_lpss_driver_platform);
>  
>  MODULE_DESCRIPTION("PWM platform driver for Intel LPSS");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(PWM_LPSS);
>  MODULE_ALIAS("platform:pwm-lpss");

While it's not wrong to add the IMPORT_NS statement to each file, I'd
had added it to pwm-lpss.h. IMHO that makes sense as every includer of
that header needs that IMPORT_NS to actually use the symbols declared
there.

> diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
> index 36d4e83e6b79..a82a57eb2482 100644
> --- a/drivers/pwm/pwm-lpss.c
> +++ b/drivers/pwm/pwm-lpss.c
> @@ -250,7 +250,7 @@ struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r,
>  
>  	return lpwm;
>  }
> -EXPORT_SYMBOL_GPL(pwm_lpss_probe);
> +EXPORT_SYMBOL_NS_GPL(pwm_lpss_probe, PWM_LPSS);

There is something possible with more magic:

	#define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS

which you only need once in pwm-lpss.c and then all exports use that
namespace. (And if you pick up my suggestion for patch 1 you also
benefit from that.)

Best regards
Uwe

On Wed, Sep 07, 2022 at 11:11:44AM +0200, Uwe Kleine-König wrote:
> On Tue, Sep 06, 2022 at 10:57:28PM +0300, Andy Shevchenko wrote:

> >  MODULE_DESCRIPTION("PWM platform driver for Intel LPSS");
> >  MODULE_LICENSE("GPL v2");
> > +MODULE_IMPORT_NS(PWM_LPSS);
> >  MODULE_ALIAS("platform:pwm-lpss");
> 
> While it's not wrong to add the IMPORT_NS statement to each file, I'd
> had added it to pwm-lpss.h. IMHO that makes sense as every includer of
> that header needs that IMPORT_NS to actually use the symbols declared
> there.

If you have an optional dependency you may not need to include namespace
to avoid dragging it for peanuts.

...

> > -EXPORT_SYMBOL_GPL(pwm_lpss_probe);
> > +EXPORT_SYMBOL_NS_GPL(pwm_lpss_probe, PWM_LPSS);
> 
> There is something possible with more magic:

I know.

> 	#define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS
> 
> which you only need once in pwm-lpss.c and then all exports use that
> namespace. (And if you pick up my suggestion for patch 1 you also
> benefit from that.)

For a single export (even for a few of them) it's an overkill.

Taking above into consideration I don't think we need to alter
a proposed change.

Thanks for review!

On Wed, Sep 07, 2022 at 05:21:53PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 07, 2022 at 11:11:44AM +0200, Uwe Kleine-König wrote:
> > On Tue, Sep 06, 2022 at 10:57:28PM +0300, Andy Shevchenko wrote:

...

> > > -EXPORT_SYMBOL_GPL(pwm_lpss_probe);
> > > +EXPORT_SYMBOL_NS_GPL(pwm_lpss_probe, PWM_LPSS);
> > 
> > There is something possible with more magic:
> > 	#define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS
> > 
> > which you only need once in pwm-lpss.c and then all exports use that
> > namespace. (And if you pick up my suggestion for patch 1 you also
> > benefit from that.)
> 
> For a single export (even for a few of them) it's an overkill.

Ah, you adding there 4 more. But still I think it's an overkill. It's so small
driver that duplicating namespace in each of the exported symbols is not an
issue, is it?

On Wed, Sep 07, 2022 at 08:00:42PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 07, 2022 at 05:21:53PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 07, 2022 at 11:11:44AM +0200, Uwe Kleine-König wrote:
> > > On Tue, Sep 06, 2022 at 10:57:28PM +0300, Andy Shevchenko wrote:

...

> > > > -EXPORT_SYMBOL_GPL(pwm_lpss_probe);
> > > > +EXPORT_SYMBOL_NS_GPL(pwm_lpss_probe, PWM_LPSS);
> > > 
> > > There is something possible with more magic:
> > > 	#define DEFAULT_SYMBOL_NAMESPACE PWM_LPSS
> > > 
> > > which you only need once in pwm-lpss.c and then all exports use that
> > > namespace. (And if you pick up my suggestion for patch 1 you also
> > > benefit from that.)
> > 
> > For a single export (even for a few of them) it's an overkill.
> 
> Ah, you adding there 4 more. But still I think it's an overkill. It's so small
> driver that duplicating namespace in each of the exported symbols is not an
> issue, is it?

Okay, now looking at the patch organization (I forgot that I moved NS one to be
not the first one) your suggestion makes a point. We won't change the code we
just introduced.

That said, I would like to get your SoB or what you agree with to the patch 1
and I make this one as you suggested.