[PATCH v3 0/3] device property: Introduce device_is_compatible()

[PATCH v3 0/3] device property: Introduce device_is_compatible()

[Andy Shevchenko]

Introduce a new helper to tell if device (node) is compatible to the
given string value. This will help some drivers to get rid of unneeded
OF APIs/etc and in may help others to be agnostic to OF/ACPI.

While doing it, I have noticed that ACPI_DEVICE_CLASS() macro seems
defined in unsuitable location. Move it to the better one.

Last patch is an example of what the first two are doing.

The entire series can go, I believe, via ACPI (linux-pm) tree in case
the last patch gets tag from the respective maintainer.

In v3:
- added tag to patch 1 (Rafael), patches 2&3 (Sakari)
- made commit message text wider in patch 3 (Sakari)

In v2:
- updated commit message and added kernel doc for a new API (Greg)
- also replaced acpi_device_get_match_data() with the agnostic API
- tried to keep header inclusions ordered (to some extent)

Andy Shevchenko (3):
  ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
  device property: Implement device_is_compatible()
  ata: ahci_platform: Make code agnostic to OF/ACPI

 drivers/ata/ahci_platform.c     |  8 ++++----
 include/linux/acpi.h            | 14 --------------
 include/linux/mod_devicetable.h | 13 +++++++++++++
 include/linux/property.h        | 12 ++++++++++++
 4 files changed, 29 insertions(+), 18 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v3 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h

[Andy Shevchenko]

The data type of struct acpi_device_id is defined in the
mod_devicetable.h. It's suboptimal to require user with
the almost agnostic code to include acpi.h solely for the
macro that affects the data type defined elsewhere.

Taking into account the above and for the sake of consistency
move ACPI_DEVICE_CLASS() to mod_devicetable.h.

Note, that with CONFIG_ACPI=n the ID table will be filed with data
but it does not really matter because either it won't be used, or
won't be compiled in some cases (when guarded by respective ifdeffery).

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/acpi.h            | 14 --------------
 include/linux/mod_devicetable.h | 13 +++++++++++++
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d41a05d68166..640f1c07c894 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -70,19 +70,6 @@ static inline void acpi_free_fwnode_static(struct fwnode_handle *fwnode)
 	kfree(fwnode);
 }
 
-/**
- * ACPI_DEVICE_CLASS - macro used to describe an ACPI device with
- * the PCI-defined class-code information
- *
- * @_cls : the class, subclass, prog-if triple for this device
- * @_msk : the class mask for this device
- *
- * This macro is used to create a struct acpi_device_id that matches a
- * specific PCI class. The .id and .driver_data fields will be left
- * initialized with the default value.
- */
-#define ACPI_DEVICE_CLASS(_cls, _msk)	.cls = (_cls), .cls_msk = (_msk),
-
 static inline bool has_acpi_companion(struct device *dev)
 {
 	return is_acpi_device_node(dev->fwnode);
@@ -782,7 +769,6 @@ const char *acpi_get_subsystem_id(acpi_handle handle);
 #define ACPI_COMPANION_SET(dev, adev)	do { } while (0)
 #define ACPI_HANDLE(dev)		(NULL)
 #define ACPI_HANDLE_FWNODE(fwnode)	(NULL)
-#define ACPI_DEVICE_CLASS(_cls, _msk)	.cls = (0), .cls_msk = (0),
 
 #include <acpi/acpi_numa.h>
 
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ccaaeda792c0..486747518aae 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -221,6 +221,19 @@ struct acpi_device_id {
 	__u32 cls_msk;
 };
 
+/**
+ * ACPI_DEVICE_CLASS - macro used to describe an ACPI device with
+ * the PCI-defined class-code information
+ *
+ * @_cls : the class, subclass, prog-if triple for this device
+ * @_msk : the class mask for this device
+ *
+ * This macro is used to create a struct acpi_device_id that matches a
+ * specific PCI class. The .id and .driver_data fields will be left
+ * initialized with the default value.
+ */
+#define ACPI_DEVICE_CLASS(_cls, _msk)	.cls = (_cls), .cls_msk = (_msk),
+
 #define PNP_ID_LEN	8
 #define PNP_MAX_DEVICES	8
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v3 2/3] device property: Implement device_is_compatible()

[Andy Shevchenko]

Some users want to use the struct device pointer to see if the
device is compatible in terms of Open Firmware specifications,
i.e. if it has a 'compatible' property and it matches to the
given value. Provide inline helper for the users.

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/property.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 3df7e10156d8..0251138c7c88 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -90,6 +90,18 @@ bool fwnode_device_is_compatible(const struct fwnode_handle *fwnode, const char
 	return fwnode_property_match_string(fwnode, "compatible", compat) >= 0;
 }
 
