[PATCH v2 0/4] typec orientation switch support via mux controller

[PATCH v2 0/4] typec orientation switch support via mux controller

[Xu Yang]

Since typec port support flip-ability, there may need various swithes to
set correct path for SuperSpeed or Sideband use cases. As a common way, mux
controller could complete such tasks in simple cases, such as GPIO-based
swich or reg-based switch. This implementation could be an alternate way to
control orientation switch.

Xu Yang (4):
  dt-bindings: connector: Add typec orientation switch properties
  mux: convert to use fwnode interface
  usb: typec: mux: add typec orientation switch support via mux
    controller
  arm64: dts: imx8mp-evk: add typec node

 .../bindings/connector/usb-connector.yaml     |  18 +++
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts  | 122 ++++++++++++++++++
 drivers/mux/core.c                            |  65 +++++-----
 drivers/usb/typec/Kconfig                     |   1 +
 drivers/usb/typec/mux.c                       |  76 ++++++++++-
 include/linux/usb/typec_mux.h                 |   7 +-
 6 files changed, 251 insertions(+), 38 deletions(-)

-- 
2.34.1


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

[PATCH v2 0/4] typec orientation switch support via mux controller

[Xu Yang]

Since typec port support flip-ability, there may need various swithes to
set correct path for SuperSpeed or Sideband use cases. As a common way, mux
controller could complete such tasks in simple cases, such as GPIO-based
swich or reg-based switch. This implementation could be an alternate way to
control orientation switch.

Xu Yang (4):
  dt-bindings: connector: Add typec orientation switch properties
  mux: convert to use fwnode interface
  usb: typec: mux: add typec orientation switch support via mux
    controller
  arm64: dts: imx8mp-evk: add typec node

 .../bindings/connector/usb-connector.yaml     |  18 +++
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts  | 122 ++++++++++++++++++
 drivers/mux/core.c                            |  65 +++++-----
 drivers/usb/typec/Kconfig                     |   1 +
 drivers/usb/typec/mux.c                       |  76 ++++++++++-
 include/linux/usb/typec_mux.h                 |   7 +-
 6 files changed, 251 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] dt-bindings: connector: Add typec orientation switch properties

[Xu Yang]

Typec orientation switch can be implemented as a consumer of mux
controller. So we can use mux controller to control simple gpio switch
or other types of switch. This will cover the following typec switch
use case: High Speed, Super Speed and Sideband switch.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- No changes.

 .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index ae515651fc6b..47f53cdbf31a 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -221,6 +221,24 @@ properties:
       SNK_READY for non-pd link.
     type: boolean
 
+  # The following are optional properties for "usb-c-connector".
+  mux-controls:
+    description: Mux controller node to use for orientation switch selection. This mux controller
+      could handle High Speed, Super Speed and Sideband switch use case one time. In orde to do so,
+      besides mux settings need to be properly configured for each switch under mux-controller node,
+      correct states should also be assigned to typec-switch-states parameter.
+    maxItems: 1
+
+  typec-switch-states:
+    description: An ordered u32 array describing the mux state value for each typec orientations.
+      Three states correspond to NONE(high impedance), NORMAL, REVERSE respectively. If there is
+      no HW mux state for NONE, use value of NORMAL or REVERSE for it. If this mux controller
+      handle more than 1 switch, correct states value need to be caculated according to the mux
+      settings.
+    minItems: 3
+    maxItems: 3
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 dependencies:
   sink-vdos-v1: [ 'sink-vdos' ]
   sink-vdos: [ 'sink-vdos-v1' ]
-- 
2.34.1


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

[PATCH v2 1/4] dt-bindings: connector: Add typec orientation switch properties

[Xu Yang]

Typec orientation switch can be implemented as a consumer of mux
controller. So we can use mux controller to control simple gpio switch
or other types of switch. This will cover the following typec switch
use case: High Speed, Super Speed and Sideband switch.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- No changes.

 .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index ae515651fc6b..47f53cdbf31a 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -221,6 +221,24 @@ properties:
       SNK_READY for non-pd link.
     type: boolean
 
+  # The following are optional properties for "usb-c-connector".
+  mux-controls:
+    description: Mux controller node to use for orientation switch selection. This mux controller
+      could handle High Speed, Super Speed and Sideband switch use case one time. In orde to do so,
+      besides mux settings need to be properly configured for each switch under mux-controller node,
+      correct states should also be assigned to typec-switch-states parameter.
+    maxItems: 1
+
+  typec-switch-states:
+    description: An ordered u32 array describing the mux state value for each typec orientations.
+      Three states correspond to NONE(high impedance), NORMAL, REVERSE respectively. If there is
+      no HW mux state for NONE, use value of NORMAL or REVERSE for it. If this mux controller
+      handle more than 1 switch, correct states value need to be caculated according to the mux
+      settings.
+    minItems: 3
+    maxItems: 3
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 dependencies:
   sink-vdos-v1: [ 'sink-vdos' ]
   sink-vdos: [ 'sink-vdos-v1' ]
-- 
2.34.1

[PATCH v2 2/4] mux: convert to use fwnode interface

[Xu Yang]

As firmware node is a more common abstract, this will convert the whole
thing to fwnode interface.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- convert to use fwnode interface

 drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 49bedbe6316c..e30e859efd33 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -18,8 +18,7 @@
 #include <linux/module.h>
 #include <linux/mux/consumer.h>
 #include <linux/mux/driver.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 /*
@@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
 EXPORT_SYMBOL_GPL(mux_state_deselect);
 
 /* Note this function returns a reference to the mux_chip dev. */
-static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
+static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)
 {
 	struct device *dev;
 
-	dev = class_find_device_by_of_node(&mux_class, np);
+	dev = class_find_device_by_fwnode(&mux_class, fwnode);
 
 	return dev ? to_mux_chip(dev) : NULL;
 }
@@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
 static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 				   unsigned int *state)
 {
-	struct device_node *np = dev->of_node;
-	struct of_phandle_args args;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct fwnode_reference_args args;
 	struct mux_chip *mux_chip;
 	unsigned int controller;
 	int index = 0;
@@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 
 	if (mux_name) {
 		if (state)
-			index = of_property_match_string(np, "mux-state-names",
-							 mux_name);
+			index = fwnode_property_match_string(fwnode,
+					"mux-state-names", mux_name);
 		else
-			index = of_property_match_string(np, "mux-control-names",
-							 mux_name);
+			index = fwnode_property_match_string(fwnode,
+					"mux-control-names", mux_name);
 		if (index < 0) {
 			dev_err(dev, "mux controller '%s' not found\n",
 				mux_name);
@@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 	}
 
 	if (state)
-		ret = of_parse_phandle_with_args(np,
-						 "mux-states", "#mux-state-cells",
-						 index, &args);
+		ret = fwnode_property_get_reference_args(fwnode,
+					"mux-states", "#mux-state-cells",
+					0, index, &args);
 	else
-		ret = of_parse_phandle_with_args(np,
-						 "mux-controls", "#mux-control-cells",
-						 index, &args);
+		ret = fwnode_property_get_reference_args(fwnode,
+					"mux-controls", "#mux-control-cells",
+					0, index, &args);
+
 	if (ret) {
-		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
-			np, state ? "state" : "control", mux_name ?: "", index);
+		dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
+			fwnode, state ? "state" : "control", mux_name ?: "",
+			index);
 		return ERR_PTR(ret);
 	}
 
