[PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Andy Shevchenko]

Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
variable, the users, which can be compiled as modules, will be failed to
build. Export the variable to the modules to prevent build breakage.

Reported-by: Woody Suwalski <terraluna977@gmail.com>
Closes: https://lore.kernel.org/r/cc8a93b2-2504-9754-e26c-5d5c3bd1265c@gmail.com
Fixes: 2a49b45cd0e7 ("PNP: Add dev_is_pnp() macro")
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pnp/driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index 0a5d0d8befa8..1394d17bd6f7 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -265,6 +265,7 @@ const struct bus_type pnp_bus_type = {
 	.pm	 = &pnp_bus_dev_pm_ops,
 	.dev_groups = pnp_dev_groups,
 };
+EXPORT_SYMBOL(pnp_bus_type);
 
 int pnp_register_driver(struct pnp_driver *drv)
 {
-- 
2.45.1

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Woody Suwalski]

Andy Shevchenko wrote:
> Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> variable, the users, which can be compiled as modules, will be failed to
> build. Export the variable to the modules to prevent build breakage.
>
> Reported-by: Woody Suwalski <terraluna977@gmail.com>
> Closes: https://lore.kernel.org/r/cc8a93b2-2504-9754-e26c-5d5c3bd1265c@gmail.com
> Fixes: 2a49b45cd0e7 ("PNP: Add dev_is_pnp() macro")
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>   drivers/pnp/driver.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index 0a5d0d8befa8..1394d17bd6f7 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -265,6 +265,7 @@ const struct bus_type pnp_bus_type = {
>   	.pm	 = &pnp_bus_dev_pm_ops,
>   	.dev_groups = pnp_dev_groups,
>   };
> +EXPORT_SYMBOL(pnp_bus_type);
>   
>   int pnp_register_driver(struct pnp_driver *drv)
>   {
Confirm the patch works OK.

Tested-by: Woody Suwalski <terraluna977@gmail.com>

Thanks, Woody

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Christoph Hellwig]

On Mon, May 27, 2024 at 11:24:24PM +0300, Andy Shevchenko wrote:
> Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> variable, the users, which can be compiled as modules, will be failed to
> build. Export the variable to the modules to prevent build breakage.

NAK.  Please move dev_is_pnp out of line and export it (as
EXPORT_SYMBOL_GPL), please.  bus types should be private unless we have
really good reasons for them not to be private.

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Andy Shevchenko]

On Tue, May 28, 2024 at 7:58 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, May 27, 2024 at 11:24:24PM +0300, Andy Shevchenko wrote:
> > Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> > variable, the users, which can be compiled as modules, will be failed to
> > build. Export the variable to the modules to prevent build breakage.
>
> NAK.  Please move dev_is_pnp out of line and export it (as
> EXPORT_SYMBOL_GPL), please.  bus types should be private unless we have
> really good reasons for them not to be private.

Works for me, I'll redo it in the v2, thank you for the review.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Andy Shevchenko]

On Tue, May 28, 2024 at 7:58 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, May 27, 2024 at 11:24:24PM +0300, Andy Shevchenko wrote:
> > Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> > variable, the users, which can be compiled as modules, will be failed to
> > build. Export the variable to the modules to prevent build breakage.
>
> NAK.  Please move dev_is_pnp out of line and export it (as
> EXPORT_SYMBOL_GPL), please.  bus types should be private unless we have
> really good reasons for them not to be private.

FWIW, it's not private, it's just not exported to the modules. Are you
suggesting to hide the bus type completely to make it static? If so,
this is out of scope of this fix.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Christoph Hellwig]

On Tue, May 28, 2024 at 10:12:31AM +0300, Andy Shevchenko wrote:
> > NAK.  Please move dev_is_pnp out of line and export it (as
> > EXPORT_SYMBOL_GPL), please.  bus types should be private unless we have
> > really good reasons for them not to be private.
> 
> FWIW, it's not private, it's just not exported to the modules. Are you
> suggesting to hide the bus type completely to make it static? If so,
> this is out of scope of this fix.

Not exporting it is the most important bit.  Making it private would
be even better of course.

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Rafael J. Wysocki]

On Mon, May 27, 2024 at 10:24 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> variable, the users, which can be compiled as modules, will be failed to
> build. Export the variable to the modules to prevent build breakage.
>
> Reported-by: Woody Suwalski <terraluna977@gmail.com>
> Closes: https://lore.kernel.org/r/cc8a93b2-2504-9754-e26c-5d5c3bd1265c@gmail.com
> Fixes: 2a49b45cd0e7 ("PNP: Add dev_is_pnp() macro")
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/pnp/driver.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index 0a5d0d8befa8..1394d17bd6f7 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -265,6 +265,7 @@ const struct bus_type pnp_bus_type = {
>         .pm      = &pnp_bus_dev_pm_ops,
>         .dev_groups = pnp_dev_groups,
>  };
> +EXPORT_SYMBOL(pnp_bus_type);

Why not EXPORT_SYMBOL_GPL()?

>  int pnp_register_driver(struct pnp_driver *drv)
>  {
> --
> 2.45.1
>
>

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Andy Shevchenko]

On Tue, May 28, 2024 at 12:42 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, May 27, 2024 at 10:24 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> > variable, the users, which can be compiled as modules, will be failed to
> > build. Export the variable to the modules to prevent build breakage.

...

> > +EXPORT_SYMBOL(pnp_bus_type);
>
> Why not EXPORT_SYMBOL_GPL()?

To be consistent with the rest, but in any case Christoph suggested
something else, where _GPL is assumed to be.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] PNP: Export pnp_bus_type for modules

[Rafael J. Wysocki]

On Tue, May 28, 2024 at 11:53 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, May 28, 2024 at 12:42 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Mon, May 27, 2024 at 10:24 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > Since we have dev_is_pnp() macro that utilises the address of pnp_bus_type
> > > variable, the users, which can be compiled as modules, will be failed to
> > > build. Export the variable to the modules to prevent build breakage.
>
> ...
>
> > > +EXPORT_SYMBOL(pnp_bus_type);
> >
> > Why not EXPORT_SYMBOL_GPL()?
>
> To be consistent with the rest, but in any case Christoph suggested
> something else, where _GPL is assumed to be.

Yes, that's even better.