Hi Peter,

-----Original Message-----
From: Peter Rosin <redacted>
Sent: Tuesday, August 23, 2022 2:13 PM
To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com;
robh+dt@kernel.org; shawnguo@kernel.org
Cc: gregkh@linuxfoundation.org; linux@roeck-us.net; Jun Li
[off-list ref]; linux-usb@vger.kernel.org; dl-linux-imx <linux-
imx@nxp.com>; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org
Subject: [EXT] Re: [PATCH 3/4] usb: typec: mux: add typec orientation switch
support via mux controller

Caution: EXT Email

Hi!

2022-08-22 at 17:35, Xu Yang wrote:

quoted

Some dedicated mux block can use existing mux controller as a mux
provider, typec port as a consumer to select channel for orientation
switch, this can be an alternate way to control typec orientation switch.
Also, one mux controller could cover highspeed, superspeed and sideband
use case one time in this implementation.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/mux.c       | 74

+++++++++++++++++++++++++++++++++++

quoted

 include/linux/usb/typec_mux.h |  7 +---
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 464330776cd6..5ee960fb668d 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c

@@ -13,6 +13,7 @@
 #include <linux/mutex.h>
 #include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/mux/consumer.h>

 #include "class.h"
 #include "mux.h"

@@ -22,6 +23,11 @@
 struct typec_switch {
      struct typec_switch_dev *sw_devs[TYPEC_MUX_MAX_DEVS];
      unsigned int num_sw_devs;
+
+     /* Could handle HighSpeed, SuperSpeed, Sideband switch one time */
+     struct mux_control *mux_switch;
+     /* 3 state correspond to NONE, NORMAL, REVERSE for all switches */
+     int mux_states[3];
 };

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

@@ -117,6 +123,58 @@ struct typec_switch

*fwnode_typec_switch_get(struct fwnode_handle *fwnode)

quoted

 }
 EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);

+static struct typec_switch *mux_control_typec_switch_get(struct device

*dev)

quoted

+{
+     struct typec_switch *sw;
+     struct mux_control *mux;
+     int ret;
+
+     if (!device_property_present(dev, "mux-controls"))
+             return NULL;
+
+     sw = kzalloc(sizeof(*sw), GFP_KERNEL);
+     if (!sw)
+             return ERR_PTR(-ENOMEM);
+
+     mux = mux_control_get(dev, NULL);
+     if (!IS_ERR(mux)) {
+             sw->mux_switch = mux;
+             ret = device_property_read_u32_array(dev,
+                     "typec-switch-states", sw->mux_states, 3);
+             if (ret) {
+                     kfree(sw);
+                     return ERR_PTR(ret);
+             }
+     } else {
+             kfree(sw);
+             return ERR_CAST(mux);
+     }
+
+     return sw;
+}
+
+/**
+ * typec_switch_get - Find USB Type-C orientation switch
+ * @dev: The device using switch
+ *
+ * Finds a switch used by @dev. Returns a reference to the switch on
+ * success, NULL if no matching connection was found, or
+ * ERR_PTR(-EPROBE_DEFER) when a connection was found but the

switch

quoted

+ * has not been enumerated yet, or ERR_PTR with a negative errno.
+ */
+struct typec_switch *typec_switch_get(struct device *dev)
+{
+     struct typec_switch *sw;
+
+     sw = fwnode_typec_switch_get(dev_fwnode(dev));
+     if (!sw)
+             /* Try get switch based on mux control */
+             sw = mux_control_typec_switch_get(dev);
+
+     return sw;
+}
+EXPORT_SYMBOL_GPL(typec_switch_get);
+
 /**
  * typec_switch_put - Release USB Type-C orientation switch
  * @sw: USB Type-C orientation switch

@@ -137,6 +195,10 @@ void typec_switch_put(struct typec_switch *sw)
              module_put(sw_dev->dev.parent->driver->owner);
              put_device(&sw_dev->dev);
      }
+
+     if (sw->mux_switch)
+             mux_control_put(sw->mux_switch);
+
      kfree(sw);
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);

@@ -204,6 +266,7 @@ int typec_switch_set(struct typec_switch *sw,
                   enum typec_orientation orientation)
 {
      struct typec_switch_dev *sw_dev;
+     struct mux_control *mux;
      unsigned int i;
      int ret;

@@ -218,6 +281,17 @@ int typec_switch_set(struct typec_switch *sw,
                      return ret;
      }

+     mux = sw->mux_switch;
+     if (mux) {
+             ret = mux_control_deselect(mux);

This is broken. Please read the documentation for mux_control_select and
mux_control_deselect. Every call to mux_control_deselect *must* be paired
with a *successful* call to mux_control_select. Here, mux_control_deselect
is called unconditionally (as long as a mux is configured).

Okay, I will improve it in v2.

Yes, agreed, that is indeed awkward (and fragile). But those are the rules.
(the mux interface was not designed for long-time selects like this)

Understood. I'll follow the rules. 

Thanks,
Xu Yang

Cheers,
Peter

quoted

+             if (ret)
+                     return ret;
+
+             ret = mux_control_select(mux, sw->mux_states[orientation]);
+             if (ret)
+                     return ret;
+     }
+
      return 0;
 }
 EXPORT_SYMBOL_GPL(typec_switch_set);

diff --git a/include/linux/usb/typec_mux.h

b/include/linux/usb/typec_mux.h

quoted

index 9292f0e07846..2287e5a5f591 100644

--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h

@@ -24,16 +24,13 @@ struct typec_switch_desc {
      void *drvdata;
 };

+
+struct typec_switch *typec_switch_get(struct device *dev);
 struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle

*fwnode);

quoted

 void typec_switch_put(struct typec_switch *sw);
 int typec_switch_set(struct typec_switch *sw,
                   enum typec_orientation orientation);

-static inline struct typec_switch *typec_switch_get(struct device *dev)
-{
-     return fwnode_typec_switch_get(dev_fwnode(dev));
-}
-
 struct typec_switch_dev *
 typec_switch_register(struct device *parent,
                    const struct typec_switch_desc *desc);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel