[PATCH v2 0/2] pwm: lpss: module namespace fixes

[PATCH v2 0/2] pwm: lpss: module namespace fixes

[Uwe Kleine-König]

Hello,

in (implicit) v1[1] I suggested to move the MODULE_IMPORT_NS magic into
include/linux/platform_data/x86/pwm-lpss.h. While I still consider that
a good idea I was overruled and so here comes the variant with the
explicit MODULE_IMPORT_NS in each module that makes use of the
respective symbols.

I don't have a fixed merge plan for this series (assuming it gets the
expected Acks). Either way (both via pwm, both via pinctrl or the
pinctrl patch first and the pwm one in the next cycle) is fine for me.

Best regards
Uwe

[1] https://lore.kernel.org/linux-pwm/cover.1733245406.git.ukleinek@kernel.org

Uwe Kleine-König (2):
  pinctrl: intel: Import namespace for pwm_lpss function
  pwm: lpss: Actually use a module namespace by defining the namespace
    earlier

 drivers/pinctrl/intel/pinctrl-intel.c | 3 +++
 drivers/pwm/pwm-lpss.c                | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

base-commit: 232f121837ad8b1c21cc80f2c8842a4090c5a2a0
-- 
2.47.1

[PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Uwe Kleine-König]

The intel pinctrl driver can provide a PWM device and for that needs to
call the function devm_pwm_lpss_probe(). That function is provided by
the pwm-lpss driver which intends to export it in the "PWM_LPSS"
namespace. To prepare fixing the pwm-lpss driver to indeed use the
"PWM_LPSS" namespace, import that namespace when used.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 527e4b87ae52..d7f1ebb1919f 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1941,3 +1941,6 @@ MODULE_AUTHOR("Mathias Nyman <mathias.nyman@linux.intel.com>");
 MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
 MODULE_DESCRIPTION("Intel pinctrl/GPIO core driver");
 MODULE_LICENSE("GPL v2");
+#if IS_REACHABLE(CONFIG_PWM_LPSS)
+MODULE_IMPORT_NS("PWM_LPSS");
+#endif
-- 
2.47.1

Re: [PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Andy Shevchenko]

On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> The intel pinctrl driver can provide a PWM device and for that needs to
> call the function devm_pwm_lpss_probe(). That function is provided by
> the pwm-lpss driver which intends to export it in the "PWM_LPSS"
> namespace. To prepare fixing the pwm-lpss driver to indeed use the
> "PWM_LPSS" namespace, import that namespace when used.

...

> +#if IS_REACHABLE(CONFIG_PWM_LPSS)

> +#endif

Why?

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

Hello Andy,

On Thu, Jan 23, 2025 at 01:45:17PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> >
> > The intel pinctrl driver can provide a PWM device and for that needs to
> > call the function devm_pwm_lpss_probe(). That function is provided by
> > the pwm-lpss driver which intends to export it in the "PWM_LPSS"
> > namespace. To prepare fixing the pwm-lpss driver to indeed use the
> > "PWM_LPSS" namespace, import that namespace when used.
> 
> ...
> 
> > +#if IS_REACHABLE(CONFIG_PWM_LPSS)
> 
> > +#endif
> 
> Why?

Because devm_pwm_lpss_probe() is only used #if
IS_REACHABLE(CONFIG_PWM_LPSS). Without the #if but with
https://lore.kernel.org/all/20250123110951.370759-2-u.kleine-koenig@baylibre.com/
this results in a warning (with W=1) :-)

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Andy Shevchenko]

On Thu, Jan 23, 2025 at 03:52:06PM +0100, Uwe Kleine-König wrote:
> On Thu, Jan 23, 2025 at 01:45:17PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
> > <u.kleine-koenig@baylibre.com> wrote:
> > >
> > > The intel pinctrl driver can provide a PWM device and for that needs to
> > > call the function devm_pwm_lpss_probe(). That function is provided by
> > > the pwm-lpss driver which intends to export it in the "PWM_LPSS"
> > > namespace. To prepare fixing the pwm-lpss driver to indeed use the
> > > "PWM_LPSS" namespace, import that namespace when used.

...

> > > +#if IS_REACHABLE(CONFIG_PWM_LPSS)
> > 
> > > +#endif
> > 
> > Why?
> 
> Because devm_pwm_lpss_probe() is only used #if
> IS_REACHABLE(CONFIG_PWM_LPSS). Without the #if but with
> https://lore.kernel.org/all/20250123110951.370759-2-u.kleine-koenig@baylibre.com/
> this results in a warning (with W=1) :-)

There is no such commit in the current Linux Next, so let's solve the issues
when they come up.

I'm okay to take your series via Intel pin control without that ifdeffery.
If you don't agree on the change, we need to find the way how to avoid ugly
ifdeffery from day 1.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

