Re: [PATCH v4 11/11] usb: typec: class: Remove both cable_match() and partner_match()

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Wed, 11 Dec 2024 08:08:13 +0800
Zijun Hu <zijun_hu@icloud.com> wrote:

> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> cable_match(), as matching function of device_find_child(), matches
> a device with device type @typec_cable_dev_type, and its task can be
> simplified by the recently introduced API device_match_type().
> 
> partner_match() is similar with cable_match() but with a different
> device type @typec_partner_dev_type.
> 
> Remove both functions and use the API plus respective device type instead.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
Looks good, but there is the same trade off here between internal
detail of type identification and reducing the use of helpers
where the generic ones are fine.  Here is less obvious even than
the CXL one as the helper macros do have other uses in these
files.

So, it's on for USB folk to decide on and I won't be giving a tag
as a result.

Jonathan

> ---
>  drivers/usb/typec/class.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 601a81aa1e1024265f2359393dee531a7779c6ea..3a4e0bd0131774afd0d746d2f0a306190219feec 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1282,11 +1282,6 @@ const struct device_type typec_cable_dev_type = {
>  	.release = typec_cable_release,
>  };
>  
> -static int cable_match(struct device *dev, const void *data)
> -{
> -	return is_typec_cable(dev);
> -}
> -
>  /**
>   * typec_cable_get - Get a reference to the USB Type-C cable
>   * @port: The USB Type-C Port the cable is connected to
> @@ -1298,7 +1293,8 @@ struct typec_cable *typec_cable_get(struct typec_port *port)
>  {
>  	struct device *dev;
>  
> -	dev = device_find_child(&port->dev, NULL, cable_match);
> +	dev = device_find_child(&port->dev, &typec_cable_dev_type,
> +				device_match_type);
>  	if (!dev)
>  		return NULL;
>  
> @@ -2028,16 +2024,12 @@ const struct device_type typec_port_dev_type = {
>  /* --------------------------------------- */
>  /* Driver callbacks to report role updates */
>  
> -static int partner_match(struct device *dev, const void *data)
> -{
> -	return is_typec_partner(dev);
> -}
> -
>  static struct typec_partner *typec_get_partner(struct typec_port *port)
>  {
>  	struct device *dev;
>  
> -	dev = device_find_child(&port->dev, NULL, partner_match);
> +	dev = device_find_child(&port->dev, &typec_partner_dev_type,
> +				device_match_type);
>  	if (!dev)
>  		return NULL;
>  
> @@ -2170,7 +2162,9 @@ void typec_set_pwr_opmode(struct typec_port *port,
>  	sysfs_notify(&port->dev.kobj, NULL, "power_operation_mode");
>  	kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
>  
> -	partner_dev = device_find_child(&port->dev, NULL, partner_match);
> +	partner_dev = device_find_child(&port->dev,
> +					&typec_partner_dev_type,
> +					device_match_type);
>  	if (partner_dev) {
>  		struct typec_partner *partner = to_typec_partner(partner_dev);
>  
> @@ -2334,7 +2328,9 @@ int typec_get_negotiated_svdm_version(struct typec_port *port)
>  	enum usb_pd_svdm_ver svdm_version;
>  	struct device *partner_dev;
>  
> -	partner_dev = device_find_child(&port->dev, NULL, partner_match);
> +	partner_dev = device_find_child(&port->dev,
> +					&typec_partner_dev_type,
> +					device_match_type);
>  	if (!partner_dev)
>  		return -ENODEV;
>  
> @@ -2361,7 +2357,8 @@ int typec_get_cable_svdm_version(struct typec_port *port)
>  	enum usb_pd_svdm_ver svdm_version;
>  	struct device *cable_dev;
>  
> -	cable_dev = device_find_child(&port->dev, NULL, cable_match);
> +	cable_dev = device_find_child(&port->dev, &typec_cable_dev_type,
> +				      device_match_type);
>  	if (!cable_dev)
>  		return -ENODEV;
>  
>