-	mux_chip = of_find_mux_chip_by_node(args.np);
-	of_node_put(args.np);
+	mux_chip = of_find_mux_chip_by_node(args.fwnode);
+	fwnode_handle_put(args.fwnode);
 	if (!mux_chip)
 		return ERR_PTR(-EPROBE_DEFER);
 
 	controller = 0;
 	if (state) {
-		if (args.args_count > 2 || args.args_count == 0 ||
-		    (args.args_count < 2 && mux_chip->controllers > 1)) {
-			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
-				np, args.np);
+		if (args.nargs > 2 || args.nargs == 0 ||
+		    (args.nargs < 2 && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
+				fwnode, args.fwnode);
 			put_device(&mux_chip->dev);
 			return ERR_PTR(-EINVAL);
 		}
 
-		if (args.args_count == 2) {
+		if (args.nargs == 2) {
 			controller = args.args[0];
 			*state = args.args[1];
 		} else {
@@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 		}
 
 	} else {
-		if (args.args_count > 1 ||
-		    (!args.args_count && mux_chip->controllers > 1)) {
-			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
-				np, args.np);
+		if (args.nargs > 1 ||
+		    (!args.nargs && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
+				fwnode, args.fwnode);
 			put_device(&mux_chip->dev);
 			return ERR_PTR(-EINVAL);
 		}
 
-		if (args.args_count)
+		if (args.nargs)
 			controller = args.args[0];
 	}
 
 	if (controller >= mux_chip->controllers) {
-		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
-			np, controller, args.np);
+		dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
+			fwnode, controller, args.fwnode);
 		put_device(&mux_chip->dev);
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.34.1


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

[PATCH v2 2/4] mux: convert to use fwnode interface

[Xu Yang]

As firmware node is a more common abstract, this will convert the whole
thing to fwnode interface.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- convert to use fwnode interface

 drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 49bedbe6316c..e30e859efd33 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -18,8 +18,7 @@
 #include <linux/module.h>
 #include <linux/mux/consumer.h>
 #include <linux/mux/driver.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 /*
@@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
 EXPORT_SYMBOL_GPL(mux_state_deselect);
 
 /* Note this function returns a reference to the mux_chip dev. */
-static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
+static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)
 {
 	struct device *dev;
 
-	dev = class_find_device_by_of_node(&mux_class, np);
+	dev = class_find_device_by_fwnode(&mux_class, fwnode);
 
 	return dev ? to_mux_chip(dev) : NULL;
 }
@@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
 static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 				   unsigned int *state)
 {
-	struct device_node *np = dev->of_node;
-	struct of_phandle_args args;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct fwnode_reference_args args;
 	struct mux_chip *mux_chip;
 	unsigned int controller;
 	int index = 0;
@@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 
 	if (mux_name) {
 		if (state)
-			index = of_property_match_string(np, "mux-state-names",
-							 mux_name);
+			index = fwnode_property_match_string(fwnode,
+					"mux-state-names", mux_name);
 		else
-			index = of_property_match_string(np, "mux-control-names",
-							 mux_name);
+			index = fwnode_property_match_string(fwnode,
+					"mux-control-names", mux_name);
 		if (index < 0) {
 			dev_err(dev, "mux controller '%s' not found\n",
 				mux_name);
@@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 	}
 
 	if (state)
-		ret = of_parse_phandle_with_args(np,
-						 "mux-states", "#mux-state-cells",
-						 index, &args);
+		ret = fwnode_property_get_reference_args(fwnode,
+					"mux-states", "#mux-state-cells",
+					0, index, &args);
 	else
-		ret = of_parse_phandle_with_args(np,
-						 "mux-controls", "#mux-control-cells",
-						 index, &args);
+		ret = fwnode_property_get_reference_args(fwnode,
+					"mux-controls", "#mux-control-cells",
+					0, index, &args);
+
 	if (ret) {
-		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
-			np, state ? "state" : "control", mux_name ?: "", index);
+		dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
+			fwnode, state ? "state" : "control", mux_name ?: "",
+			index);
 		return ERR_PTR(ret);
 	}
 
-	mux_chip = of_find_mux_chip_by_node(args.np);
-	of_node_put(args.np);
+	mux_chip = of_find_mux_chip_by_node(args.fwnode);
+	fwnode_handle_put(args.fwnode);
 	if (!mux_chip)
 		return ERR_PTR(-EPROBE_DEFER);
 
 	controller = 0;
 	if (state) {
-		if (args.args_count > 2 || args.args_count == 0 ||
-		    (args.args_count < 2 && mux_chip->controllers > 1)) {
-			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
-				np, args.np);
+		if (args.nargs > 2 || args.nargs == 0 ||
+		    (args.nargs < 2 && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
+				fwnode, args.fwnode);
 			put_device(&mux_chip->dev);
 			return ERR_PTR(-EINVAL);
 		}
 
-		if (args.args_count == 2) {
+		if (args.nargs == 2) {
 			controller = args.args[0];
 			*state = args.args[1];
 		} else {
@@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 		}
 
 	} else {
-		if (args.args_count > 1 ||
-		    (!args.args_count && mux_chip->controllers > 1)) {
-			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
-				np, args.np);
+		if (args.nargs > 1 ||
+		    (!args.nargs && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
+				fwnode, args.fwnode);
 			put_device(&mux_chip->dev);
 			return ERR_PTR(-EINVAL);
 		}
 
-		if (args.args_count)
+		if (args.nargs)
 			controller = args.args[0];
 	}
 
 	if (controller >= mux_chip->controllers) {
-		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
-			np, controller, args.np);
+		dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
+			fwnode, controller, args.fwnode);
 		put_device(&mux_chip->dev);
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.34.1

[PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Xu Yang]

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.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- add build dependence (select MULTIPLEXER)
- improve Multiplexer control logic

 drivers/usb/typec/Kconfig     |  1 +
 drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
 include/linux/usb/typec_mux.h |  7 +---
 3 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 5defdfead653..73d4976d8148 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -2,6 +2,7 @@
 
 menuconfig TYPEC
 	tristate "USB Type-C Support"
+	select MULTIPLEXER
 	help
 	  USB Type-C Specification defines a cable and connector for USB where
 	  only one type of plug is supported on both ends, i.e. there will not
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 464330776cd6..05e6ed217620 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)
 }
 EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
 
+static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
+{
+	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
+ * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
 			return ret;
 	}
 
-	return 0;
+	mux = sw->mux_switch;
+	if (!mux)
+		return 0;
+
+	ret = mux_control_select(mux, sw->mux_states[orientation]);
+	if (ret)
+		return ret;
+
+	/* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
+	ret = mux_control_deselect(mux);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(typec_switch_set);
 
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
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);
 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);
-- 
2.34.1


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

[PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Xu Yang]

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.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- add build dependence (select MULTIPLEXER)
- improve Multiplexer control logic

 drivers/usb/typec/Kconfig     |  1 +
 drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
 include/linux/usb/typec_mux.h |  7 +---
 3 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 5defdfead653..73d4976d8148 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -2,6 +2,7 @@
 
 menuconfig TYPEC
 	tristate "USB Type-C Support"
+	select MULTIPLEXER
 	help
 	  USB Type-C Specification defines a cable and connector for USB where
 	  only one type of plug is supported on both ends, i.e. there will not
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 464330776cd6..05e6ed217620 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)
 }
 EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
 
+static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
+{
+	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
+ * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
 			return ret;
 	}
 
-	return 0;
+	mux = sw->mux_switch;
+	if (!mux)
+		return 0;
+
+	ret = mux_control_select(mux, sw->mux_states[orientation]);
+	if (ret)
+		return ret;
+
+	/* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
+	ret = mux_control_deselect(mux);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(typec_switch_set);
 
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
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);
 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);
-- 
2.34.1

[PATCH v2 4/4] arm64: dts: imx8mp-evk: add typec node

[Xu Yang]

The first port of USB with type-C connector, which has dual data
role and dual power role.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- change mux controller idle-state to MUX_IDLE_AS_IS

 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 122 +++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f6b017ab5f53..a8e45a724fd9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -5,6 +5,8 @@
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
+#include <dt-bindings/mux/mux.h>
 #include "imx8mp.dtsi"
 
 / {
@@ -65,6 +67,22 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
 		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 	};
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec_mux>;
+		mux-gpios = <&gpio4 20 GPIO_ACTIVE_HIGH>,
+			    <&gpio2 20 GPIO_ACTIVE_HIGH>;
+		idle-state = <MUX_IDLE_AS_IS>;
+
+		port {
+			usb3_data_ss: endpoint {
+				remote-endpoint = <&typec_con_ss>;
+			};
+		};
+	};
 };
 
 &A53_0 {
@@ -299,6 +317,56 @@ LDO5 {
 	};
 };
 
+&i2c2 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+
+	ptn5110: tcpc@50 {
+		compatible = "nxp,ptn5110";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec>;
+		reg = <0x50>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <19 8>;
+
+		port {
+			typec_dr_sw: endpoint {
+				remote-endpoint = <&usb3_drd_sw>;
+			};
+		};
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			data-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_VAR(5000, 20000, 3000)>;
+			op-sink-microwatt = <15000000>;
+			self-powered;
+
+			mux-controls = <&mux>;
+			typec-switch-states = <2>, <0>, <1>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+					typec_con_ss: endpoint {
+						remote-endpoint = <&usb3_data_ss>;
+					};
+				};
+			};
+		};
+	};
+};
+
 &i2c3 {
 	clock-frequency = <400000>;
 	pinctrl-names = "default";
@@ -361,7 +429,41 @@ &uart2 {
 	status = "okay";
 };
 
+&usb3_phy0 {
+	fsl,phy-tx-vref-tune = <0xe>;
+	fsl,phy-tx-preemp-amp-tune = <3>;
+	fsl,phy-tx-vboost-level = <5>;
+	fsl,phy-comp-dis-tune = <7>;
+	fsl,pcs-tx-deemph-3p5db = <0x21>;
+	fsl,phy-pcs-tx-swing-full = <0x7f>;
+	status = "okay";
+};
+
+&usb3_0 {
+	status = "okay";
+};
+
+&usb_dwc3_0 {
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	usb-role-switch;
+	role-switch-default-mode = "none";
+	snps,dis-u1-entry-quirk;
+	snps,dis-u2-entry-quirk;
+	status = "okay";
+
+	port {
+		usb3_drd_sw: endpoint {
+			remote-endpoint = <&typec_dr_sw>;
+		};
+	};
+};
+
 &usb3_phy1 {
+	fsl,phy-tx-preemp-amp-tune = <3>;
+	fsl,phy-tx-vref-tune = <0xb>;
 	status = "okay";
 };
 
@@ -488,6 +590,13 @@ MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c2
 		>;
 	};
 
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL         0x400001c2
+			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA         0x400001c2
+		>;
+	};
+
 	pinctrl_i2c3: i2c3grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL		0x400001c2
@@ -527,6 +636,19 @@ MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x140
 		>;
 	};
 
+	pinctrl_typec: typec1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19      0x1c4
+		>;
+	};
+
+	pinctrl_typec_mux: typec1muxgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20      0x16
+			MX8MP_IOMUXC_SD2_WP__GPIO2_IO20		0x16
+		>;
+	};
+
 	pinctrl_usb1_vbus: usb1grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_GPIO1_IO14__USB2_OTG_PWR	0x10
-- 
2.34.1


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

[PATCH v2 4/4] arm64: dts: imx8mp-evk: add typec node

[Xu Yang]

The first port of USB with type-C connector, which has dual data
role and dual power role.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- change mux controller idle-state to MUX_IDLE_AS_IS

 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 122 +++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f6b017ab5f53..a8e45a724fd9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -5,6 +5,8 @@
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
+#include <dt-bindings/mux/mux.h>
 #include "imx8mp.dtsi"
 
 / {
@@ -65,6 +67,22 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
 		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 	};
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec_mux>;
+		mux-gpios = <&gpio4 20 GPIO_ACTIVE_HIGH>,
+			    <&gpio2 20 GPIO_ACTIVE_HIGH>;
+		idle-state = <MUX_IDLE_AS_IS>;
+
+		port {
+			usb3_data_ss: endpoint {
+				remote-endpoint = <&typec_con_ss>;
+			};
+		};
+	};
 };
 
 &A53_0 {
@@ -299,6 +317,56 @@ LDO5 {
 	};
 };
 
+&i2c2 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+
+	ptn5110: tcpc@50 {
+		compatible = "nxp,ptn5110";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_typec>;
+		reg = <0x50>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <19 8>;
+
+		port {
+			typec_dr_sw: endpoint {
+				remote-endpoint = <&usb3_drd_sw>;
+			};
+		};
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			power-role = "dual";
+			data-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+				     PDO_VAR(5000, 20000, 3000)>;
+			op-sink-microwatt = <15000000>;
+			self-powered;
+
+			mux-controls = <&mux>;
+			typec-switch-states = <2>, <0>, <1>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+					typec_con_ss: endpoint {
+						remote-endpoint = <&usb3_data_ss>;
+					};
+				};
+			};
+		};
+	};
+};
+
 &i2c3 {
 	clock-frequency = <400000>;
 	pinctrl-names = "default";
@@ -361,7 +429,41 @@ &uart2 {
 	status = "okay";
 };
 
+&usb3_phy0 {
+	fsl,phy-tx-vref-tune = <0xe>;
+	fsl,phy-tx-preemp-amp-tune = <3>;
+	fsl,phy-tx-vboost-level = <5>;
+	fsl,phy-comp-dis-tune = <7>;
+	fsl,pcs-tx-deemph-3p5db = <0x21>;
+	fsl,phy-pcs-tx-swing-full = <0x7f>;
+	status = "okay";
+};
+
+&usb3_0 {
+	status = "okay";
+};
+
+&usb_dwc3_0 {
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	adp-disable;
+	usb-role-switch;
+	role-switch-default-mode = "none";
+	snps,dis-u1-entry-quirk;
+	snps,dis-u2-entry-quirk;
+	status = "okay";
+
+	port {
+		usb3_drd_sw: endpoint {
+			remote-endpoint = <&typec_dr_sw>;
+		};
+	};
+};
+
 &usb3_phy1 {
+	fsl,phy-tx-preemp-amp-tune = <3>;
+	fsl,phy-tx-vref-tune = <0xb>;
 	status = "okay";
 };
 
@@ -488,6 +590,13 @@ MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c2
 		>;
 	};
 
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL         0x400001c2
+			MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA         0x400001c2
+		>;
+	};
+
 	pinctrl_i2c3: i2c3grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL		0x400001c2
@@ -527,6 +636,19 @@ MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX	0x140
 		>;
 	};
 
+	pinctrl_typec: typec1grp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19      0x1c4
+		>;
+	};
+
+	pinctrl_typec_mux: typec1muxgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20      0x16
+			MX8MP_IOMUXC_SD2_WP__GPIO2_IO20		0x16
+		>;
+	};
+
 	pinctrl_usb1_vbus: usb1grp {
 		fsl,pins = <
 			MX8MP_IOMUXC_GPIO1_IO14__USB2_OTG_PWR	0x10
-- 
2.34.1

Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Peter Rosin]

Hi!

2022-08-23 at 21:54, Xu Yang wrote:
> 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.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - add build dependence (select MULTIPLEXER)
> - improve Multiplexer control logic
> 
>  drivers/usb/typec/Kconfig     |  1 +
>  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
>  include/linux/usb/typec_mux.h |  7 +---
>  3 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 5defdfead653..73d4976d8148 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -2,6 +2,7 @@
>  
>  menuconfig TYPEC
>  	tristate "USB Type-C Support"
> +	select MULTIPLEXER
>  	help
>  	  USB Type-C Specification defines a cable and connector for USB where
>  	  only one type of plug is supported on both ends, i.e. there will not
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 464330776cd6..05e6ed217620 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)
>  }
>  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
>  
> +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> +{
> +	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
> + * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
>  			return ret;
>  	}
>  
> -	return 0;
> +	mux = sw->mux_switch;
> +	if (!mux)
> +		return 0;
> +
> +	ret = mux_control_select(mux, sw->mux_states[orientation]);
> +	if (ret)
> +		return ret;
> +
> +	/* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
> +	ret = mux_control_deselect(mux);

No, this is also broken. You cannot, in any client driver, rely on a
mux keeping its state unless you keep it selected. As soon as you
deselect it, it might be selected by some other driver. Sure, you
might know that there are no other users of the mux in question, and
you might also know that the idles state is "as-is". But the driver
does not see the bigger picture and has no way of knowing that. So,
it needs to keep the mux selected.

Cheers,
Peter

> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(typec_switch_set);
>  
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> 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);
>  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);

Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Peter Rosin]

Hi!

2022-08-23 at 21:54, Xu Yang wrote:
> 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.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - add build dependence (select MULTIPLEXER)
> - improve Multiplexer control logic
> 
>  drivers/usb/typec/Kconfig     |  1 +
>  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
>  include/linux/usb/typec_mux.h |  7 +---
>  3 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 5defdfead653..73d4976d8148 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -2,6 +2,7 @@
>  
>  menuconfig TYPEC
>  	tristate "USB Type-C Support"
> +	select MULTIPLEXER
>  	help
>  	  USB Type-C Specification defines a cable and connector for USB where
>  	  only one type of plug is supported on both ends, i.e. there will not
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 464330776cd6..05e6ed217620 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)
>  }
>  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
>  
> +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> +{
> +	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
> + * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
>  			return ret;
>  	}
>  
> -	return 0;
> +	mux = sw->mux_switch;
> +	if (!mux)
> +		return 0;
> +
> +	ret = mux_control_select(mux, sw->mux_states[orientation]);
> +	if (ret)
> +		return ret;
> +
> +	/* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
> +	ret = mux_control_deselect(mux);

No, this is also broken. You cannot, in any client driver, rely on a
mux keeping its state unless you keep it selected. As soon as you
deselect it, it might be selected by some other driver. Sure, you
might know that there are no other users of the mux in question, and
you might also know that the idles state is "as-is". But the driver
does not see the bigger picture and has no way of knowing that. So,
it needs to keep the mux selected.

Cheers,
Peter

> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(typec_switch_set);
>  
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> 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);
>  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

Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Heikki Krogerus]

Hi,

On Wed, Aug 24, 2022 at 03:54:28AM +0800, Xu Yang wrote:
> 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.
> 
> Reported-by: kernel test robot <lkp@intel.com>

Drop that Reported-by tag.

> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - add build dependence (select MULTIPLEXER)
> - improve Multiplexer control logic
> 
>  drivers/usb/typec/Kconfig     |  1 +
>  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
>  include/linux/usb/typec_mux.h |  7 +---
>  3 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 5defdfead653..73d4976d8148 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -2,6 +2,7 @@
>  
>  menuconfig TYPEC
>  	tristate "USB Type-C Support"
> +	select MULTIPLEXER

Whoa, do not tie TYPEC to another subsystem like that! You probable
want to make your driver depend on that instead.

>  	help
>  	  USB Type-C Specification defines a cable and connector for USB where
>  	  only one type of plug is supported on both ends, i.e. there will not
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 464330776cd6..05e6ed217620 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;

That does not belong here...

> +	/* 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)
>  }
>  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
>  
> +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> +{
> +	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;
> +}

That code is broken - you return a switch that has never been
registered - but never mind that...

You need to register the switch from the place/driver that actually
handles the orientation. If you really think that you have to have a
generic multiplexer class wrapper layer for these switches, then you
need to propose a multiplexer class wrapper driver for that. Though,
I'm not sure you need that.

There is a GPIO driver proposal for these switches, so can you take a
look if that covers your case as well:
https://lore.kernel.org/linux-usb/20220810204750.3672362-3-bjorn.andersson@linaro.org/#t

In any case, you can't mix that code into the device class code itself
like you do above. That kind of coupling is always going to be fragile
(as we can see above), but more importantly, it forces the dependency
on every Type-C driver, and that is completely wrong.

So please keep the subsystems themselves independent of each other and
handle things in the device drivers.

thanks,

-- 
heikki

Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Heikki Krogerus]

Hi,

On Wed, Aug 24, 2022 at 03:54:28AM +0800, Xu Yang wrote:
> 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.
> 
> Reported-by: kernel test robot <lkp@intel.com>

Drop that Reported-by tag.

> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - add build dependence (select MULTIPLEXER)
> - improve Multiplexer control logic
> 
>  drivers/usb/typec/Kconfig     |  1 +
>  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
>  include/linux/usb/typec_mux.h |  7 +---
>  3 files changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 5defdfead653..73d4976d8148 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -2,6 +2,7 @@
>  
>  menuconfig TYPEC
>  	tristate "USB Type-C Support"
> +	select MULTIPLEXER

Whoa, do not tie TYPEC to another subsystem like that! You probable
want to make your driver depend on that instead.

>  	help
>  	  USB Type-C Specification defines a cable and connector for USB where
>  	  only one type of plug is supported on both ends, i.e. there will not
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 464330776cd6..05e6ed217620 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;

That does not belong here...

> +	/* 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)
>  }
>  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
>  
> +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> +{
> +	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;
> +}

That code is broken - you return a switch that has never been
registered - but never mind that...

You need to register the switch from the place/driver that actually
handles the orientation. If you really think that you have to have a
generic multiplexer class wrapper layer for these switches, then you
need to propose a multiplexer class wrapper driver for that. Though,
I'm not sure you need that.

There is a GPIO driver proposal for these switches, so can you take a
look if that covers your case as well:
https://lore.kernel.org/linux-usb/20220810204750.3672362-3-bjorn.andersson@linaro.org/#t

In any case, you can't mix that code into the device class code itself
like you do above. That kind of coupling is always going to be fragile
(as we can see above), but more importantly, it forces the dependency
on every Type-C driver, and that is completely wrong.

So please keep the subsystems themselves independent of each other and
handle things in the device drivers.

thanks,

-- 
heikki

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

Re: [PATCH v2 2/4] mux: convert to use fwnode interface

[Peter Rosin]

Hi!

Much better, one small thing though...

2022-08-23 at 21:54, Xu Yang wrote:
> As firmware node is a more common abstract, this will convert the whole
> thing to fwnode interface.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - convert to use fwnode interface
> 
>  drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 49bedbe6316c..e30e859efd33 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -18,8 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/mux/consumer.h>
>  #include <linux/mux/driver.h>
> -#include <linux/of.h>
> -#include <linux/of_platform.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  
>  /*
> @@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
>  EXPORT_SYMBOL_GPL(mux_state_deselect);
>  
>  /* Note this function returns a reference to the mux_chip dev. */
> -static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> +static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)

