[PATCH v1 0/2] usb: typec: mux: Remove the "svid" device property checks

[PATCH v1 0/2] usb: typec: mux: Remove the "svid" device property checks

[Heikki Krogerus]

Hi,

The background for this change:
https://lore.kernel.org/lkml/20210726231351.655302-1-bjorn.andersson@linaro.org/

The idea with that device property was that it would allow us to
support separate mode specific switches behind a single port if
necessary.

Although, I guess it is still possible that we could have that kind of
separate mode switches, especially now that the mode switch represents
a kind of virtual aggregate device that can be build from multiple
physical muxes (so the modes could need different "combos" of the
muxes), but now no such systems exist.

We can look at how to handle that kind of switches when/if we actually
have them, but for now, keeping the functions as simple as possible.

thanks,

Heikki Krogerus (2):
  usb: typec: mux: Clean up mux_fwnode_match()
  usb: typec: mux: Remove alt mode parameters from the API

 drivers/platform/chrome/cros_ec_typec.c |  2 +-
 drivers/soc/qcom/pmic_glink_altmode.c   |  5 +-
 drivers/usb/typec/class.c               |  4 +-
 drivers/usb/typec/mux.c                 | 61 ++++---------------------
 include/linux/usb/typec_mux.h           | 11 ++---
 5 files changed, 17 insertions(+), 66 deletions(-)

-- 
2.39.2

[PATCH v1 1/2] usb: typec: mux: Clean up mux_fwnode_match()

[Heikki Krogerus]

Removing the "svid" and "accessory" device property checks.
Those properties are not supported on any platform.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/lkml/20230522215348.uoyboow26n2o3tel@ripper/
Cc: Bjorn Andersson <andersson@kernel.org>
---
 drivers/usb/typec/mux.c | 55 ++++++-----------------------------------
 1 file changed, 7 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index d9eaf9a0b0bfd..089c2fd478318 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -265,60 +265,19 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode)
 static void *typec_mux_match(const struct fwnode_handle *fwnode,
 			     const char *id, void *data)
 {
-	const struct typec_altmode_desc *desc = data;
 	struct device *dev;
-	bool match;
-	int nval;
-	u16 *val;
-	int ret;
-	int i;
 
 	/*
-	 * Check has the identifier already been "consumed". If it
-	 * has, no need to do any extra connection identification.
+	 * Device graph (OF graph) does not give any means to identify the
+	 * device type or the device class of the remote port parent that @fwnode
+	 * represents, so in order to identify the type or the class of @fwnode
+	 * an additional device property is needed. With typec switches the
+	 * property is named "orientation-switch" (@id). The value of the device
+	 * property is ignored.
 	 */
-	match = !id;
-	if (match)
-		goto find_mux;
-
-	if (!desc) {
-		/*
-		 * Accessory Mode muxes & muxes which explicitly specify
-		 * the required identifier can avoid SVID matching.
-		 */
-		match = fwnode_property_present(fwnode, "accessory") ||
-			fwnode_property_present(fwnode, id);
-		if (match)
-			goto find_mux;
-		return NULL;
-	}
-
-	/* Alternate Mode muxes */
-	nval = fwnode_property_count_u16(fwnode, "svid");
-	if (nval <= 0)
+	if (id && !fwnode_property_present(fwnode, id))
 		return NULL;
 
-	val = kcalloc(nval, sizeof(*val), GFP_KERNEL);
-	if (!val)
-		return ERR_PTR(-ENOMEM);
-
-	ret = fwnode_property_read_u16_array(fwnode, "svid", val, nval);
-	if (ret < 0) {
-		kfree(val);
-		return ERR_PTR(ret);
-	}
-
-	for (i = 0; i < nval; i++) {
-		match = val[i] == desc->svid;
-		if (match) {
-			kfree(val);
-			goto find_mux;
-		}
-	}
-	kfree(val);
-	return NULL;
-
-find_mux:
 	dev = class_find_device(&typec_mux_class, NULL, fwnode,
 				mux_fwnode_match);
 
-- 
2.39.2

[PATCH v1 2/2] usb: typec: mux: Remove alt mode parameters from the API

[Heikki Krogerus]

The alt mode descriptor parameters are not used anymore.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Bjorn Andersson <andersson@kernel.org>
---
 drivers/platform/chrome/cros_ec_typec.c |  2 +-
 drivers/soc/qcom/pmic_glink_altmode.c   |  5 +----
 drivers/usb/typec/class.c               |  4 ++--
 drivers/usb/typec/mux.c                 |  6 ++----
 include/linux/usb/typec_mux.h           | 11 ++++-------
 5 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index a673c33424706..25f9767c28e82 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -77,7 +77,7 @@ static int cros_typec_get_switch_handles(struct cros_typec_port *port,
 {
 	int ret = 0;
 
-	port->mux = fwnode_typec_mux_get(fwnode, NULL);
+	port->mux = fwnode_typec_mux_get(fwnode);
 	if (IS_ERR(port->mux)) {
 		ret = PTR_ERR(port->mux);
 		dev_dbg(dev, "Mux handle not found: %d.\n", ret);
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
index 4d7895bdeaf2f..df48fbea4b686 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -369,7 +369,6 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
 {
 	struct pmic_glink_altmode_port *alt_port;
 	struct pmic_glink_altmode *altmode;
-	struct typec_altmode_desc mux_desc = {};
 	const struct of_device_id *match;
 	struct fwnode_handle *fwnode;
 	struct device *dev = &adev->dev;
@@ -427,9 +426,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
 		alt_port->dp_alt.mode = USB_TYPEC_DP_MODE;
 		alt_port->dp_alt.active = 1;
 
-		mux_desc.svid = USB_TYPEC_DP_SID;
-		mux_desc.mode = USB_TYPEC_DP_MODE;
-		alt_port->typec_mux = fwnode_typec_mux_get(fwnode, &mux_desc);
+		alt_port->typec_mux = fwnode_typec_mux_get(fwnode);
 		if (IS_ERR(alt_port->typec_mux))
 			return dev_err_probe(dev, PTR_ERR(alt_port->typec_mux),
 					     "failed to acquire mode-switch for port: %d\n",
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 349cc2030c903..faa184ae3dac8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -2110,7 +2110,7 @@ typec_port_register_altmode(struct typec_port *port,
 	struct typec_mux *mux;
 	struct typec_retimer *retimer;
 
-	mux = typec_mux_get(&port->dev, desc);
+	mux = typec_mux_get(&port->dev);
 	if (IS_ERR(mux))
 		return ERR_CAST(mux);
 
@@ -2274,7 +2274,7 @@ struct typec_port *typec_register_port(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
-	port->mux = typec_mux_get(&port->dev, NULL);
+	port->mux = typec_mux_get(&port->dev);
 	if (IS_ERR(port->mux)) {
 		ret = PTR_ERR(port->mux);
 		put_device(&port->dev);
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 089c2fd478318..a29945e2eb077 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -287,15 +287,13 @@ static void *typec_mux_match(const struct fwnode_handle *fwnode,
 /**
  * fwnode_typec_mux_get - Find USB Type-C Multiplexer
  * @fwnode: The caller device node
- * @desc: Alt Mode description
  *
  * Finds a mux linked to the caller. This function is primarily meant for the
  * Type-C drivers. Returns a reference to the mux on success, NULL if no
  * matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a connection
  * was found but the mux has not been enumerated yet.
  */
-struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
-				       const struct typec_altmode_desc *desc)
+struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode)
 {
 	struct typec_mux_dev *mux_devs[TYPEC_MUX_MAX_DEVS];
 	struct typec_mux *mux;
@@ -308,7 +306,7 @@ struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
 		return ERR_PTR(-ENOMEM);
 
 	count = fwnode_connection_find_matches(fwnode, "mode-switch",
-					       (void *)desc, typec_mux_match,
+					       NULL, typec_mux_match,
 					       (void **)mux_devs,
 					       ARRAY_SIZE(mux_devs));
 	if (count <= 0) {
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 9292f0e078464..11bfa314529fd 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -60,8 +60,7 @@ struct typec_mux_desc {
 
 #if IS_ENABLED(CONFIG_TYPEC)
 
-struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
-				       const struct typec_altmode_desc *desc);
+struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode);
 void typec_mux_put(struct typec_mux *mux);
 int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
 
@@ -74,8 +73,7 @@ void *typec_mux_get_drvdata(struct typec_mux_dev *mux);
 
 #else
 
-static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
-				       const struct typec_altmode_desc *desc)
+static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode);
 {
 	return NULL;
 }
@@ -102,10 +100,9 @@ static inline void *typec_mux_get_drvdata(struct typec_mux_dev *mux)
 
 #endif /* CONFIG_TYPEC */
 
-static inline struct typec_mux *
-typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
+static inline struct typec_mux *typec_mux_get(struct device *dev)
 {
-	return fwnode_typec_mux_get(dev_fwnode(dev), desc);
+	return fwnode_typec_mux_get(dev_fwnode(dev));
 }
 
 #endif /* __USB_TYPEC_MUX */
-- 
2.39.2

Re: [PATCH v1 2/2] usb: typec: mux: Remove alt mode parameters from the API

[Prashant Malani]

Hi Heikki,

On Wed, May 24, 2023 at 5:29 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> The alt mode descriptor parameters are not used anymore.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Prashant Malani <pmalani@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Bjorn Andersson <andersson@kernel.org>

For cros-ec-typec:
Acked-by: Prashant Malani <pmalani@chromium.org>

Thanks,

-Prashant

Re: [PATCH v1 1/2] usb: typec: mux: Clean up mux_fwnode_match()

[Bjorn Andersson]

On Wed, May 24, 2023 at 03:29:00PM +0300, Heikki Krogerus wrote:
> Removing the "svid" and "accessory" device property checks.
> Those properties are not supported on any platform.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Link: https://lore.kernel.org/lkml/20230522215348.uoyboow26n2o3tel@ripper/
> Cc: Bjorn Andersson <andersson@kernel.org>
> ---
>  drivers/usb/typec/mux.c | 55 ++++++-----------------------------------
>  1 file changed, 7 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index d9eaf9a0b0bfd..089c2fd478318 100644
> --- a/drivers/usb/typec/mux.c
> +++ b/drivers/usb/typec/mux.c
> @@ -265,60 +265,19 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode)
>  static void *typec_mux_match(const struct fwnode_handle *fwnode,
>  			     const char *id, void *data)
>  {
> -	const struct typec_altmode_desc *desc = data;
>  	struct device *dev;
> -	bool match;
> -	int nval;
> -	u16 *val;
> -	int ret;
> -	int i;
>  
>  	/*
> -	 * Check has the identifier already been "consumed". If it
> -	 * has, no need to do any extra connection identification.
> +	 * Device graph (OF graph) does not give any means to identify the
> +	 * device type or the device class of the remote port parent that @fwnode
> +	 * represents, so in order to identify the type or the class of @fwnode
> +	 * an additional device property is needed. With typec switches the

s/switches/muxes/

> +	 * property is named "orientation-switch" (@id). The value of the device

s/orientation-switch/mode-switch/

With that:

Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Tested-by: Bjorn Andersson <andersson@kernel.org>

Thank you,
Bjorn

> +	 * property is ignored.
>  	 */
> -	match = !id;
> -	if (match)
> -		goto find_mux;
> -
> -	if (!desc) {
> -		/*
> -		 * Accessory Mode muxes & muxes which explicitly specify
> -		 * the required identifier can avoid SVID matching.
> -		 */
> -		match = fwnode_property_present(fwnode, "accessory") ||
> -			fwnode_property_present(fwnode, id);
> -		if (match)
> -			goto find_mux;
> -		return NULL;
> -	}
> -
> -	/* Alternate Mode muxes */
> -	nval = fwnode_property_count_u16(fwnode, "svid");
> -	if (nval <= 0)
> +	if (id && !fwnode_property_present(fwnode, id))
>  		return NULL;
>  
> -	val = kcalloc(nval, sizeof(*val), GFP_KERNEL);
> -	if (!val)
> -		return ERR_PTR(-ENOMEM);
> -
> -	ret = fwnode_property_read_u16_array(fwnode, "svid", val, nval);
> -	if (ret < 0) {
> -		kfree(val);
> -		return ERR_PTR(ret);
> -	}
> -
> -	for (i = 0; i < nval; i++) {
> -		match = val[i] == desc->svid;
> -		if (match) {
> -			kfree(val);
> -			goto find_mux;
> -		}
> -	}
> -	kfree(val);
> -	return NULL;
> -
> -find_mux:
>  	dev = class_find_device(&typec_mux_class, NULL, fwnode,
>  				mux_fwnode_match);
>  
> -- 
> 2.39.2
> 

Re: [PATCH v1 2/2] usb: typec: mux: Remove alt mode parameters from the API

[Bjorn Andersson]

On Wed, May 24, 2023 at 03:29:01PM +0300, Heikki Krogerus wrote:
> The alt mode descriptor parameters are not used anymore.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Prashant Malani <pmalani@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Bjorn Andersson <andersson@kernel.org>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Tested-by: Bjorn Andersson <andersson@kernel.org> #pmic_glink_altmode

Regards,
Bjorn

> ---
>  drivers/platform/chrome/cros_ec_typec.c |  2 +-
>  drivers/soc/qcom/pmic_glink_altmode.c   |  5 +----
>  drivers/usb/typec/class.c               |  4 ++--
>  drivers/usb/typec/mux.c                 |  6 ++----
>  include/linux/usb/typec_mux.h           | 11 ++++-------
>  5 files changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index a673c33424706..25f9767c28e82 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -77,7 +77,7 @@ static int cros_typec_get_switch_handles(struct cros_typec_port *port,
>  {
>  	int ret = 0;
>  
> -	port->mux = fwnode_typec_mux_get(fwnode, NULL);
> +	port->mux = fwnode_typec_mux_get(fwnode);
>  	if (IS_ERR(port->mux)) {
>  		ret = PTR_ERR(port->mux);
>  		dev_dbg(dev, "Mux handle not found: %d.\n", ret);
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> index 4d7895bdeaf2f..df48fbea4b686 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -369,7 +369,6 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
>  {
>  	struct pmic_glink_altmode_port *alt_port;
>  	struct pmic_glink_altmode *altmode;
> -	struct typec_altmode_desc mux_desc = {};
>  	const struct of_device_id *match;
>  	struct fwnode_handle *fwnode;
>  	struct device *dev = &adev->dev;
> @@ -427,9 +426,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
>  		alt_port->dp_alt.mode = USB_TYPEC_DP_MODE;
>  		alt_port->dp_alt.active = 1;
>  
> -		mux_desc.svid = USB_TYPEC_DP_SID;
> -		mux_desc.mode = USB_TYPEC_DP_MODE;
> -		alt_port->typec_mux = fwnode_typec_mux_get(fwnode, &mux_desc);
> +		alt_port->typec_mux = fwnode_typec_mux_get(fwnode);
>  		if (IS_ERR(alt_port->typec_mux))
>  			return dev_err_probe(dev, PTR_ERR(alt_port->typec_mux),
>  					     "failed to acquire mode-switch for port: %d\n",
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 349cc2030c903..faa184ae3dac8 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -2110,7 +2110,7 @@ typec_port_register_altmode(struct typec_port *port,
>  	struct typec_mux *mux;
>  	struct typec_retimer *retimer;
>  
> -	mux = typec_mux_get(&port->dev, desc);
> +	mux = typec_mux_get(&port->dev);
>  	if (IS_ERR(mux))
>  		return ERR_CAST(mux);
>  
> @@ -2274,7 +2274,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  		return ERR_PTR(ret);
>  	}
>  
> -	port->mux = typec_mux_get(&port->dev, NULL);
> +	port->mux = typec_mux_get(&port->dev);
>  	if (IS_ERR(port->mux)) {
>  		ret = PTR_ERR(port->mux);
>  		put_device(&port->dev);
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 089c2fd478318..a29945e2eb077 100644
> --- a/drivers/usb/typec/mux.c
> +++ b/drivers/usb/typec/mux.c
> @@ -287,15 +287,13 @@ static void *typec_mux_match(const struct fwnode_handle *fwnode,
>  /**
>   * fwnode_typec_mux_get - Find USB Type-C Multiplexer
>   * @fwnode: The caller device node
> - * @desc: Alt Mode description
>   *
>   * Finds a mux linked to the caller. This function is primarily meant for the
>   * Type-C drivers. Returns a reference to the mux on success, NULL if no
>   * matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a connection
>   * was found but the mux has not been enumerated yet.
>   */
> -struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> -				       const struct typec_altmode_desc *desc)
> +struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode)
>  {
>  	struct typec_mux_dev *mux_devs[TYPEC_MUX_MAX_DEVS];
>  	struct typec_mux *mux;
> @@ -308,7 +306,7 @@ struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
>  		return ERR_PTR(-ENOMEM);
>  
>  	count = fwnode_connection_find_matches(fwnode, "mode-switch",
> -					       (void *)desc, typec_mux_match,
> +					       NULL, typec_mux_match,
>  					       (void **)mux_devs,
>  					       ARRAY_SIZE(mux_devs));
>  	if (count <= 0) {
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> index 9292f0e078464..11bfa314529fd 100644
> --- a/include/linux/usb/typec_mux.h
> +++ b/include/linux/usb/typec_mux.h
> @@ -60,8 +60,7 @@ struct typec_mux_desc {
>  
>  #if IS_ENABLED(CONFIG_TYPEC)
>  
> -struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> -				       const struct typec_altmode_desc *desc);
> +struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode);
>  void typec_mux_put(struct typec_mux *mux);
>  int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
>  
> @@ -74,8 +73,7 @@ void *typec_mux_get_drvdata(struct typec_mux_dev *mux);
>  
>  #else
>  
> -static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> -				       const struct typec_altmode_desc *desc)
> +static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode);
>  {
>  	return NULL;
>  }
> @@ -102,10 +100,9 @@ static inline void *typec_mux_get_drvdata(struct typec_mux_dev *mux)
>  
>  #endif /* CONFIG_TYPEC */
>  
> -static inline struct typec_mux *
> -typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
> +static inline struct typec_mux *typec_mux_get(struct device *dev)
>  {
> -	return fwnode_typec_mux_get(dev_fwnode(dev), desc);
> +	return fwnode_typec_mux_get(dev_fwnode(dev));
>  }
>  
>  #endif /* __USB_TYPEC_MUX */
> -- 
> 2.39.2
>