On Thu, Jan 23, 2025 at 05:48:17PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 23, 2025 at 03:52:06PM +0100, Uwe Kleine-König wrote:
> > On Thu, Jan 23, 2025 at 01:45:17PM +0200, Andy Shevchenko wrote:
> > > On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
> > > <u.kleine-koenig@baylibre.com> wrote:
> > > >
> > > > The intel pinctrl driver can provide a PWM device and for that needs to
> > > > call the function devm_pwm_lpss_probe(). That function is provided by
> > > > the pwm-lpss driver which intends to export it in the "PWM_LPSS"
> > > > namespace. To prepare fixing the pwm-lpss driver to indeed use the
> > > > "PWM_LPSS" namespace, import that namespace when used.
> 
> ...
> 
> > > > +#if IS_REACHABLE(CONFIG_PWM_LPSS)
> > > 
> > > > +#endif
> > > 
> > > Why?
> > 
> > Because devm_pwm_lpss_probe() is only used #if
> > IS_REACHABLE(CONFIG_PWM_LPSS). Without the #if but with
> > https://lore.kernel.org/all/20250123110951.370759-2-u.kleine-koenig@baylibre.com/
> > this results in a warning (with W=1) :-)
> 
> There is no such commit in the current Linux Next, so let's solve the issues
> when they come up.
> 
> I'm okay to take your series via Intel pin control without that ifdeffery.
> If you don't agree on the change, we need to find the way how to avoid ugly
> ifdeffery from day 1.

I'm ok with dropping the #if. Should I resend or do you want to edit the
patch? Would you take both patches then?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] pinctrl: intel: Import namespace for pwm_lpss function

[Andy Shevchenko]

On Thu, Jan 23, 2025 at 05:30:07PM +0100, Uwe Kleine-König wrote:
> On Thu, Jan 23, 2025 at 05:48:17PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 23, 2025 at 03:52:06PM +0100, Uwe Kleine-König wrote:
> > > On Thu, Jan 23, 2025 at 01:45:17PM +0200, Andy Shevchenko wrote:
> > > > On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > >
> > > > > The intel pinctrl driver can provide a PWM device and for that needs to
> > > > > call the function devm_pwm_lpss_probe(). That function is provided by
> > > > > the pwm-lpss driver which intends to export it in the "PWM_LPSS"
> > > > > namespace. To prepare fixing the pwm-lpss driver to indeed use the
> > > > > "PWM_LPSS" namespace, import that namespace when used.

...

> > > > > +#if IS_REACHABLE(CONFIG_PWM_LPSS)
> > > > 
> > > > > +#endif
> > > > 
> > > > Why?
> > > 
> > > Because devm_pwm_lpss_probe() is only used #if
> > > IS_REACHABLE(CONFIG_PWM_LPSS). Without the #if but with
> > > https://lore.kernel.org/all/20250123110951.370759-2-u.kleine-koenig@baylibre.com/
> > > this results in a warning (with W=1) :-)
> > 
> > There is no such commit in the current Linux Next, so let's solve the issues
> > when they come up.
> > 
> > I'm okay to take your series via Intel pin control without that ifdeffery.
> > If you don't agree on the change, we need to find the way how to avoid ugly
> > ifdeffery from day 1.
> 
> I'm ok with dropping the #if. Should I resend or do you want to edit the
> patch? Would you take both patches then?

I can take both. No need to resend.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] pwm: lpss: Actually use a module namespace by defining the namespace earlier

[Uwe Kleine-König]

DEFAULT_SYMBOL_NAMESPACE must be already defined when <linux/export.h>
is included. So move the define above the include block.

With the DEFAULT_SYMBOL_NAMESPACE being defined too late, the exported
symbols end up in the default namespace. So the respective modules can
use the symbols defined in pwm-lpss.c just fine and up to now just
imported the PWM_LPSS namespace without any gain.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-lpss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 3b99feb3bb49..e3c72ed7fff1 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -10,6 +10,8 @@
  * Author: Alan Cox <alan@linux.intel.com>
  */
 
+#define DEFAULT_SYMBOL_NAMESPACE "PWM_LPSS"
+
 #include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -19,8 +21,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/time.h>
 
-#define DEFAULT_SYMBOL_NAMESPACE "PWM_LPSS"
-
 #include "pwm-lpss.h"
 
 #define PWM				0x00000000
-- 
2.47.1

Re: [PATCH v2 0/2] pwm: lpss: module namespace fixes

[Andy Shevchenko]

On Thu, Jan 23, 2025 at 12:11 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello,
>
> in (implicit) v1[1] I suggested to move the MODULE_IMPORT_NS magic into
> include/linux/platform_data/x86/pwm-lpss.h. While I still consider that
> a good idea I was overruled and so here comes the variant with the
> explicit MODULE_IMPORT_NS in each module that makes use of the
> respective symbols.
>
> I don't have a fixed merge plan for this series (assuming it gets the
> expected Acks). Either way (both via pwm, both via pinctrl or the
> pinctrl patch first and the pwm one in the next cycle) is fine for me.

I can take this via the Intel pin control tree. I only don't like the
ifdeffery in MODULE_*() block.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] pwm: lpss: module namespace fixes

[Andy Shevchenko]

On Thu, Jan 23, 2025 at 11:11:09AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> in (implicit) v1[1] I suggested to move the MODULE_IMPORT_NS magic into
> include/linux/platform_data/x86/pwm-lpss.h. While I still consider that
> a good idea I was overruled and so here comes the variant with the
> explicit MODULE_IMPORT_NS in each module that makes use of the
> respective symbols.
> 
> I don't have a fixed merge plan for this series (assuming it gets the
> expected Acks). Either way (both via pwm, both via pinctrl or the
> pinctrl patch first and the pwm one in the next cycle) is fine for me.

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko