mbox series

[0/2] usb: typec: mux: a few improvements

Message ID 20210526153548.61276-1-heikki.krogerus@linux.intel.com
Headers show
Series usb: typec: mux: a few improvements | expand

Message

Heikki Krogerus May 26, 2021, 3:35 p.m. UTC
Hi,

The first patch should be trivial cleanup, but in the second one I'm
removing the condition that in practice forces the orientation switch
to always have the device property named "orientation-switch". This
change only impacts connections described with device graph since only
in that case the callback function is supplied with the "id" parameter
which is the first part of the condition (if the id is not supplied,
the function also does not expect the device property to exist).

But when the connection between the connector and the switch is
described with device graph, I don't see any need for that device
property. Therefore let's just remove the condition and the
requirement for the device property with it.

thanks,

Heikki Krogerus (2):
  usb: typec: mux: Use device type instead of device name for matching
  usb: typec: mux: Remove requirement for the "orientation-switch"
    device property

 drivers/usb/typec/mux.c | 29 ++++++++++-------------------
 drivers/usb/typec/mux.h |  6 ++++++
 2 files changed, 16 insertions(+), 19 deletions(-)

Comments

Heikki Krogerus May 31, 2021, 7:30 a.m. UTC | #1
On Wed, May 26, 2021 at 06:35:47PM +0300, Heikki Krogerus wrote:
> Both the USB Type-C switch and mux have already a device

> type defined for them. We can use those types instead of the

> device name to differentiate the two.


This should still be OK, right?

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---

>  drivers/usb/typec/mux.c | 26 ++++++++++----------------

>  drivers/usb/typec/mux.h |  6 ++++++

>  2 files changed, 16 insertions(+), 16 deletions(-)

> 

> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c

> index 8514bec7e1b89..e40a555724fb6 100644

> --- a/drivers/usb/typec/mux.c

> +++ b/drivers/usb/typec/mux.c

> @@ -17,21 +17,12 @@

>  #include "class.h"

>  #include "mux.h"

>  

> -static bool dev_name_ends_with(struct device *dev, const char *suffix)

> -{

> -	const char *name = dev_name(dev);

> -	const int name_len = strlen(name);

> -	const int suffix_len = strlen(suffix);

> -

> -	if (suffix_len > name_len)

> -		return false;

> -

> -	return strcmp(name + (name_len - suffix_len), suffix) == 0;

> -}

> -

>  static int switch_fwnode_match(struct device *dev, const void *fwnode)

>  {

> -	return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch");

> +	if (!is_typec_switch(dev))

> +		return 0;

> +

> +	return dev_fwnode(dev) == fwnode;

>  }

>  

>  static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id,

> @@ -90,7 +81,7 @@ static void typec_switch_release(struct device *dev)

>  	kfree(to_typec_switch(dev));

>  }

>  

> -static const struct device_type typec_switch_dev_type = {

> +const struct device_type typec_switch_dev_type = {

>  	.name = "orientation_switch",

>  	.release = typec_switch_release,

>  };

> @@ -180,7 +171,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get_drvdata);

>  

>  static int mux_fwnode_match(struct device *dev, const void *fwnode)

>  {

> -	return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux");

> +	if (!is_typec_mux(dev))

> +		return 0;

> +

> +	return dev_fwnode(dev) == fwnode;

>  }

>  

>  static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id,

> @@ -295,7 +289,7 @@ static void typec_mux_release(struct device *dev)

>  	kfree(to_typec_mux(dev));

>  }

>  