Please rename this function to mux_chip_find_by_fwnode()

Cheers,
Peter

>  {
>  	struct device *dev;
>  
> -	dev = class_find_device_by_of_node(&mux_class, np);
> +	dev = class_find_device_by_fwnode(&mux_class, fwnode);
>  
>  	return dev ? to_mux_chip(dev) : NULL;
>  }
> @@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
>  static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  				   unsigned int *state)
>  {
> -	struct device_node *np = dev->of_node;
> -	struct of_phandle_args args;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +	struct fwnode_reference_args args;
>  	struct mux_chip *mux_chip;
>  	unsigned int controller;
>  	int index = 0;
> @@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  
>  	if (mux_name) {
>  		if (state)
> -			index = of_property_match_string(np, "mux-state-names",
> -							 mux_name);
> +			index = fwnode_property_match_string(fwnode,
> +					"mux-state-names", mux_name);
>  		else
> -			index = of_property_match_string(np, "mux-control-names",
> -							 mux_name);
> +			index = fwnode_property_match_string(fwnode,
> +					"mux-control-names", mux_name);
>  		if (index < 0) {
>  			dev_err(dev, "mux controller '%s' not found\n",
>  				mux_name);
> @@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  	}
>  
>  	if (state)
> -		ret = of_parse_phandle_with_args(np,
> -						 "mux-states", "#mux-state-cells",
> -						 index, &args);
> +		ret = fwnode_property_get_reference_args(fwnode,
> +					"mux-states", "#mux-state-cells",
> +					0, index, &args);
>  	else
> -		ret = of_parse_phandle_with_args(np,
> -						 "mux-controls", "#mux-control-cells",
> -						 index, &args);
> +		ret = fwnode_property_get_reference_args(fwnode,
> +					"mux-controls", "#mux-control-cells",
> +					0, index, &args);
> +
>  	if (ret) {
> -		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> -			np, state ? "state" : "control", mux_name ?: "", index);
> +		dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
> +			fwnode, state ? "state" : "control", mux_name ?: "",
> +			index);
>  		return ERR_PTR(ret);
>  	}
>  
> -	mux_chip = of_find_mux_chip_by_node(args.np);
> -	of_node_put(args.np);
> +	mux_chip = of_find_mux_chip_by_node(args.fwnode);
> +	fwnode_handle_put(args.fwnode);
>  	if (!mux_chip)
>  		return ERR_PTR(-EPROBE_DEFER);
>  
>  	controller = 0;
>  	if (state) {
> -		if (args.args_count > 2 || args.args_count == 0 ||
> -		    (args.args_count < 2 && mux_chip->controllers > 1)) {
> -			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
> -				np, args.np);
> +		if (args.nargs > 2 || args.nargs == 0 ||
> +		    (args.nargs < 2 && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
> +				fwnode, args.fwnode);
>  			put_device(&mux_chip->dev);
>  			return ERR_PTR(-EINVAL);
>  		}
>  
> -		if (args.args_count == 2) {
> +		if (args.nargs == 2) {
>  			controller = args.args[0];
>  			*state = args.args[1];
>  		} else {
> @@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  		}
>  
>  	} else {
> -		if (args.args_count > 1 ||
> -		    (!args.args_count && mux_chip->controllers > 1)) {
> -			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
> -				np, args.np);
> +		if (args.nargs > 1 ||
> +		    (!args.nargs && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
> +				fwnode, args.fwnode);
>  			put_device(&mux_chip->dev);
>  			return ERR_PTR(-EINVAL);
>  		}
>  
> -		if (args.args_count)
> +		if (args.nargs)
>  			controller = args.args[0];
>  	}
>  
>  	if (controller >= mux_chip->controllers) {
> -		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
> -			np, controller, args.np);
> +		dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
> +			fwnode, controller, args.fwnode);
>  		put_device(&mux_chip->dev);
>  		return ERR_PTR(-EINVAL);
>  	}