+/**
+ * device_is_compatible - match 'compatible' property of the device with a given string
+ * @dev: Pointer to the struct device
+ * @compat: The string to match 'compatible' property with
+ *
+ * Returns: true if matches, otherwise false.
+ */
+static inline bool device_is_compatible(const struct device *dev, const char *compat)
+{
+	return fwnode_device_is_compatible(dev_fwnode(dev), compat);
+}
+
 int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 				       const char *prop, const char *nargs_prop,
 				       unsigned int nargs, unsigned int index,
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v3 2/3] device property: Implement device_is_compatible()

[Serge Semin]

On Mon, Jun 12, 2023 at 07:10:10PM +0300, Andy Shevchenko wrote:
> Some users want to use the struct device pointer to see if the
> device is compatible in terms of Open Firmware specifications,
> i.e. if it has a 'compatible' property and it matches to the
> given value. Provide inline helper for the users.
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


C/P v2: IMO much useful wrapper. Thanks for the patch.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

-Serge(y)

> ---
>  include/linux/property.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 3df7e10156d8..0251138c7c88 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -90,6 +90,18 @@ bool fwnode_device_is_compatible(const struct fwnode_handle *fwnode, const char
>  	return fwnode_property_match_string(fwnode, "compatible", compat) >= 0;
>  }
>  
> +/**
> + * device_is_compatible - match 'compatible' property of the device with a given string
> + * @dev: Pointer to the struct device
> + * @compat: The string to match 'compatible' property with
> + *
> + * Returns: true if matches, otherwise false.
> + */
> +static inline bool device_is_compatible(const struct device *dev, const char *compat)
> +{
> +	return fwnode_device_is_compatible(dev_fwnode(dev), compat);
> +}
> +
>  int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
>  				       const char *prop, const char *nargs_prop,
>  				       unsigned int nargs, unsigned int index,
> -- 
> 2.40.0.1.gaa8946217a0b
> 
> 

