2023-05-24 12:34:31

by Heikki Krogerus

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

Hi,

The background for this change:
https://lore.kernel.org/lkml/[email protected]/

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



2023-05-24 12:42:52

by Heikki Krogerus

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

The alt mode descriptor parameters are not used anymore.

Signed-off-by: Heikki Krogerus <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Bjorn Andersson <[email protected]>
---
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


2023-05-24 12:48:27

by Heikki Krogerus

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

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

Signed-off-by: Heikki Krogerus <[email protected]>
Link: https://lore.kernel.org/lkml/20230522215348.uoyboow26n2o3tel@ripper/
Cc: Bjorn Andersson <[email protected]>
---
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


2023-05-24 17:55:27

by Prashant Malani

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

Hi Heikki,

On Wed, May 24, 2023 at 5:29 AM Heikki Krogerus
<[email protected]> wrote:
>
> The alt mode descriptor parameters are not used anymore.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> Cc: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Bjorn Andersson <[email protected]>

For cros-ec-typec:
Acked-by: Prashant Malani <[email protected]>

Thanks,

-Prashant

2023-05-25 03:34:30

by Bjorn Andersson

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

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 <[email protected]>
> Link: https://lore.kernel.org/lkml/20230522215348.uoyboow26n2o3tel@ripper/
> Cc: Bjorn Andersson <[email protected]>
> ---
> 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 <[email protected]>
Tested-by: Bjorn Andersson <[email protected]>

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
>

2023-05-25 03:36:12

by Bjorn Andersson

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

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 <[email protected]>
> Cc: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Bjorn Andersson <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>
Tested-by: Bjorn Andersson <[email protected]> #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
>