Re: [PATCH v2 2/4] mux: convert to use fwnode interface

[Peter Rosin]

Hi!

Much better, one small thing though...

2022-08-23 at 21:54, Xu Yang wrote:
> As firmware node is a more common abstract, this will convert the whole
> thing to fwnode interface.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - convert to use fwnode interface
> 
>  drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 49bedbe6316c..e30e859efd33 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -18,8 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/mux/consumer.h>
>  #include <linux/mux/driver.h>
> -#include <linux/of.h>
> -#include <linux/of_platform.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  
>  /*
> @@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
>  EXPORT_SYMBOL_GPL(mux_state_deselect);
>  
>  /* Note this function returns a reference to the mux_chip dev. */
> -static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> +static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)

Please rename this function to mux_chip_find_by_fwnode()

Cheers,
Peter

>  {
>  	struct device *dev;
>  
> -	dev = class_find_device_by_of_node(&mux_class, np);
> +	dev = class_find_device_by_fwnode(&mux_class, fwnode);
>  
>  	return dev ? to_mux_chip(dev) : NULL;
>  }
> @@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
>  static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  				   unsigned int *state)
>  {
> -	struct device_node *np = dev->of_node;
> -	struct of_phandle_args args;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +	struct fwnode_reference_args args;
>  	struct mux_chip *mux_chip;
>  	unsigned int controller;
>  	int index = 0;
> @@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  
>  	if (mux_name) {
>  		if (state)
> -			index = of_property_match_string(np, "mux-state-names",
> -							 mux_name);
> +			index = fwnode_property_match_string(fwnode,
> +					"mux-state-names", mux_name);
>  		else
> -			index = of_property_match_string(np, "mux-control-names",
> -							 mux_name);
> +			index = fwnode_property_match_string(fwnode,
> +					"mux-control-names", mux_name);
>  		if (index < 0) {
>  			dev_err(dev, "mux controller '%s' not found\n",
>  				mux_name);
> @@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  	}
>  
>  	if (state)
> -		ret = of_parse_phandle_with_args(np,
> -						 "mux-states", "#mux-state-cells",
> -						 index, &args);
> +		ret = fwnode_property_get_reference_args(fwnode,
> +					"mux-states", "#mux-state-cells",
> +					0, index, &args);
>  	else
> -		ret = of_parse_phandle_with_args(np,
> -						 "mux-controls", "#mux-control-cells",
> -						 index, &args);
> +		ret = fwnode_property_get_reference_args(fwnode,
> +					"mux-controls", "#mux-control-cells",
> +					0, index, &args);
> +
>  	if (ret) {
> -		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> -			np, state ? "state" : "control", mux_name ?: "", index);
> +		dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
> +			fwnode, state ? "state" : "control", mux_name ?: "",
> +			index);
>  		return ERR_PTR(ret);
>  	}
>  
> -	mux_chip = of_find_mux_chip_by_node(args.np);
> -	of_node_put(args.np);
> +	mux_chip = of_find_mux_chip_by_node(args.fwnode);
> +	fwnode_handle_put(args.fwnode);
>  	if (!mux_chip)
>  		return ERR_PTR(-EPROBE_DEFER);
>  
>  	controller = 0;
>  	if (state) {
> -		if (args.args_count > 2 || args.args_count == 0 ||
> -		    (args.args_count < 2 && mux_chip->controllers > 1)) {
> -			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
> -				np, args.np);
> +		if (args.nargs > 2 || args.nargs == 0 ||
> +		    (args.nargs < 2 && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
> +				fwnode, args.fwnode);
>  			put_device(&mux_chip->dev);
>  			return ERR_PTR(-EINVAL);
>  		}
>  
> -		if (args.args_count == 2) {
> +		if (args.nargs == 2) {
>  			controller = args.args[0];
>  			*state = args.args[1];
>  		} else {
> @@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  		}
>  
>  	} else {
> -		if (args.args_count > 1 ||
> -		    (!args.args_count && mux_chip->controllers > 1)) {
> -			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
> -				np, args.np);
> +		if (args.nargs > 1 ||
> +		    (!args.nargs && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
> +				fwnode, args.fwnode);
>  			put_device(&mux_chip->dev);
>  			return ERR_PTR(-EINVAL);
>  		}
>  
> -		if (args.args_count)
> +		if (args.nargs)
>  			controller = args.args[0];
>  	}
>  
>  	if (controller >= mux_chip->controllers) {
> -		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
> -			np, controller, args.np);
> +		dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
> +			fwnode, controller, args.fwnode);
>  		put_device(&mux_chip->dev);
>  		return ERR_PTR(-EINVAL);
>  	}

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

