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

[PATCH v2 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 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 v2 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).

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

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

[Rafael J. Wysocki]

On Fri, Jun 9, 2023 at 5:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> 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).
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

or please let me know if you want me to apply this.

> ---
>  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
>

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

[Andy Shevchenko]

On Fri, Jun 09, 2023 at 07:32:53PM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 9, 2023 at 5:49 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > 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).
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> or please let me know if you want me to apply this.

The first patch can be applied independently. But it may be better to apply
all three via one tree (linux-pm was mentioned in the cover letter) as it
shows the point.

-- 
With Best Regards,
Andy Shevchenko

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

[Damien Le Moal]

On 6/10/23 02:32, Rafael J. Wysocki wrote:
> On Fri, Jun 9, 2023 at 5:49 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>>
>> 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).
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> or please let me know if you want me to apply this.

Probably better if you take the whole thing. But if needed, I can take this
through the ata tree.


-- 
Damien Le Moal
Western Digital Research

[PATCH v2 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.

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 695053c60306..0222b77dd75c 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -85,6 +85,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 v2 2/3] device property: Implement device_is_compatible()

[Sakari Ailus]

Hi Andy,

On Fri, Jun 09, 2023 at 06:48:59PM +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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

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

[Serge Semin]

On Fri, Jun 09, 2023 at 06:48:59PM +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.

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

-Serge(y)

> 
> 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 695053c60306..0222b77dd75c 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -85,6 +85,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 v2 2/3] device property: Implement device_is_compatible()

[Andy Shevchenko]

On Tue, Jun 13, 2023 at 12:45:08PM +0300, Serge Semin wrote:
> On Fri, Jun 09, 2023 at 06:48:59PM +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.
> 
> IMO much useful wrapper. Thanks for the patch.
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

There is a v3. Do you mind to tag it as well?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jun 13, 2023 at 06:14:30PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 13, 2023 at 12:45:08PM +0300, Serge Semin wrote:
> > On Fri, Jun 09, 2023 at 06:48:59PM +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.
> > 
> > IMO much useful wrapper. Thanks for the patch.
> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> 
> There is a v3. Do you mind to tag it as well?

Ah, and thank you for review! Appreciate it.

-- 
With Best Regards,
Andy Shevchenko

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

[Serge Semin]

On Tue, Jun 13, 2023 at 06:14:51PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 13, 2023 at 06:14:30PM +0300, Andy Shevchenko wrote:
> > On Tue, Jun 13, 2023 at 12:45:08PM +0300, Serge Semin wrote:
> > > On Fri, Jun 09, 2023 at 06:48:59PM +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.
> > > 
> > > IMO much useful wrapper. Thanks for the patch.
> > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > 
> > There is a v3. Do you mind to tag it as well?
> 
> Ah, and thank you for review! Appreciate it.

Always welcome.) I've resent the tags for the patches 2 and 3. Patch 1
is out of my competence, sorry.

-Serge(y)

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
> 

[PATCH v2 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).

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 v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI

[Sakari Ailus]

Hi Andy,

On Fri, Jun 09, 2023 at 06:49:00PM +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).

You're wrapping the lines well before 75. Why?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

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

[Andy Shevchenko]

On Mon, Jun 12, 2023 at 08:02:44AM +0000, Sakari Ailus wrote:
> On Fri, Jun 09, 2023 at 06:49:00PM +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).
> 
> You're wrapping the lines well before 75. Why?

Didn't pay attention to that much. Is it a problem? Should I send a new
version because of that?

> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Thank you!

-- 
With Best Regards,
Andy Shevchenko

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

[Sakari Ailus]

On Mon, Jun 12, 2023 at 06:20:17PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 12, 2023 at 08:02:44AM +0000, Sakari Ailus wrote:
> > On Fri, Jun 09, 2023 at 06:49:00PM +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).
> > 
> > You're wrapping the lines well before 75. Why?
> 
> Didn't pay attention to that much. Is it a problem? Should I send a new
> version because of that?

I guess not. But it's a good practice to wrap at 75 instead.

-- 
Sakari Ailus

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

[Sakari Ailus]

Hi Andy,

On Fri, Jun 09, 2023 at 06:49:00PM +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).
> 
> 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);

There are just a handful of users for acpi_device_get_match_data() in the
tree. The code could be moved to acpi_fwnode_device_get_match_data() after
coverting these. May be out of scope of this set though.

>  	if (!port)
>  		port = &ahci_port_info;
>  

-- 
Kind regards,

Sakari Ailus

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

[Andy Shevchenko]

On Mon, Jun 12, 2023 at 09:06:52AM +0000, Sakari Ailus wrote:
> On Fri, Jun 09, 2023 at 06:49:00PM +0300, Andy Shevchenko wrote:

...

> > -	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);
> 
> There are just a handful of users for acpi_device_get_match_data() in the
> tree. The code could be moved to acpi_fwnode_device_get_match_data() after
> coverting these. May be out of scope of this set though.

Why do we need that one if we can use device_get_match_data() directly?
It will be also flexible in case one of OF code will need something like
this (custom info structure for the respective compatible string).
That said, I don't think we need to change to acpi_*() whatever.

-- 
With Best Regards,
Andy Shevchenko

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

[Sakari Ailus]

Hi Andy,

On Mon, Jun 12, 2023 at 06:19:22PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 12, 2023 at 09:06:52AM +0000, Sakari Ailus wrote:
> > On Fri, Jun 09, 2023 at 06:49:00PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > -	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);
> > 
> > There are just a handful of users for acpi_device_get_match_data() in the
> > tree. The code could be moved to acpi_fwnode_device_get_match_data() after
> > coverting these. May be out of scope of this set though.
> 
> Why do we need that one if we can use device_get_match_data() directly?

That was what I wanted to point your attention to. ;-)

> It will be also flexible in case one of OF code will need something like
> this (custom info structure for the respective compatible string).
> That said, I don't think we need to change to acpi_*() whatever.

I agree.

-- 
Kind regards,

Sakari Ailus

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

[Serge Semin]

On Fri, Jun 09, 2023 at 06:49:00PM +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).

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)

> 
> 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
> 
>