[PATCH 0/3] device property: fwnode_is_compatible() helper

[PATCH 0/3] device property: fwnode_is_compatible() helper

[Heikki Krogerus]

Hi,

Basically the helper is just a generic version of the function
of_device_is_compatible() that should work also with ACPI and
software nodes.

As the first user for the helper, I'm converting in this series the
USB role switch code to expect the "compatible" property to have the
value "usb-role-switch" instead of expecting a boolean property named
"usb-role-switch". I'm doing the same for the USB Type-C mux code.

thanks,

Heikki Krogerus (3):
  device property: Add fwnode_is_compatible() and device_is_compatible()
    helpers
  usb: roles: Use the "compatible" property instead of a boolean
    property
  usb: typec: mux: Use the "compatible" property instead of a boolean
    property

 drivers/base/property.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/usb/roles/class.c |  2 +-
 drivers/usb/typec/mux.c   |  8 ++------
 include/linux/property.h  |  3 +++
 4 files changed, 41 insertions(+), 7 deletions(-)

-- 
2.20.1

[PATCH 1/3] device property: Add fwnode_is_compatible() and device_is_compatible() helpers

[Heikki Krogerus]

Since there are also some ACPI platforms where the
"compatible" property is used, introducing a generic helper
function fwnode_is_compatible() that can be used with
DT, ACPI and swnodes, and a wrapper function
device_is_compatible() with it.

The function calls of_device_is_comaptible() with OF nodes,
and with ACPI and swnodes it matches the given string
against the "compatible" string property array.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/property.c  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/property.h |  3 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8b91ab380d14..af9f5aac5d02 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1006,3 +1006,38 @@ const void *device_get_match_data(struct device *dev)
 	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
 }
 EXPORT_SYMBOL_GPL(device_get_match_data);
+
+/**
+ * fwnode_is_compatible - Check does fwnode have the given compatible string
+ * @fwnode: fwnode with the "compatible" property
+ * @compat: The compatible string
+ *
+ * Match the compatible strings of @fwnode against @compat. Returns positive
+ * value on match, and 0 when no matching compatible string is found.
+ */
+int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat)
+{
+	int ret;
+
+	if (is_of_node(fwnode))
+		return of_device_is_compatible(to_of_node(fwnode), compat);
+
+	ret = fwnode_property_match_string(fwnode, "compatible", compat);
+
+	return ret < 0 ? 0 : 1;
+}
+EXPORT_SYMBOL_GPL(fwnode_is_compatible);
+
+/**
+ * device_is_compatible - Check does a device have the given compatible string
+ * @dev: Device with the "compatible" property
+ * @compat: The compatible string
+ *
+ * Match the compatible strings of @dev against @compat. Returns positive value
+ * on match, and 0 when no matching compatible string is found.
+ */
+int device_is_compatible(struct device *dev, const char *compat)
+{
+	return fwnode_is_compatible(dev_fwnode(dev), compat);
+}
+EXPORT_SYMBOL_GPL(device_is_compatible);
diff --git a/include/linux/property.h b/include/linux/property.h
index 65d3420dd5d1..d22788ec36cd 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -311,6 +311,9 @@ fwnode_graph_get_remote_node(const struct fwnode_handle *fwnode, u32 port,
 int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 				struct fwnode_endpoint *endpoint);
 
+int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat);
+int device_is_compatible(struct device *dev, const char *compat);
+
 /* -------------------------------------------------------------------------- */
 /* Software fwnode support - when HW description is incomplete or missing */
 
-- 
2.20.1

Re: [PATCH 1/3] device property: Add fwnode_is_compatible() and device_is_compatible() helpers

[Rafael J. Wysocki]