RE: [EXT] Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Xu Yang]

Hi Peter,

> -----Original Message-----
> From: Peter Rosin <peda@axentia.se>
> Sent: Wednesday, August 24, 2022 7:51 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 <jun.li@nxp.com>; 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 v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller
> 
> Caution: EXT Email
> 
> Hi!
> 
> 2022-08-23 at 21:54, Xu Yang wrote:
> > 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.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes since v1:
> > - add build dependence (select MULTIPLEXER)
> > - improve Multiplexer control logic
> >
> >  drivers/usb/typec/Kconfig     |  1 +
> >  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
> >  include/linux/usb/typec_mux.h |  7 +---
> >  3 files changed, 78 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 5defdfead653..73d4976d8148 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -2,6 +2,7 @@
> >
> >  menuconfig TYPEC
> >       tristate "USB Type-C Support"
> > +     select MULTIPLEXER
> >       help
> >         USB Type-C Specification defines a cable and connector for USB where
> >         only one type of plug is supported on both ends, i.e. there will not
> > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> > index 464330776cd6..05e6ed217620 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)
> >  }
> >  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
> >
> > +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> > +{
> > +     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
> > + * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
> >                       return ret;
> >       }
> >
> > -     return 0;
> > +     mux = sw->mux_switch;
> > +     if (!mux)
> > +             return 0;
> > +
> > +     ret = mux_control_select(mux, sw->mux_states[orientation]);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
> > +     ret = mux_control_deselect(mux);
> 
> No, this is also broken. You cannot, in any client driver, rely on a
> mux keeping its state unless you keep it selected. As soon as you
> deselect it, it might be selected by some other driver. Sure, you
> might know that there are no other users of the mux in question, and
> you might also know that the idles state is "as-is". But the driver
> does not see the bigger picture and has no way of knowing that. So,
> it needs to keep the mux selected.
> 

Understood.  May be a flag is needed like mdio-mux-multiplexer does.
I will improve it in next version.

Thanks,
Xu Yang

> Cheers,
> Peter
> 
> > +
> > +     return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_switch_set);
> >
> > diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> > 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);
> >  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);

RE: [EXT] Re: [PATCH v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller

[Xu Yang]

Hi Peter,

> -----Original Message-----
> From: Peter Rosin <peda@axentia.se>
> Sent: Wednesday, August 24, 2022 7:51 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 <jun.li@nxp.com>; 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 v2 3/4] usb: typec: mux: add typec orientation switch support via mux controller
> 
> Caution: EXT Email
> 
> Hi!
> 
> 2022-08-23 at 21:54, Xu Yang wrote:
> > 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.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes since v1:
> > - add build dependence (select MULTIPLEXER)
> > - improve Multiplexer control logic
> >
> >  drivers/usb/typec/Kconfig     |  1 +
> >  drivers/usb/typec/mux.c       | 76 ++++++++++++++++++++++++++++++++++-
> >  include/linux/usb/typec_mux.h |  7 +---
> >  3 files changed, 78 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 5defdfead653..73d4976d8148 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -2,6 +2,7 @@
> >
> >  menuconfig TYPEC
> >       tristate "USB Type-C Support"
> > +     select MULTIPLEXER
> >       help
> >         USB Type-C Specification defines a cable and connector for USB where
> >         only one type of plug is supported on both ends, i.e. there will not
> > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> > index 464330776cd6..05e6ed217620 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)
> >  }
> >  EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
> >
> > +static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
> > +{
> > +     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
> > + * 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,7 +281,18 @@ int typec_switch_set(struct typec_switch *sw,
> >                       return ret;
> >       }
> >
> > -     return 0;
> > +     mux = sw->mux_switch;
> > +     if (!mux)
> > +             return 0;
> > +
> > +     ret = mux_control_select(mux, sw->mux_states[orientation]);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Keep it as it is since idle_state is MUX_IDLE_AS_IS */
> > +     ret = mux_control_deselect(mux);
> 
> No, this is also broken. You cannot, in any client driver, rely on a
> mux keeping its state unless you keep it selected. As soon as you
> deselect it, it might be selected by some other driver. Sure, you
> might know that there are no other users of the mux in question, and
> you might also know that the idles state is "as-is". But the driver
> does not see the bigger picture and has no way of knowing that. So,
> it needs to keep the mux selected.
> 

Understood.  May be a flag is needed like mdio-mux-multiplexer does.
I will improve it in next version.

Thanks,
Xu Yang

> Cheers,
> Peter
> 
> > +
> > +     return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_switch_set);
> >
> > diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> > 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);
> >  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

RE: [EXT] Re: [PATCH v2 2/4] mux: convert to use fwnode interface

[Xu Yang]

Hi Peter,

> -----Original Message-----
> From: Peter Rosin <peda@axentia.se>
> Sent: Thursday, August 25, 2022 5:15 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 <jun.li@nxp.com>; 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 v2 2/4] mux: convert to use fwnode interface
> 
> Caution: EXT Email
> 
> Hi!
> 
> Much better, one small thing though...
> 
> 2022-08-23 at 21:54, Xu Yang wrote:
> > As firmware node is a more common abstract, this will convert the whole
> > thing to fwnode interface.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes since v1:
> > - convert to use fwnode interface
> >
> >  drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
> >  1 file changed, 33 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> > index 49bedbe6316c..e30e859efd33 100644
> > --- a/drivers/mux/core.c
> > +++ b/drivers/mux/core.c
> > @@ -18,8 +18,7 @@
> >  #include <linux/module.h>
> >  #include <linux/mux/consumer.h>
> >  #include <linux/mux/driver.h>
> > -#include <linux/of.h>
> > -#include <linux/of_platform.h>
> > +#include <linux/property.h>
> >  #include <linux/slab.h>
> >
> >  /*
> > @@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
> >  EXPORT_SYMBOL_GPL(mux_state_deselect);
> >
> >  /* Note this function returns a reference to the mux_chip dev. */
> > -static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> > +static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)
> 
> Please rename this function to mux_chip_find_by_fwnode()
> 

