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

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

[kernel test robot]

Hi Xu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on shawnguo/for-next linus/master v6.0-rc2 next-20220822]
[cannot apply to robh/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xu-Yang/typec-orientation-switch-support-via-mux-controller/20220822-153600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220822/202208221924.BfrOX4DZ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xu-Yang/typec-orientation-switch-support-via-mux-controller/20220822-153600
        git checkout 703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/usb/typec/mux.o: in function `typec_switch_put':
>> drivers/usb/typec/mux.c:200: undefined reference to `mux_control_put'
   ld: drivers/usb/typec/mux.o: in function `typec_switch_set':
>> drivers/usb/typec/mux.c:286: undefined reference to `mux_control_deselect'
   ld: drivers/usb/typec/mux.o: in function `mux_control_typec_switch_get':
>> drivers/usb/typec/mux.c:139: undefined reference to `mux_control_get'
   ld: drivers/usb/typec/mux.o: in function `mux_control_select':
>> include/linux/mux/consumer.h:34: undefined reference to `mux_control_select_delay'


vim +200 drivers/usb/typec/mux.c

   125	
   126	static struct typec_switch *mux_control_typec_switch_get(struct device *dev)
   127	{
   128		struct typec_switch *sw;
   129		struct mux_control *mux;
   130		int ret;
   131	
   132		if (!device_property_present(dev, "mux-controls"))
   133			return NULL;
   134	
   135		sw = kzalloc(sizeof(*sw), GFP_KERNEL);
   136		if (!sw)
   137			return ERR_PTR(-ENOMEM);
   138	
 > 139		mux = mux_control_get(dev, NULL);
   140		if (!IS_ERR(mux)) {
   141			sw->mux_switch = mux;
   142			ret = device_property_read_u32_array(dev,
   143				"typec-switch-states", sw->mux_states, 3);
   144			if (ret) {
   145				kfree(sw);
   146				return ERR_PTR(ret);
   147			}
   148		} else {
   149			kfree(sw);
   150			return ERR_CAST(mux);
   151		}
   152	
   153		return sw;
   154	}
   155	
   156	/**
   157	 * typec_switch_get - Find USB Type-C orientation switch
   158	 * @dev: The device using switch
   159	 *
   160	 * Finds a switch used by @dev. Returns a reference to the switch on
   161	 * success, NULL if no matching connection was found, or
   162	 * ERR_PTR(-EPROBE_DEFER) when a connection was found but the switch
   163	 * has not been enumerated yet, or ERR_PTR with a negative errno.
   164	 */
   165	struct typec_switch *typec_switch_get(struct device *dev)
   166	{
   167		struct typec_switch *sw;
   168	
   169		sw = fwnode_typec_switch_get(dev_fwnode(dev));
   170		if (!sw)
   171			/* Try get switch based on mux control */
   172			sw = mux_control_typec_switch_get(dev);
   173	
   174		return sw;
   175	}
   176	EXPORT_SYMBOL_GPL(typec_switch_get);
   177	
   178	/**
   179	 * typec_switch_put - Release USB Type-C orientation switch
   180	 * @sw: USB Type-C orientation switch
   181	 *
   182	 * Decrement reference count for @sw.
   183	 */
   184	void typec_switch_put(struct typec_switch *sw)
   185	{
   186		struct typec_switch_dev *sw_dev;
   187		unsigned int i;
   188	
   189		if (IS_ERR_OR_NULL(sw))
   190			return;
   191	
   192		for (i = 0; i < sw->num_sw_devs; i++) {
   193			sw_dev = sw->sw_devs[i];
   194	
   195			module_put(sw_dev->dev.parent->driver->owner);
   196			put_device(&sw_dev->dev);
   197		}
   198	
   199		if (sw->mux_switch)
 > 200			mux_control_put(sw->mux_switch);
   201	
   202		kfree(sw);
   203	}
   204	EXPORT_SYMBOL_GPL(typec_switch_put);
   205	
   206	static void typec_switch_release(struct device *dev)
   207	{
   208		kfree(to_typec_switch_dev(dev));
   209	}
   210	
   211	const struct device_type typec_switch_dev_type = {
   212		.name = "orientation_switch",
   213		.release = typec_switch_release,
   214	};
   215	
   216	/**
   217	 * typec_switch_register - Register USB Type-C orientation switch
   218	 * @parent: Parent device
   219	 * @desc: Orientation switch description
   220	 *
   221	 * This function registers a switch that can be used for routing the correct
   222	 * data pairs depending on the cable plug orientation from the USB Type-C
   223	 * connector to the USB controllers. USB Type-C plugs can be inserted
   224	 * right-side-up or upside-down.
   225	 */
   226	struct typec_switch_dev *
   227	typec_switch_register(struct device *parent,
   228			      const struct typec_switch_desc *desc)
   229	{
   230		struct typec_switch_dev *sw_dev;
   231		int ret;
   232	
   233		if (!desc || !desc->set)
   234			return ERR_PTR(-EINVAL);
   235	
   236		sw_dev = kzalloc(sizeof(*sw_dev), GFP_KERNEL);
   237		if (!sw_dev)
   238			return ERR_PTR(-ENOMEM);
   239	
   240		sw_dev->set = desc->set;
   241	
   242		device_initialize(&sw_dev->dev);
   243		sw_dev->dev.parent = parent;
   244		sw_dev->dev.fwnode = desc->fwnode;
   245		sw_dev->dev.class = &typec_mux_class;
   246		sw_dev->dev.type = &typec_switch_dev_type;
   247		sw_dev->dev.driver_data = desc->drvdata;
   248		ret = dev_set_name(&sw_dev->dev, "%s-switch", desc->name ? desc->name : dev_name(parent));
   249		if (ret) {
   250			put_device(&sw_dev->dev);
   251			return ERR_PTR(ret);
   252		}
   253	
   254		ret = device_add(&sw_dev->dev);
   255		if (ret) {
   256			dev_err(parent, "failed to register switch (%d)\n", ret);
   257			put_device(&sw_dev->dev);
   258			return ERR_PTR(ret);
   259		}
   260	
   261		return sw_dev;
   262	}
   263	EXPORT_SYMBOL_GPL(typec_switch_register);
   264	
   265	int typec_switch_set(struct typec_switch *sw,
   266			     enum typec_orientation orientation)
   267	{
   268		struct typec_switch_dev *sw_dev;
   269		struct mux_control *mux;
   270		unsigned int i;
   271		int ret;
   272	
   273		if (IS_ERR_OR_NULL(sw))
   274			return 0;
   275	
   276		for (i = 0; i < sw->num_sw_devs; i++) {
   277			sw_dev = sw->sw_devs[i];
   278	
   279			ret = sw_dev->set(sw_dev, orientation);
   280			if (ret)
   281				return ret;
   282		}
   283	
   284		mux = sw->mux_switch;
   285		if (mux) {
 > 286			ret = mux_control_deselect(mux);
   287			if (ret)
   288				return ret;
   289	
   290			ret = mux_control_select(mux, sw->mux_states[orientation]);
   291			if (ret)
   292				return ret;
   293		}
   294	
   295		return 0;
   296	}
   297	EXPORT_SYMBOL_GPL(typec_switch_set);
   298	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[PATCH 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: allow get mux_control from fwnode if of_node is NULL
  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  | 120 ++++++++++++++++++
 drivers/mux/core.c                            |   3 +
 drivers/usb/typec/mux.c                       |  74 +++++++++++
 include/linux/usb/typec_mux.h                 |   7 +-
 5 files changed, 217 insertions(+), 5 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 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>
---
 .../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 2/4] mux: allow get mux_control from fwnode if of_node is NULL

[Xu Yang]

Since some devices may link fwnode to dev but doesn't link of_node,
so here we could get mux_control from fwnode again.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/mux/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 49bedbe6316c..06d4e232e056 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -538,6 +538,9 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 	int index = 0;
 	int ret;
 
+	if (!np)
+		np = to_of_node(dev_fwnode(dev));
+
 	if (mux_name) {
 		if (state)
 			index = of_property_match_string(np, "mux-state-names",
-- 
2.34.1


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

[PATCH 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.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/mux.c       | 74 +++++++++++++++++++++++++++++++++++
 include/linux/usb/typec_mux.h |  7 +---
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 464330776cd6..5ee960fb668d 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -13,6 +13,7 @@
 #include <linux/mutex.h>
 #include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/mux/consumer.h>
 
 #include "class.h"
 #include "mux.h"
@@ -22,6 +23,11 @@
 struct typec_switch {
 	struct typec_switch_dev *sw_devs[TYPEC_MUX_MAX_DEVS];
 	unsigned int num_sw_devs;
+
+	/* Could handle HighSpeed, SuperSpeed, Sideband switch one time */
+	struct mux_control *mux_switch;
+	/* 3 state correspond to NONE, NORMAL, REVERSE for all switches */
+	int mux_states[3];
 };
 
 static int switch_fwnode_match(struct device *dev, const void *fwnode)
@@ -117,6 +123,58 @@ struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode)
 }
 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,6 +281,17 @@ int typec_switch_set(struct typec_switch *sw,
 			return ret;
 	}
 
+	mux = sw->mux_switch;
+	if (mux) {
+		ret = mux_control_deselect(mux);
+		if (ret)
+			return ret;
+
+		ret = mux_control_select(mux, sw->mux_states[orientation]);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(typec_switch_set);
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
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 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>
---
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 120 +++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f6b017ab5f53..bf0fc6b18164 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
 #include "imx8mp.dtsi"
 
 / {
@@ -65,6 +66,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 = <2>;
+
+		port {
+			usb3_data_ss: endpoint {
+				remote-endpoint = <&typec_con_ss>;
+			};
+		};
+	};
 };
 
 &A53_0 {
@@ -299,6 +316,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 +428,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 +589,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 +635,18 @@ 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
+		>;
+	};
+
 	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

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

[kernel test robot]

Hi Xu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on shawnguo/for-next linus/master v6.0-rc2 next-20220822]
[cannot apply to robh/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xu-Yang/typec-orientation-switch-support-via-mux-controller/20220822-153600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-a013-20220822 (https://download.01.org/0day-ci/archive/20220822/202208222312.IFf74Ze6-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xu-Yang/typec-orientation-switch-support-via-mux-controller/20220822-153600
        git checkout 703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/usb/typec/mux.o: in function `typec_switch_put':
>> mux.c:(.text+0x21d): undefined reference to `mux_control_put'
   ld: drivers/usb/typec/mux.o: in function `typec_switch_set':
>> mux.c:(.text+0x29a): undefined reference to `mux_control_deselect'
>> ld: mux.c:(.text+0x2ab): undefined reference to `mux_control_select_delay'
   ld: drivers/usb/typec/mux.o: in function `typec_switch_get':
>> mux.c:(.text+0x767): undefined reference to `mux_control_get'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

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

[Xu Yang]

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: Monday, August 22, 2022 11:47 PM
> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com;
> robh+dt@kernel.org; peda@axentia.se; shawnguo@kernel.org
> Cc: kbuild-all@lists.01.org; gregkh@linuxfoundation.org; linux@roeck-us.net;
> Jun Li <jun.li@nxp.com>; Xu Yang <xu.yang_2@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 3/4] usb: typec: mux: add typec orientation switch
> support via mux controller
> 
> Caution: EXT Email
> 
> Hi Xu,
> 
> Thank you for the patch! Yet something to improve:

Thank you, test robot! I will fix it and submit a v2 for it.

Xu Yang

> 
> [auto build test ERROR on usb/usb-testing] [also build test ERROR on
> shawnguo/for-next linus/master v6.0-rc2 next-20220822] [cannot apply to
> robh/for-next] [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-
> scm.com%2Fdocs%2Fgit-format-
> patch%23_base_tree_information&amp;data=05%7C01%7Cxu.yang_2%40nx
> p.com%7Cbfc9a4dc76fb43d05f7108da8455c2ab%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C637967801123509493%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C3000%7C%7C%7C&amp;sdata=1F2u5Ljc%2FxLAOzsnN9945e%2FeRl
> AVm5kMo1gLNbhUCjo%3D&amp;reserved=0]
> 
> url:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FXu-Yang%2Ftypec-
> orientation-switch-support-via-mux-controller%2F20220822-
> 153600&amp;data=05%7C01%7Cxu.yang_2%40nxp.com%7Cbfc9a4dc76fb43d
> 05f7108da8455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 37967801123509493%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
> &amp;sdata=f52r9hf0%2F5tfiSwZEjriijZ4da2XO7A8r8V9FjCRG%2BA%3D&amp
> ;reserved=0
> base:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fgregkh%2Fusb.git&am
> p;data=05%7C01%7Cxu.yang_2%40nxp.com%7Cbfc9a4dc76fb43d05f7108da8
> 455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63796780112
> 3509493%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata
> =MDXVH0E9FZ4zZBJkVeG5KuWoUqxl7lVDIbtdLcayD6Q%3D&amp;reserved=
> 0 usb-testing
> config: i386-randconfig-a013-20220822
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdow
> nload.01.org%2F0day-ci%2Farchive%2F20220822%2F202208222312.IFf74Ze6-
> lkp%40intel.com%2Fconfig&amp;data=05%7C01%7Cxu.yang_2%40nxp.com%
> 7Cbfc9a4dc76fb43d05f7108da8455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C637967801123665718%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000%7C%7C%7C&amp;sdata=oZ%2FSbZ5Rp%2Bhj9l1x0fJPW1dIZ7p5ezqZ
> Y4ouw8EpmC4%3D&amp;reserved=0)
> compiler: gcc-11 (Debian 11.3.0-5) 11.3.0 reproduce (this is a W=1 build):
>         #
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Fintel-lab-
> lkp%2Flinux%2Fcommit%2F703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd&am
> p;data=05%7C01%7Cxu.yang_2%40nxp.com%7Cbfc9a4dc76fb43d05f7108da8
> 455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63796780112
> 3665718%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata
> =6RJFht%2FkdfQ4domUP29WJC5m515k2RkAswDRQqwL85s%3D&amp;reserv
> ed=0
>         git remote add linux-review
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> b.com%2Fintel-lab-
> lkp%2Flinux&amp;data=05%7C01%7Cxu.yang_2%40nxp.com%7Cbfc9a4dc76f
> b43d05f7108da8455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C637967801123665718%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
> %7C&amp;sdata=wJeCdRdZ8UB0rHYQb1xEIlnOdqhJs53I%2BMM4%2FXvo8B
> w%3D&amp;reserved=0
>         git fetch --no-tags linux-review Xu-Yang/typec-orientation-switch-
> support-via-mux-controller/20220822-153600
>         git checkout 703ba3cfec5b6f9422ac9a859bc6121f7c4a12fd
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    ld: drivers/usb/typec/mux.o: in function `typec_switch_put':
> >> mux.c:(.text+0x21d): undefined reference to `mux_control_put'
>    ld: drivers/usb/typec/mux.o: in function `typec_switch_set':
> >> mux.c:(.text+0x29a): undefined reference to `mux_control_deselect'
> >> ld: mux.c:(.text+0x2ab): undefined reference to
> `mux_control_select_delay'
>    ld: drivers/usb/typec/mux.o: in function `typec_switch_get':
> >> mux.c:(.text+0x767): undefined reference to `mux_control_get'
> 
> --
> 0-DAY CI Kernel Test Service
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F01.or
> g%2Flkp&amp;data=05%7C01%7Cxu.yang_2%40nxp.com%7Cbfc9a4dc76fb43
> d05f7108da8455c2ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 637967801123665718%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7
> C&amp;sdata=915lo2yL7e5Zk0SBUqNX1TRvqNvAx3A2wCMVgho%2Fblk%3D
> &amp;reserved=0

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

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

[Peter Rosin]

Hi!

2022-08-22 at 17:35, 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.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/typec/mux.c       | 74 +++++++++++++++++++++++++++++++++++
>  include/linux/usb/typec_mux.h |  7 +---
>  2 files changed, 76 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 464330776cd6..5ee960fb668d 100644
> --- a/drivers/usb/typec/mux.c
> +++ b/drivers/usb/typec/mux.c
> @@ -13,6 +13,7 @@
>  #include <linux/mutex.h>
>  #include <linux/property.h>
>  #include <linux/slab.h>
> +#include <linux/mux/consumer.h>
>  
>  #include "class.h"
>  #include "mux.h"
> @@ -22,6 +23,11 @@
>  struct typec_switch {
>  	struct typec_switch_dev *sw_devs[TYPEC_MUX_MAX_DEVS];
>  	unsigned int num_sw_devs;
> +
> +	/* Could handle HighSpeed, SuperSpeed, Sideband switch one time */
> +	struct mux_control *mux_switch;
> +	/* 3 state correspond to NONE, NORMAL, REVERSE for all switches */
> +	int mux_states[3];
>  };
>  
>  static int switch_fwnode_match(struct device *dev, const void *fwnode)
> @@ -117,6 +123,58 @@ struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode)
>  }
>  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,6 +281,17 @@ int typec_switch_set(struct typec_switch *sw,
>  			return ret;
>  	}
>  
> +	mux = sw->mux_switch;
> +	if (mux) {
> +		ret = mux_control_deselect(mux);

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

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

Cheers,
Peter

> +		if (ret)
> +			return ret;
> +
> +		ret = mux_control_select(mux, sw->mux_states[orientation]);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(typec_switch_set);
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> 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 2/4] mux: allow get mux_control from fwnode if of_node is NULL

[Peter Rosin]

Hi!

2022-08-22 at 17:35, Xu Yang wrote:
> Since some devices may link fwnode to dev but doesn't link of_node,
> so here we could get mux_control from fwnode again.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/mux/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 49bedbe6316c..06d4e232e056 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -538,6 +538,9 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  	int index = 0;
>  	int ret;
>  
> +	if (!np)
> +		np = to_of_node(dev_fwnode(dev));
> +
>  	if (mux_name) {
>  		if (state)
>  			index = of_property_match_string(np, "mux-state-names",

This feels like a band aid. Is it not possible to convert the whole thing to
the fwnode interface?

Cheers,
Peter

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

RE: [EXT] Re: [PATCH 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: Tuesday, August 23, 2022 2:13 PM
> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com;
> robh+dt@kernel.org; shawnguo@kernel.org
> Cc: gregkh@linuxfoundation.org; linux@roeck-us.net; Jun Li
> <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 3/4] usb: typec: mux: add typec orientation switch
> support via mux controller
> 
> Caution: EXT Email
> 
> Hi!
> 
> 2022-08-22 at 17:35, Xu Yang wrote:
> > Some dedicated mux block can use existing mux controller as a mux
> > provider, typec port as a consumer to select channel for orientation
> > switch, this can be an alternate way to control typec orientation switch.
> > Also, one mux controller could cover highspeed, superspeed and sideband
> > use case one time in this implementation.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/typec/mux.c       | 74
> +++++++++++++++++++++++++++++++++++
> >  include/linux/usb/typec_mux.h |  7 +---
> >  2 files changed, 76 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> > index 464330776cd6..5ee960fb668d 100644
> > --- a/drivers/usb/typec/mux.c
> > +++ b/drivers/usb/typec/mux.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/mutex.h>
> >  #include <linux/property.h>
> >  #include <linux/slab.h>
> > +#include <linux/mux/consumer.h>
> >
> >  #include "class.h"
> >  #include "mux.h"
> > @@ -22,6 +23,11 @@
> >  struct typec_switch {
> >       struct typec_switch_dev *sw_devs[TYPEC_MUX_MAX_DEVS];
> >       unsigned int num_sw_devs;
> > +
> > +     /* Could handle HighSpeed, SuperSpeed, Sideband switch one time */
> > +     struct mux_control *mux_switch;
> > +     /* 3 state correspond to NONE, NORMAL, REVERSE for all switches */
> > +     int mux_states[3];
> >  };
> >
> >  static int switch_fwnode_match(struct device *dev, const void *fwnode)
> > @@ -117,6 +123,58 @@ struct typec_switch
> *fwnode_typec_switch_get(struct fwnode_handle *fwnode)
> >  }
> >  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,6 +281,17 @@ int typec_switch_set(struct typec_switch *sw,
> >                       return ret;
> >       }
> >
> > +     mux = sw->mux_switch;
> > +     if (mux) {
> > +             ret = mux_control_deselect(mux);
> 
> This is broken. Please read the documentation for mux_control_select and
> mux_control_deselect. Every call to mux_control_deselect *must* be paired
> with a *successful* call to mux_control_select. Here, mux_control_deselect
> is called unconditionally (as long as a mux is configured).

Okay, I will improve it in v2.

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

Understood. I'll follow the rules. 

Thanks,
Xu Yang

> Cheers,
> Peter
> 
> > +             if (ret)
> > +                     return ret;
> > +
> > +             ret = mux_control_select(mux, sw->mux_states[orientation]);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> >       return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_switch_set);
> > diff --git a/include/linux/usb/typec_mux.h
> b/include/linux/usb/typec_mux.h
> > 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 2/4] mux: allow get mux_control from fwnode if of_node is NULL

[Xu Yang]

Hi Peter,

> -----Original Message-----
> From: Peter Rosin <peda@axentia.se>
> Sent: Tuesday, August 23, 2022 2:23 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 2/4] mux: allow get mux_control from fwnode if
> of_node is NULL
> 
> Caution: EXT Email
> 
> Hi!
> 
> 2022-08-22 at 17:35, Xu Yang wrote:
> > Since some devices may link fwnode to dev but doesn't link of_node, so
> > here we could get mux_control from fwnode again.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/mux/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mux/core.c b/drivers/mux/core.c index
> > 49bedbe6316c..06d4e232e056 100644
> > --- a/drivers/mux/core.c
> > +++ b/drivers/mux/core.c
> > @@ -538,6 +538,9 @@ static struct mux_control *mux_get(struct device
> *dev, const char *mux_name,
> >       int index = 0;
> >       int ret;
> >
> > +     if (!np)
> > +             np = to_of_node(dev_fwnode(dev));
> > +
> >       if (mux_name) {
> >               if (state)
> >                       index = of_property_match_string(np,
> > "mux-state-names",
> 
> This feels like a band aid. Is it not possible to convert the whole thing to the
> fwnode interface?
> 

Agreed. Actually, I'm hesitate to convert to fwnode interface due to not have a
whole picture of all the mux controller users before. But now it should be okay to
do so since all the existing drivers get mux controller based on platform device after
my check. So, I will try to convert the whole thing to the fwnode interface in v2.

Thanks,
Xu Yang

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