On Wednesday, March 27, 2019 5:43:37 PM CEST Heikki Krogerus wrote:
> Since there are also some ACPI platforms where the
> "compatible" property is used, introducing a generic helper
> function fwnode_is_compatible() that can be used with
> DT, ACPI and swnodes, and a wrapper function
> device_is_compatible() with it.
> 
> The function calls of_device_is_comaptible() with OF nodes,
> and with ACPI and swnodes it matches the given string
> against the "compatible" string property array.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/base/property.c  | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/property.h |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8b91ab380d14..af9f5aac5d02 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1006,3 +1006,38 @@ const void *device_get_match_data(struct device *dev)
>  	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
>  }
>  EXPORT_SYMBOL_GPL(device_get_match_data);
> +
> +/**
> + * fwnode_is_compatible - Check does fwnode have the given compatible string
> + * @fwnode: fwnode with the "compatible" property
> + * @compat: The compatible string
> + *
> + * Match the compatible strings of @fwnode against @compat. Returns positive
> + * value on match, and 0 when no matching compatible string is found.
> + */
> +int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat)
> +{
> +	int ret;
> +
> +	if (is_of_node(fwnode))
> +		return of_device_is_compatible(to_of_node(fwnode), compat);
> +
> +	ret = fwnode_property_match_string(fwnode, "compatible", compat);
> +
> +	return ret < 0 ? 0 : 1;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_is_compatible);
> +
> +/**
> + * device_is_compatible - Check does a device have the given compatible string
> + * @dev: Device with the "compatible" property
> + * @compat: The compatible string
> + *
> + * Match the compatible strings of @dev against @compat. Returns positive value
> + * on match, and 0 when no matching compatible string is found.
> + */
> +int device_is_compatible(struct device *dev, const char *compat)
> +{
> +	return fwnode_is_compatible(dev_fwnode(dev), compat);
> +}
> +EXPORT_SYMBOL_GPL(device_is_compatible);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 65d3420dd5d1..d22788ec36cd 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -311,6 +311,9 @@ fwnode_graph_get_remote_node(const struct fwnode_handle *fwnode, u32 port,
>  int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>  				struct fwnode_endpoint *endpoint);
>  
> +int fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compat);
> +int device_is_compatible(struct device *dev, const char *compat);
> +
>  /* -------------------------------------------------------------------------- */
>  /* Software fwnode support - when HW description is incomplete or missing */
>  
> 

The new helpers would not be used anywhere for the time being, so it is better to
add them along with the first user IMO.

Please resend when this is needed.

[PATCH 2/3] usb: roles: Use the "compatible" property instead of a boolean property

[Heikki Krogerus]