Thanks for your review. 
Will change it in next version.

Best Regards,
Xu Yang

> Cheers,
> Peter
> 
> >  {
> >       struct device *dev;
> >
> > -     dev = class_find_device_by_of_node(&mux_class, np);
> > +     dev = class_find_device_by_fwnode(&mux_class, fwnode);
> >
> >       return dev ? to_mux_chip(dev) : NULL;
> >  }
> > @@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> >  static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >                                  unsigned int *state)
> >  {
> > -     struct device_node *np = dev->of_node;
> > -     struct of_phandle_args args;
> > +     struct fwnode_handle *fwnode = dev_fwnode(dev);
> > +     struct fwnode_reference_args args;
> >       struct mux_chip *mux_chip;
> >       unsigned int controller;
> >       int index = 0;
> > @@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >
> >       if (mux_name) {
> >               if (state)
> > -                     index = of_property_match_string(np, "mux-state-names",
> > -                                                      mux_name);
> > +                     index = fwnode_property_match_string(fwnode,
> > +                                     "mux-state-names", mux_name);
> >               else
> > -                     index = of_property_match_string(np, "mux-control-names",
> > -                                                      mux_name);
> > +                     index = fwnode_property_match_string(fwnode,
> > +                                     "mux-control-names", mux_name);
> >               if (index < 0) {
> >                       dev_err(dev, "mux controller '%s' not found\n",
> >                               mux_name);
> > @@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >       }
> >
> >       if (state)
> > -             ret = of_parse_phandle_with_args(np,
> > -                                              "mux-states", "#mux-state-cells",
> > -                                              index, &args);
> > +             ret = fwnode_property_get_reference_args(fwnode,
> > +                                     "mux-states", "#mux-state-cells",
> > +                                     0, index, &args);
> >       else
> > -             ret = of_parse_phandle_with_args(np,
> > -                                              "mux-controls", "#mux-control-cells",
> > -                                              index, &args);
> > +             ret = fwnode_property_get_reference_args(fwnode,
> > +                                     "mux-controls", "#mux-control-cells",
> > +                                     0, index, &args);
> > +
> >       if (ret) {
> > -             dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> > -                     np, state ? "state" : "control", mux_name ?: "", index);
> > +             dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
> > +                     fwnode, state ? "state" : "control", mux_name ?: "",
> > +                     index);
> >               return ERR_PTR(ret);
> >       }
> >
> > -     mux_chip = of_find_mux_chip_by_node(args.np);
> > -     of_node_put(args.np);
> > +     mux_chip = of_find_mux_chip_by_node(args.fwnode);
> > +     fwnode_handle_put(args.fwnode);
> >       if (!mux_chip)
> >               return ERR_PTR(-EPROBE_DEFER);
> >
> >       controller = 0;
> >       if (state) {
> > -             if (args.args_count > 2 || args.args_count == 0 ||
> > -                 (args.args_count < 2 && mux_chip->controllers > 1)) {
> > -                     dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
> > -                             np, args.np);
> > +             if (args.nargs > 2 || args.nargs == 0 ||
> > +                 (args.nargs < 2 && mux_chip->controllers > 1)) {
> > +                     dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
> > +                             fwnode, args.fwnode);
> >                       put_device(&mux_chip->dev);
> >                       return ERR_PTR(-EINVAL);
> >               }
> >
> > -             if (args.args_count == 2) {
> > +             if (args.nargs == 2) {
> >                       controller = args.args[0];
> >                       *state = args.args[1];
> >               } else {
> > @@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >               }
> >
> >       } else {
> > -             if (args.args_count > 1 ||
> > -                 (!args.args_count && mux_chip->controllers > 1)) {
> > -                     dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
> > -                             np, args.np);
> > +             if (args.nargs > 1 ||
> > +                 (!args.nargs && mux_chip->controllers > 1)) {
> > +                     dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
> > +                             fwnode, args.fwnode);
> >                       put_device(&mux_chip->dev);
> >                       return ERR_PTR(-EINVAL);
> >               }
> >
> > -             if (args.args_count)
> > +             if (args.nargs)
> >                       controller = args.args[0];
> >       }
> >
> >       if (controller >= mux_chip->controllers) {
> > -             dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
> > -                     np, controller, args.np);
> > +             dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
> > +                     fwnode, controller, args.fwnode);
> >               put_device(&mux_chip->dev);
> >               return ERR_PTR(-EINVAL);
> >       }

RE: [EXT] Re: [PATCH v2 2/4] mux: convert to use fwnode interface

[Xu Yang]

Hi Peter,

> -----Original Message-----
> From: Peter Rosin <peda@axentia.se>
> Sent: Thursday, August 25, 2022 5:15 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 <jun.li@nxp.com>; 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 v2 2/4] mux: convert to use fwnode interface
> 
> Caution: EXT Email
> 
> Hi!
> 
> Much better, one small thing though...
> 
> 2022-08-23 at 21:54, Xu Yang wrote:
> > As firmware node is a more common abstract, this will convert the whole
> > thing to fwnode interface.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes since v1:
> > - convert to use fwnode interface
> >
> >  drivers/mux/core.c | 65 +++++++++++++++++++++++-----------------------
> >  1 file changed, 33 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> > index 49bedbe6316c..e30e859efd33 100644
> > --- a/drivers/mux/core.c
> > +++ b/drivers/mux/core.c
> > @@ -18,8 +18,7 @@
> >  #include <linux/module.h>
> >  #include <linux/mux/consumer.h>
> >  #include <linux/mux/driver.h>
> > -#include <linux/of.h>
> > -#include <linux/of_platform.h>
> > +#include <linux/property.h>
> >  #include <linux/slab.h>
> >
> >  /*
> > @@ -510,11 +509,11 @@ int mux_state_deselect(struct mux_state *mstate)
> >  EXPORT_SYMBOL_GPL(mux_state_deselect);
> >
> >  /* Note this function returns a reference to the mux_chip dev. */
> > -static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> > +static struct mux_chip *of_find_mux_chip_by_node(struct fwnode_handle *fwnode)
> 
> Please rename this function to mux_chip_find_by_fwnode()
> 

Thanks for your review. 
Will change it in next version.

Best Regards,
Xu Yang