[PATCH v3 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI

[Andy Shevchenko]

With the help of a new device_is_compatible() make the driver code
agnostic to the OF/ACPI. This makes it neater. As a side effect
the header inclusions is corrected (seems mod_devicetable.h was
implicitly included).

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/ahci_platform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index ab30c7138d73..81fc63f6b008 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -9,14 +9,14 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/device.h>
-#include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
-#include <linux/acpi.h>
 #include <linux/pci_ids.h>
 #include "ahci.h"
 
@@ -56,10 +56,10 @@ static int ahci_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	if (of_device_is_compatible(dev->of_node, "hisilicon,hisi-ahci"))
+	if (device_is_compatible(dev, "hisilicon,hisi-ahci"))
 		hpriv->flags |= AHCI_HFLAG_NO_FBS | AHCI_HFLAG_NO_NCQ;
 
-	port = acpi_device_get_match_data(dev);
+	port = device_get_match_data(dev);
 	if (!port)
 		port = &ahci_port_info;
 
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v3 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI

[Damien Le Moal]

On 6/13/23 01:10, Andy Shevchenko wrote:
> With the help of a new device_is_compatible() make the driver code
> agnostic to the OF/ACPI. This makes it neater. As a side effect
> the header inclusions is corrected (seems mod_devicetable.h was
> implicitly included).
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI

[Serge Semin]

On Mon, Jun 12, 2023 at 07:10:11PM +0300, Andy Shevchenko wrote:
> With the help of a new device_is_compatible() make the driver code
> agnostic to the OF/ACPI. This makes it neater. As a side effect
> the header inclusions is corrected (seems mod_devicetable.h was
> implicitly included).
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

C/P v2:
I don't think the driver will get to be fully agnostic after this
patch because for instance the ahci_platform_get_resources() method
directly uses the OF-available functions, walks over the OF subnodes,
touches the OF-properties, etc. So AFAICS in order to be fully OF/ACPI
agnostic the entire libahci_platform.o driver needs to be converted
too, but it's not trivial at all.

Anyway as a start this patch looks good.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

-Serge(y)

> ---
>  drivers/ata/ahci_platform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index ab30c7138d73..81fc63f6b008 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -9,14 +9,14 @@
>   */
>  
>  #include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/pm.h>
>  #include <linux/device.h>
> -#include <linux/of_device.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/libata.h>
>  #include <linux/ahci_platform.h>
> -#include <linux/acpi.h>
>  #include <linux/pci_ids.h>
>  #include "ahci.h"
>  
> @@ -56,10 +56,10 @@ static int ahci_probe(struct platform_device *pdev)
>  	if (rc)
>  		return rc;
>  
> -	if (of_device_is_compatible(dev->of_node, "hisilicon,hisi-ahci"))
> +	if (device_is_compatible(dev, "hisilicon,hisi-ahci"))
>  		hpriv->flags |= AHCI_HFLAG_NO_FBS | AHCI_HFLAG_NO_NCQ;
>  
> -	port = acpi_device_get_match_data(dev);
> +	port = device_get_match_data(dev);
>  	if (!port)
>  		port = &ahci_port_info;
>  
> -- 
> 2.40.0.1.gaa8946217a0b
> 
> 

Re: [PATCH v3 0/3] device property: Introduce device_is_compatible()

[Rafael J. Wysocki]

On Mon, Jun 12, 2023 at 6:12 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Introduce a new helper to tell if device (node) is compatible to the
> given string value. This will help some drivers to get rid of unneeded
> OF APIs/etc and in may help others to be agnostic to OF/ACPI.
>
> While doing it, I have noticed that ACPI_DEVICE_CLASS() macro seems
> defined in unsuitable location. Move it to the better one.
>
> Last patch is an example of what the first two are doing.
>
> The entire series can go, I believe, via ACPI (linux-pm) tree in case
> the last patch gets tag from the respective maintainer.
>
> In v3:
> - added tag to patch 1 (Rafael), patches 2&3 (Sakari)
> - made commit message text wider in patch 3 (Sakari)
>
> In v2:
> - updated commit message and added kernel doc for a new API (Greg)
> - also replaced acpi_device_get_match_data() with the agnostic API
> - tried to keep header inclusions ordered (to some extent)
>
> Andy Shevchenko (3):
>   ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
>   device property: Implement device_is_compatible()
>   ata: ahci_platform: Make code agnostic to OF/ACPI
>
>  drivers/ata/ahci_platform.c     |  8 ++++----
>  include/linux/acpi.h            | 14 --------------
>  include/linux/mod_devicetable.h | 13 +++++++++++++
>  include/linux/property.h        | 12 ++++++++++++
>  4 files changed, 29 insertions(+), 18 deletions(-)
>
> --

All applied as 6.5 material, thanks!

Re: [PATCH v3 0/3] device property: Introduce device_is_compatible()

[Rafael J. Wysocki]

On Fri, Jun 16, 2023 at 7:55 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Jun 12, 2023 at 6:12 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Introduce a new helper to tell if device (node) is compatible to the
> > given string value. This will help some drivers to get rid of unneeded
> > OF APIs/etc and in may help others to be agnostic to OF/ACPI.
> >
> > While doing it, I have noticed that ACPI_DEVICE_CLASS() macro seems
> > defined in unsuitable location. Move it to the better one.
> >
> > Last patch is an example of what the first two are doing.
> >
> > The entire series can go, I believe, via ACPI (linux-pm) tree in case
> > the last patch gets tag from the respective maintainer.
> >
> > In v3:
> > - added tag to patch 1 (Rafael), patches 2&3 (Sakari)
> > - made commit message text wider in patch 3 (Sakari)
> >
> > In v2:
> > - updated commit message and added kernel doc for a new API (Greg)
> > - also replaced acpi_device_get_match_data() with the agnostic API
> > - tried to keep header inclusions ordered (to some extent)
> >
> > Andy Shevchenko (3):
> >   ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
> >   device property: Implement device_is_compatible()
> >   ata: ahci_platform: Make code agnostic to OF/ACPI
> >
> >  drivers/ata/ahci_platform.c     |  8 ++++----
> >  include/linux/acpi.h            | 14 --------------
> >  include/linux/mod_devicetable.h | 13 +++++++++++++
> >  include/linux/property.h        | 12 ++++++++++++
> >  4 files changed, 29 insertions(+), 18 deletions(-)
> >
> > --
>
> All applied as 6.5 material, thanks!

But I see that Greg has taken it too, so I'll drop it.

Re: [PATCH v3 0/3] device property: Introduce device_is_compatible()

[Andy Shevchenko]

On Fri, Jun 16, 2023 at 09:07:06PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 16, 2023 at 7:55 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Mon, Jun 12, 2023 at 6:12 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:

> > > --
> >
> > All applied as 6.5 material, thanks!
> 
> But I see that Greg has taken it too, so I'll drop it.

Yep, thank you, guys!

-- 
With Best Regards,
Andy Shevchenko