[PATCH v2 0/2] PNP: Fix handling of pnp_bus_type

[PATCH v2 0/2] PNP: Fix handling of pnp_bus_type

[Andy Shevchenko]

There is a fix for recently introduced dev_is_pnp() that involves pnp_bus_type
and one improvement to hide that variable from the wider audience for good.

In v2:
- reworked patch 1 as suggested by Christoph
- added a new patch to make less possibilities for (ab)using pnp_bus_type

Andy Shevchenko (2):
  PNP: Make dev_is_pnp() to be a function and export it for modules
  PNP: Hide pnp_bus_type from the non-PNP code

 drivers/pnp/base.h   | 1 +
 drivers/pnp/driver.c | 6 ++++++
 include/linux/pnp.h  | 6 ++----
 3 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.45.1

[PATCH v2 1/2] PNP: Make dev_is_pnp() to be a function and export it for modules

[Andy Shevchenko]

Since we have a dev_is_pnp() macro that utilises the address of the
pnp_bus_type variable, the users, which can be compiled as modules,
will fail to build. Convert the macro to be a function and export it
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 | 6 ++++++
 include/linux/pnp.h  | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index 0a5d0d8befa8..3483e52e3a81 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -266,6 +266,12 @@ const struct bus_type pnp_bus_type = {
 	.dev_groups = pnp_dev_groups,
 };
 
+bool dev_is_pnp(const struct device *dev)
+{
+	return dev->bus == &pnp_bus_type;
+}
+EXPORT_SYMBOL_GPL(dev_is_pnp);
+
 int pnp_register_driver(struct pnp_driver *drv)
 {
 	drv->driver.name = drv->name;
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 82561242cda4..a8def1cea32c 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -469,7 +469,7 @@ int compare_pnp_id(struct pnp_id *pos, const char *id);
 int pnp_register_driver(struct pnp_driver *drv);
 void pnp_unregister_driver(struct pnp_driver *drv);
 
-#define dev_is_pnp(d) ((d)->bus == &pnp_bus_type)
+bool dev_is_pnp(const struct device *dev);
 
 #else
 
@@ -502,7 +502,7 @@ static inline int compare_pnp_id(struct pnp_id *pos, const char *id) { return -E
 static inline int pnp_register_driver(struct pnp_driver *drv) { return -ENODEV; }
 static inline void pnp_unregister_driver(struct pnp_driver *drv) { }
 
-#define dev_is_pnp(d) false
+static inline bool dev_is_pnp(const struct device *dev) { return false; }
 
 #endif /* CONFIG_PNP */
 
-- 
2.45.1

[PATCH v2 2/2] PNP: Hide pnp_bus_type from the non-PNP code

[Andy Shevchenko]

The pnp_bus_type is defined only when CONFIG_PNP=y, while being
not guarded by ifdeffery in the header. Moreover, it's not used
outside of the PNP code. Move it to the internal header to make
sure no-one will try to (ab)use it.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pnp/base.h  | 1 +
 include/linux/pnp.h | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index e74a0f6a3157..4e80273dfb1e 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -6,6 +6,7 @@
 
 extern struct mutex pnp_lock;
 extern const struct attribute_group *pnp_dev_groups[];
+extern const struct bus_type pnp_bus_type;
 
 int pnp_register_protocol(struct pnp_protocol *protocol);
 void pnp_unregister_protocol(struct pnp_protocol *protocol);
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index a8def1cea32c..7f2ff95d2deb 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -435,8 +435,6 @@ struct pnp_protocol {
 #define protocol_for_each_dev(protocol, dev)	\
 	list_for_each_entry(dev, &(protocol)->devices, protocol_list)
 
-extern const struct bus_type pnp_bus_type;
-
 #if defined(CONFIG_PNP)
 
 /* device management */
-- 
2.45.1

Re: [PATCH v2 1/2] PNP: Make dev_is_pnp() to be a function and export it for modules

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 2/2] PNP: Hide pnp_bus_type from the non-PNP code

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 1/2] PNP: Make dev_is_pnp() to be a function and export it for modules

[Rafael J. Wysocki]

On Tue, May 28, 2024 at 12:27 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> Since we have a dev_is_pnp() macro that utilises the address of the
> pnp_bus_type variable, the users, which can be compiled as modules,
> will fail to build. Convert the macro to be a function and export it
> 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 | 6 ++++++
>  include/linux/pnp.h  | 4 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index 0a5d0d8befa8..3483e52e3a81 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -266,6 +266,12 @@ const struct bus_type pnp_bus_type = {
>         .dev_groups = pnp_dev_groups,
>  };
>
> +bool dev_is_pnp(const struct device *dev)
> +{
> +       return dev->bus == &pnp_bus_type;
> +}
> +EXPORT_SYMBOL_GPL(dev_is_pnp);
> +
>  int pnp_register_driver(struct pnp_driver *drv)
>  {
>         drv->driver.name = drv->name;
> diff --git a/include/linux/pnp.h b/include/linux/pnp.h
> index 82561242cda4..a8def1cea32c 100644
> --- a/include/linux/pnp.h
> +++ b/include/linux/pnp.h
> @@ -469,7 +469,7 @@ int compare_pnp_id(struct pnp_id *pos, const char *id);
>  int pnp_register_driver(struct pnp_driver *drv);
>  void pnp_unregister_driver(struct pnp_driver *drv);
>
> -#define dev_is_pnp(d) ((d)->bus == &pnp_bus_type)
> +bool dev_is_pnp(const struct device *dev);
>
>  #else
>
> @@ -502,7 +502,7 @@ static inline int compare_pnp_id(struct pnp_id *pos, const char *id) { return -E
>  static inline int pnp_register_driver(struct pnp_driver *drv) { return -ENODEV; }
>  static inline void pnp_unregister_driver(struct pnp_driver *drv) { }
>
> -#define dev_is_pnp(d) false
> +static inline bool dev_is_pnp(const struct device *dev) { return false; }
>
>  #endif /* CONFIG_PNP */
>
> --

Applied along with the [2/2] as 6.10-rc material, thanks!