> Cheers,
> Peter
> 
> >  {
> >       struct device *dev;
> >
> > -     dev = class_find_device_by_of_node(&mux_class, np);
> > +     dev = class_find_device_by_fwnode(&mux_class, fwnode);
> >
> >       return dev ? to_mux_chip(dev) : NULL;
> >  }
> > @@ -531,8 +530,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> >  static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >                                  unsigned int *state)
> >  {
> > -     struct device_node *np = dev->of_node;
> > -     struct of_phandle_args args;
> > +     struct fwnode_handle *fwnode = dev_fwnode(dev);
> > +     struct fwnode_reference_args args;
> >       struct mux_chip *mux_chip;
> >       unsigned int controller;
> >       int index = 0;
> > @@ -540,11 +539,11 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >
> >       if (mux_name) {
> >               if (state)
> > -                     index = of_property_match_string(np, "mux-state-names",
> > -                                                      mux_name);
> > +                     index = fwnode_property_match_string(fwnode,
> > +                                     "mux-state-names", mux_name);
> >               else
> > -                     index = of_property_match_string(np, "mux-control-names",
> > -                                                      mux_name);
> > +                     index = fwnode_property_match_string(fwnode,
> > +                                     "mux-control-names", mux_name);
> >               if (index < 0) {
> >                       dev_err(dev, "mux controller '%s' not found\n",
> >                               mux_name);
> > @@ -553,35 +552,37 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >       }
> >
> >       if (state)
> > -             ret = of_parse_phandle_with_args(np,
> > -                                              "mux-states", "#mux-state-cells",
> > -                                              index, &args);
> > +             ret = fwnode_property_get_reference_args(fwnode,
> > +                                     "mux-states", "#mux-state-cells",
> > +                                     0, index, &args);
> >       else
> > -             ret = of_parse_phandle_with_args(np,
> > -                                              "mux-controls", "#mux-control-cells",
> > -                                              index, &args);
> > +             ret = fwnode_property_get_reference_args(fwnode,
> > +                                     "mux-controls", "#mux-control-cells",
> > +                                     0, index, &args);
> > +
> >       if (ret) {
> > -             dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> > -                     np, state ? "state" : "control", mux_name ?: "", index);
> > +             dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
> > +                     fwnode, state ? "state" : "control", mux_name ?: "",
> > +                     index);
> >               return ERR_PTR(ret);
> >       }
> >
> > -     mux_chip = of_find_mux_chip_by_node(args.np);
> > -     of_node_put(args.np);
> > +     mux_chip = of_find_mux_chip_by_node(args.fwnode);
> > +     fwnode_handle_put(args.fwnode);
> >       if (!mux_chip)
> >               return ERR_PTR(-EPROBE_DEFER);
> >
> >       controller = 0;
> >       if (state) {
> > -             if (args.args_count > 2 || args.args_count == 0 ||
> > -                 (args.args_count < 2 && mux_chip->controllers > 1)) {
> > -                     dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
> > -                             np, args.np);
> > +             if (args.nargs > 2 || args.nargs == 0 ||
> > +                 (args.nargs < 2 && mux_chip->controllers > 1)) {
> > +                     dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
> > +                             fwnode, args.fwnode);
> >                       put_device(&mux_chip->dev);
> >                       return ERR_PTR(-EINVAL);
> >               }
> >
> > -             if (args.args_count == 2) {
> > +             if (args.nargs == 2) {
> >                       controller = args.args[0];
> >                       *state = args.args[1];
> >               } else {
> > @@ -589,21 +590,21 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >               }
> >
> >       } else {
> > -             if (args.args_count > 1 ||
> > -                 (!args.args_count && mux_chip->controllers > 1)) {
> > -                     dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
> > -                             np, args.np);
> > +             if (args.nargs > 1 ||
> > +                 (!args.nargs && mux_chip->controllers > 1)) {
> > +                     dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
> > +                             fwnode, args.fwnode);
> >                       put_device(&mux_chip->dev);
> >                       return ERR_PTR(-EINVAL);
> >               }
> >
> > -             if (args.args_count)
> > +             if (args.nargs)
> >                       controller = args.args[0];
> >       }
> >
> >       if (controller >= mux_chip->controllers) {
> > -             dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
> > -                     np, controller, args.np);
> > +             dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
> > +                     fwnode, controller, args.fwnode);
> >               put_device(&mux_chip->dev);
> >               return ERR_PTR(-EINVAL);
> >       }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: connector: Add typec orientation switch properties

[Rob Herring]

On Wed, Aug 24, 2022 at 03:54:26AM +0800, Xu Yang wrote:
> Typec orientation switch can be implemented as a consumer of mux
> controller. So we can use mux controller to control simple gpio switch
> or other types of switch. This will cover the following typec switch
> use case: High Speed, Super Speed and Sideband switch.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Please see and participate in the recent discussions around USB-C 
connectors:

https://lore.kernel.org/all/20220810204750.3672362-2-bjorn.andersson@linaro.org/
https://lore.kernel.org/all/20220622173605.1168416-1-pmalani@chromium.org/

As mentioned there, I want to see block diagrams of the h/w for these 
bindings.

The mux binding may be a good solution here, but different muxing 
scenarios need to be considered.


> ---
> Changes since v1:
> - No changes.
> 
>  .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index ae515651fc6b..47f53cdbf31a 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -221,6 +221,24 @@ properties:
>        SNK_READY for non-pd link.
>      type: boolean
>  
> +  # The following are optional properties for "usb-c-connector".
> +  mux-controls:
> +    description: Mux controller node to use for orientation switch selection. This mux controller
> +      could handle High Speed, Super Speed and Sideband switch use case one time. In orde to do so,
> +      besides mux settings need to be properly configured for each switch under mux-controller node,
> +      correct states should also be assigned to typec-switch-states parameter.

Wrap lines at 80 char.

> +    maxItems: 1
> +
> +  typec-switch-states:
> +    description: An ordered u32 array describing the mux state value for each typec orientations.
> +      Three states correspond to NONE(high impedance), NORMAL, REVERSE respectively. If there is
> +      no HW mux state for NONE, use value of NORMAL or REVERSE for it. If this mux controller
> +      handle more than 1 switch, correct states value need to be caculated according to the mux
> +      settings.
> +    minItems: 3
> +    maxItems: 3
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

I think that 'mux-states' is what you want to use here.

Rob

Re: [PATCH v2 1/4] dt-bindings: connector: Add typec orientation switch properties

[Rob Herring]

On Wed, Aug 24, 2022 at 03:54:26AM +0800, Xu Yang wrote:
> Typec orientation switch can be implemented as a consumer of mux
> controller. So we can use mux controller to control simple gpio switch
> or other types of switch. This will cover the following typec switch
> use case: High Speed, Super Speed and Sideband switch.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Please see and participate in the recent discussions around USB-C 
connectors:

https://lore.kernel.org/all/20220810204750.3672362-2-bjorn.andersson@linaro.org/
https://lore.kernel.org/all/20220622173605.1168416-1-pmalani@chromium.org/

As mentioned there, I want to see block diagrams of the h/w for these 
bindings.

The mux binding may be a good solution here, but different muxing 
scenarios need to be considered.


> ---
> Changes since v1:
> - No changes.
> 
>  .../bindings/connector/usb-connector.yaml      | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index ae515651fc6b..47f53cdbf31a 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -221,6 +221,24 @@ properties:
>        SNK_READY for non-pd link.
>      type: boolean
>  
> +  # The following are optional properties for "usb-c-connector".
> +  mux-controls:
> +    description: Mux controller node to use for orientation switch selection. This mux controller
> +      could handle High Speed, Super Speed and Sideband switch use case one time. In orde to do so,
> +      besides mux settings need to be properly configured for each switch under mux-controller node,
> +      correct states should also be assigned to typec-switch-states parameter.

Wrap lines at 80 char.

> +    maxItems: 1
> +
> +  typec-switch-states:
> +    description: An ordered u32 array describing the mux state value for each typec orientations.
> +      Three states correspond to NONE(high impedance), NORMAL, REVERSE respectively. If there is
> +      no HW mux state for NONE, use value of NORMAL or REVERSE for it. If this mux controller
> +      handle more than 1 switch, correct states value need to be caculated according to the mux
> +      settings.
> +    minItems: 3
> +    maxItems: 3
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

I think that 'mux-states' is what you want to use here.

Rob

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