LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] device property: Introduce device_is_compatible()
@ 2023-06-09 15:48 Andy Shevchenko
  2023-06-09 15:48 ` [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h Andy Shevchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-09 15:48 UTC (permalink / raw)
  To: Damien Le Moal, Serge Semin, Andy Shevchenko, Greg Kroah-Hartman,
	linux-ide, linux-kernel, linux-acpi
  Cc: Hans de Goede, Jens Axboe, Rafael J. Wysocki, Len Brown,
	Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
  2023-06-09 15:48 [PATCH v2 0/3] device property: Introduce device_is_compatible() Andy Shevchenko
@ 2023-06-09 15:48 ` Andy Shevchenko
  2023-06-09 17:32   ` Rafael J. Wysocki
  2023-06-09 15:48 ` [PATCH v2 2/3] device property: Implement device_is_compatible() Andy Shevchenko
  2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
  2 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-09 15:48 UTC (permalink / raw)
  To: Damien Le Moal, Serge Semin, Andy Shevchenko, Greg Kroah-Hartman,
	linux-ide, linux-kernel, linux-acpi
  Cc: Hans de Goede, Jens Axboe, Rafael J. Wysocki, Len Brown,
	Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-09 15:48 [PATCH v2 0/3] device property: Introduce device_is_compatible() Andy Shevchenko
  2023-06-09 15:48 ` [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h Andy Shevchenko
@ 2023-06-09 15:48 ` Andy Shevchenko
  2023-06-12  7:57   ` Sakari Ailus
  2023-06-13  9:45   ` Serge Semin
  2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
  2 siblings, 2 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-09 15:48 UTC (permalink / raw)
  To: Damien Le Moal, Serge Semin, Andy Shevchenko, Greg Kroah-Hartman,
	linux-ide, linux-kernel, linux-acpi
  Cc: Hans de Goede, Jens Axboe, Rafael J. Wysocki, Len Brown,
	Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-09 15:48 [PATCH v2 0/3] device property: Introduce device_is_compatible() Andy Shevchenko
  2023-06-09 15:48 ` [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h Andy Shevchenko
  2023-06-09 15:48 ` [PATCH v2 2/3] device property: Implement device_is_compatible() Andy Shevchenko