Instead of searching for a boolean property, matching
against the "compatible" property.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/roles/class.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index f45d8df5cfb8..3dc1e05959c1 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -101,7 +101,7 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
 	struct device *dev;
 
 	if (con->fwnode) {
-		if (!fwnode_property_present(con->fwnode, con->id))
+		if (!fwnode_is_compatible(con->fwnode, con->id))
 			return NULL;
 
 		dev = class_find_device(role_class, NULL, con->fwnode,
-- 
2.20.1

[PATCH 3/3] usb: typec: mux: Use the "compatible" property instead of a boolean property

[Heikki Krogerus]

Instead of searching for a boolean property, matching
against the "compatible" property.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/mux.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 2ce54f3fc79c..9462b90f1c09 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -32,11 +32,7 @@ static void *typec_switch_match(struct device_connection *con, int ep,
 		return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	/*
-	 * With OF graph the mux node must have a boolean device property named
-	 * "orientation-switch".
-	 */
-	if (con->id && !fwnode_property_present(con->fwnode, con->id))
+	if (con->id && !fwnode_is_compatible(con->fwnode, con->id))
 		return NULL;
 
 	list_for_each_entry(sw, &switch_list, entry)
@@ -148,7 +144,7 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
 
 	/* Accessory Mode muxes */
 	if (!desc) {
-		match = fwnode_property_present(con->fwnode, "accessory");
+		match = fwnode_is_compatible(con->fwnode, "accessory");
 		if (match)
 			goto find_mux;
 		return NULL;
-- 
2.20.1

Re: [PATCH 3/3] usb: typec: mux: Use the "compatible" property instead of a boolean property

[Jun Li]

Hi Heikki,

Heikki Krogerus <heikki.krogerus@linux.intel.com> 于2019年3月28日周四 上午12:45写道：
>
> Instead of searching for a boolean property, matching
> against the "compatible" property.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/mux.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 2ce54f3fc79c..9462b90f1c09 100644
> --- a/drivers/usb/typec/mux.c
> +++ b/drivers/usb/typec/mux.c
> @@ -32,11 +32,7 @@ static void *typec_switch_match(struct device_connection *con, int ep,
>                 return ERR_PTR(-EPROBE_DEFER);
>         }
>
> -       /*
> -        * With OF graph the mux node must have a boolean device property named
> -        * "orientation-switch".
> -        */
> -       if (con->id && !fwnode_property_present(con->fwnode, con->id))
> +       if (con->id && !fwnode_is_compatible(con->fwnode, con->id))

This is still the right approach for orientation switch match, right?

Li Jun
>                 return NULL;
>
>         list_for_each_entry(sw, &switch_list, entry)
> @@ -148,7 +144,7 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
>
>         /* Accessory Mode muxes */
>         if (!desc) {
> -               match = fwnode_property_present(con->fwnode, "accessory");
> +               match = fwnode_is_compatible(con->fwnode, "accessory");
>                 if (match)
>                         goto find_mux;
>                 return NULL;
> --
> 2.20.1
>

Re: [PATCH 0/3] device property: fwnode_is_compatible() helper

[Andy Shevchenko]

On Wed, Mar 27, 2019 at 6:43 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> Basically the helper is just a generic version of the function
> of_device_is_compatible() that should work also with ACPI and
> software nodes.
>
> As the first user for the helper, I'm converting in this series the
> USB role switch code to expect the "compatible" property to have the
> value "usb-role-switch" instead of expecting a boolean property named
> "usb-role-switch". I'm doing the same for the USB Type-C mux code.
>

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> thanks,
>
> Heikki Krogerus (3):
>   device property: Add fwnode_is_compatible() and device_is_compatible()
>     helpers
>   usb: roles: Use the "compatible" property instead of a boolean
>     property
>   usb: typec: mux: Use the "compatible" property instead of a boolean
>     property
>
>  drivers/base/property.c   | 35 +++++++++++++++++++++++++++++++++++
>  drivers/usb/roles/class.c |  2 +-
>  drivers/usb/typec/mux.c   |  8 ++------
>  include/linux/property.h  |  3 +++
>  4 files changed, 41 insertions(+), 7 deletions(-)
>
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] device property: fwnode_is_compatible() helper

[Heikki Krogerus]

On Wed, Mar 27, 2019 at 07:43:36PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> Basically the helper is just a generic version of the function
> of_device_is_compatible() that should work also with ACPI and
> software nodes.
> 
> As the first user for the helper, I'm converting in this series the
> USB role switch code to expect the "compatible" property to have the
> value "usb-role-switch" instead of expecting a boolean property named
> "usb-role-switch". I'm doing the same for the USB Type-C mux code.
> 
> thanks,
> 
> Heikki Krogerus (3):
>   device property: Add fwnode_is_compatible() and device_is_compatible()
>     helpers
>   usb: roles: Use the "compatible" property instead of a boolean
>     property
>   usb: typec: mux: Use the "compatible" property instead of a boolean
>     property
> 
>  drivers/base/property.c   | 35 +++++++++++++++++++++++++++++++++++
>  drivers/usb/roles/class.c |  2 +-
>  drivers/usb/typec/mux.c   |  8 ++------
>  include/linux/property.h  |  3 +++
>  4 files changed, 41 insertions(+), 7 deletions(-)

Don't take this series yet. We are having a discussion on whether this
is the correct approach with the USB mux or not in an other mail
thread. The helper itself (patch 1/3) I guess is fine though.


thanks,

-- 
heikki