> -static const struct device_type typec_mux_dev_type = {

> +const struct device_type typec_mux_dev_type = {

>  	.name = "mode_switch",

>  	.release = typec_mux_release,

>  };

> diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h

> index 4fd9426ee44f6..b1d6e837cb747 100644

> --- a/drivers/usb/typec/mux.h

> +++ b/drivers/usb/typec/mux.h

> @@ -18,4 +18,10 @@ struct typec_mux {

>  #define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev)

>  #define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev)

>  

> +extern const struct device_type typec_switch_dev_type;

> +extern const struct device_type typec_mux_dev_type;

> +

> +#define is_typec_switch(dev) ((dev)->type == &typec_switch_dev_type)

> +#define is_typec_mux(dev) ((dev)->type == &typec_mux_dev_type)

> +

>  #endif /* __USB_TYPEC_MUX__ */

> -- 

> 2.30.2


-- 
heikki
Hans de Goede May 31, 2021, 8:09 a.m. UTC | #2
Hi,

On 5/31/21 9:30 AM, Heikki Krogerus wrote:
> On Wed, May 26, 2021 at 06:35:47PM +0300, Heikki Krogerus wrote:

>> Both the USB Type-C switch and mux have already a device

>> type defined for them. We can use those types instead of the

>> device name to differentiate the two.

> 

> This should still be OK, right?


I believe so and a dev_type check also seems more robust then
a name-suffix check.

Regards,

Hans


> 

>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

>> ---

>>  drivers/usb/typec/mux.c | 26 ++++++++++----------------

>>  drivers/usb/typec/mux.h |  6 ++++++

>>  2 files changed, 16 insertions(+), 16 deletions(-)

>>

>> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c

>> index 8514bec7e1b89..e40a555724fb6 100644

>> --- a/drivers/usb/typec/mux.c

>> +++ b/drivers/usb/typec/mux.c

>> @@ -17,21 +17,12 @@

>>  #include "class.h"

>>  #include "mux.h"

>>  

>> -static bool dev_name_ends_with(struct device *dev, const char *suffix)

>> -{

>> -	const char *name = dev_name(dev);

>> -	const int name_len = strlen(name);

>> -	const int suffix_len = strlen(suffix);

>> -

>> -	if (suffix_len > name_len)

>> -		return false;

>> -

>> -	return strcmp(name + (name_len - suffix_len), suffix) == 0;

>> -}

>> -

>>  static int switch_fwnode_match(struct device *dev, const void *fwnode)

>>  {

>> -	return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch");

>> +	if (!is_typec_switch(dev))

>> +		return 0;

>> +

>> +	return dev_fwnode(dev) == fwnode;

>>  }

>>  

>>  static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id,

>> @@ -90,7 +81,7 @@ static void typec_switch_release(struct device *dev)

>>  	kfree(to_typec_switch(dev));

>>  }

>>  

>> -static const struct device_type typec_switch_dev_type = {

>> +const struct device_type typec_switch_dev_type = {

>>  	.name = "orientation_switch",

>>  	.release = typec_switch_release,

>>  };

>> @@ -180,7 +171,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get_drvdata);

>>  

>>  static int mux_fwnode_match(struct device *dev, const void *fwnode)

>>  {

>> -	return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux");

>> +	if (!is_typec_mux(dev))

>> +		return 0;

>> +

>> +	return dev_fwnode(dev) == fwnode;

>>  }

>>  

>>  static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id,

>> @@ -295,7 +289,7 @@ static void typec_mux_release(struct device *dev)

>>  	kfree(to_typec_mux(dev));

>>  }

>>  

>> -static const struct device_type typec_mux_dev_type = {

>> +const struct device_type typec_mux_dev_type = {

>>  	.name = "mode_switch",

>>  	.release = typec_mux_release,

>>  };

>> diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h

>> index 4fd9426ee44f6..b1d6e837cb747 100644

>> --- a/drivers/usb/typec/mux.h

>> +++ b/drivers/usb/typec/mux.h

>> @@ -18,4 +18,10 @@ struct typec_mux {

>>  #define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev)

>>  #define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev)

>>  

>> +extern const struct device_type typec_switch_dev_type;

>> +extern const struct device_type typec_mux_dev_type;

>> +

>> +#define is_typec_switch(dev) ((dev)->type == &typec_switch_dev_type)

>> +#define is_typec_mux(dev) ((dev)->type == &typec_mux_dev_type)

>> +

>>  #endif /* __USB_TYPEC_MUX__ */

>> -- 

>> 2.30.2

>