@ 2023-06-09 15:49 ` Andy Shevchenko
  2023-06-12  8:02   ` Sakari Ailus
                     ` (2 more replies)
  2 siblings, 3 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-09 15:49 UTC (permalink / raw)
  To: Damien Le Moal, Serge Semin, Andy Shevchenko, Greg Kroah-Hartman,
	linux-ide, linux-kernel, linux-acpi
  Cc: Hans de Goede, Jens Axboe, Rafael J. Wysocki, Len Brown,
	Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
  2023-06-09 15:48 ` [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h Andy Shevchenko
@ 2023-06-09 17:32   ` Rafael J. Wysocki
  2023-06-12 15:21     ` Andy Shevchenko
  2023-06-12 22:07     ` Damien Le Moal
  0 siblings, 2 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2023-06-09 17:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus,
	Sakari Ailus

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
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-09 15:48 ` [PATCH v2 2/3] device property: Implement device_is_compatible() Andy Shevchenko
@ 2023-06-12  7:57   ` Sakari Ailus
  2023-06-13  9:45   ` Serge Semin
  1 sibling, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-12  7:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
@ 2023-06-12  8:02   ` Sakari Ailus
  2023-06-12 15:20     ` Andy Shevchenko
  2023-06-12  9:06   ` Sakari Ailus
  2023-06-13  9:52   ` Serge Semin
  2 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-06-12  8:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
  2023-06-12  8:02   ` Sakari Ailus
@ 2023-06-12  9:06   ` Sakari Ailus
  2023-06-12 15:19     ` Andy Shevchenko
  2023-06-13  9:52   ` Serge Semin
  2 siblings, 1 reply; 19+ messages in thread
From: Sakari Ailus @ 2023-06-12  9:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-12  9:06   ` Sakari Ailus
@ 2023-06-12 15:19     ` Andy Shevchenko
  2023-06-13  9:27       ` Sakari Ailus
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-12 15:19 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-12  8:02   ` Sakari Ailus
@ 2023-06-12 15:20     ` Andy Shevchenko
  2023-06-13  9:26       ` Sakari Ailus
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-12 15:20 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
  2023-06-09 17:32   ` Rafael J. Wysocki
@ 2023-06-12 15:21     ` Andy Shevchenko
  2023-06-12 22:07     ` Damien Le Moal
  1 sibling, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-12 15:21 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe, Len Brown,
	Daniel Scally, Heikki Krogerus, Sakari Ailus

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h
  2023-06-09 17:32   ` Rafael J. Wysocki
  2023-06-12 15:21     ` Andy Shevchenko
@ 2023-06-12 22:07     ` Damien Le Moal
  1 sibling, 0 replies; 19+ messages in thread
From: Damien Le Moal @ 2023-06-12 22:07 UTC (permalink / raw)
  To: Rafael J. Wysocki, Andy Shevchenko
  Cc: Serge Semin, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Len Brown, Daniel Scally,
	Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-12 15:20     ` Andy Shevchenko
@ 2023-06-13  9:26       ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-13  9:26 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-12 15:19     ` Andy Shevchenko
@ 2023-06-13  9:27       ` Sakari Ailus
  0 siblings, 0 replies; 19+ messages in thread
From: Sakari Ailus @ 2023-06-13  9:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Serge Semin, Greg Kroah-Hartman, linux-ide,
	linux-kernel, linux-acpi, Hans de Goede, Jens Axboe,
	Rafael J. Wysocki, Len Brown, Daniel Scally, Heikki Krogerus

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-09 15:48 ` [PATCH v2 2/3] device property: Implement device_is_compatible() Andy Shevchenko
  2023-06-12  7:57   ` Sakari Ailus
@ 2023-06-13  9:45   ` Serge Semin
  2023-06-13 15:14     ` Andy Shevchenko
  1 sibling, 1 reply; 19+ messages in thread
From: Serge Semin @ 2023-06-13  9:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Rafael J. Wysocki,
	Len Brown, Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI
  2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
  2023-06-12  8:02   ` Sakari Ailus
  2023-06-12  9:06   ` Sakari Ailus
@ 2023-06-13  9:52   ` Serge Semin
  2 siblings, 0 replies; 19+ messages in thread
From: Serge Semin @ 2023-06-13  9:52 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Rafael J. Wysocki,
	Len Brown, Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-13  9:45   ` Serge Semin
@ 2023-06-13 15:14     ` Andy Shevchenko
  2023-06-13 15:14       ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-13 15:14 UTC (permalink / raw)
  To: Serge Semin
  Cc: Damien Le Moal, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Rafael J. Wysocki,
	Len Brown, Daniel Scally, Heikki Krogerus, Sakari Ailus

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-13 15:14     ` Andy Shevchenko
@ 2023-06-13 15:14       ` Andy Shevchenko
  2023-06-13 19:23         ` Serge Semin
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-06-13 15:14 UTC (permalink / raw)
  To: Serge Semin
  Cc: Damien Le Moal, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Rafael J. Wysocki,
	Len Brown, Daniel Scally, Heikki Krogerus, Sakari Ailus

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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/3] device property: Implement device_is_compatible()
  2023-06-13 15:14       ` Andy Shevchenko
@ 2023-06-13 19:23         ` Serge Semin
  0 siblings, 0 replies; 19+ messages in thread
From: Serge Semin @ 2023-06-13 19:23 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Damien Le Moal, Greg Kroah-Hartman, linux-ide, linux-kernel,
	linux-acpi, Hans de Goede, Jens Axboe, Rafael J. Wysocki,
	Len Brown, Daniel Scally, Heikki Krogerus, Sakari Ailus

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-06-13 19:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 15:48 [PATCH v2 0/3] device property: Introduce device_is_compatible() Andy Shevchenko
2023-06-09 15:48 ` [PATCH v2 1/3] ACPI: Move ACPI_DEVICE_CLASS() to mod_devicetable.h Andy Shevchenko
2023-06-09 17:32   ` Rafael J. Wysocki
2023-06-12 15:21     ` Andy Shevchenko
2023-06-12 22:07     ` Damien Le Moal
2023-06-09 15:48 ` [PATCH v2 2/3] device property: Implement device_is_compatible() Andy Shevchenko
2023-06-12  7:57   ` Sakari Ailus
2023-06-13  9:45   ` Serge Semin
2023-06-13 15:14     ` Andy Shevchenko
2023-06-13 15:14       ` Andy Shevchenko
2023-06-13 19:23         ` Serge Semin
2023-06-09 15:49 ` [PATCH v2 3/3] ata: ahci_platform: Make code agnostic to OF/ACPI Andy Shevchenko
2023-06-12  8:02   ` Sakari Ailus
2023-06-12 15:20     ` Andy Shevchenko
2023-06-13  9:26       ` Sakari Ailus
2023-06-12  9:06   ` Sakari Ailus
2023-06-12 15:19     ` Andy Shevchenko
2023-06-13  9:27       ` Sakari Ailus
2023-06-13  9:52   ` Serge Semin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).