[PATCH 0/2] usb: typec: fsa4480: Support reporting hotplug events to a codec

[PATCH 0/2] usb: typec: fsa4480: Support reporting hotplug events to a codec

[Val Packett]

Currently the fsa4480 USB-C SBU MUX is not actually used with audio
codecs in mainline. I have gotten it to work with the wcd937x codec
on sm7325-motorola-dubai (copying the Type-C notification handler
code from wcd939x to wcd937x, will be sent after this).

The fsa4480 driver changes Luca Weiss sent me were the only thing missing
for me on top of the wcd changes.. I've added a backwards-compatible
dt-bindings change here and it seems to pass dt_binding_check dtbs_check.

Thanks,
~val

---

Luca Weiss (1):
  usb: typec: fsa4480: Report switch & mux events to codec

Val Packett (1):
  dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

 .../devicetree/bindings/usb/fcs,fsa4480.yaml  | 90 +++++++++++++------
 drivers/usb/typec/mux/fsa4480.c               | 51 +++++++++--
 2 files changed, 108 insertions(+), 33 deletions(-)

-- 
2.55.0

[PATCH 1/2] dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

[Val Packett]

To actually support analog audio output, the fsa4480 driver needs to
deliver the hotplug notifications over the Type-C graph to an audio
codec driver. Describe a second port intended for the codec. Keep it
optional to make sure existing device trees do still validate.

Signed-off-by: Val Packett <val@packett.cool>
---
 .../devicetree/bindings/usb/fcs,fsa4480.yaml  | 90 +++++++++++++------
 1 file changed, 63 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
index 89b1fb90aeeb..9dfe95b6adf3 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
+++ b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
@@ -9,31 +9,11 @@ title: ON Semiconductor Analog Audio Switch
 maintainers:
   - Bjorn Andersson <bjorn.andersson@linaro.org>
 
-properties:
-  compatible:
-    oneOf:
-      - const: fcs,fsa4480
-      - items:
-          - enum:
-              - ocs,ocp96011
-          - const: fcs,fsa4480
-
-  reg:
-    maxItems: 1
-
-  interrupts:
-    maxItems: 1
-
-  vcc-supply:
-    description: power supply (2.7V-5.5V)
-
-  mode-switch: true
-  orientation-switch: true
-
-  port:
+$defs:
+  controller-port:
     $ref: /schemas/graph.yaml#/$defs/port-base
     description:
-      A port node to link the FSA4480 to a TypeC controller for the purpose of
+      A port node to link the FSA4480 to a Type-C controller for the purpose of
       handling altmode muxing and orientation switching.
     unevaluatedProperties: false
 
@@ -69,15 +49,56 @@ properties:
                   - AUX+ connected to SBU2
                   - AUX- connected to SBU1
 
+properties:
+  compatible:
+    oneOf:
+      - const: fcs,fsa4480
+      - items:
+          - enum:
+              - ocs,ocp96011
+          - const: fcs,fsa4480
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vcc-supply:
+    description: power supply (2.7V-5.5V)
+
+  mode-switch: true
+  orientation-switch: true
+
+  port:
+    $ref: "#/$defs/controller-port"
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    properties:
+      port@0:
+        $ref: "#/$defs/controller-port"
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description:
+          A port node to link the FSA4480 to an audio codec in order to
+          notify it whenever an analog audio adapter is plugged/unplugged.
+        unevaluatedProperties: false
+
 required:
   - compatible
   - reg
-  - port
 
 allOf:
   - $ref: usb-switch.yaml#
   - $ref: usb-switch-ports.yaml#
 
+oneOf:
+  - required:
+      - port
+  - required:
+      - ports
+
 additionalProperties: false
 
 examples:
@@ -98,9 +119,24 @@ examples:
             mode-switch;
             orientation-switch;
 
-            port {
-                fsa4480_ept: endpoint {
-                    remote-endpoint = <&typec_controller>;
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+
+                    fsa4480_sbu_mux: endpoint {
+                        remote-endpoint = <&typec_controller>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+
+                    fsa4480_audio: endpoint {
+                        remote-endpoint = <&audio_codec>;
+                    };
                 };
             };
         };
-- 
2.55.0

Re: [PATCH 1/2] dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: usb: fsa4480: conflicting usb-switch-ports.yaml ref in allOf
--

commit bdd98503313bd1c358f2a30a54c1cf8d1fe906dd
Author: Val Packett <val@packett.cool>

dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

This commit updates the fcs,fsa4480 binding to support a multi-port graph
layout so that hotplug notifications can be delivered to an audio codec
driver through a second port while keeping the single-port layout valid.

> diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> index 89b1fb90aeebc..9dfe95b6adf35 100644
> --- a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> +++ b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> @@ -9,31 +9,11 @@ title: ON Semiconductor Analog Audio Switch
>  maintainers:
>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>  
> -properties:
> -  compatible:
> -    oneOf:
> -      - const: fcs,fsa4480
> -      - items:
> -          - enum:
> -              - ocs,ocp96011
> -          - const: fcs,fsa4480
> -
> -  reg:
> -    maxItems: 1
> -
> -  interrupts:
> -    maxItems: 1
> -
> -  vcc-supply:
> -    description: power supply (2.7V-5.5V)
> -
> -  mode-switch: true
> -  orientation-switch: true
> -
> -  port:
> +$defs:
> +  controller-port:
>      $ref: /schemas/graph.yaml#/$defs/port-base
>      description:
> -      A port node to link the FSA4480 to a TypeC controller for the purpose of
> +      A port node to link the FSA4480 to a Type-C controller for the purpose of
>        handling altmode muxing and orientation switching.
>      unevaluatedProperties: false
>  
> @@ -69,15 +49,56 @@ properties:
>                    - AUX+ connected to SBU2
>                    - AUX- connected to SBU1
>  
> +properties:

[ ... ]

> +  port:
> +    $ref: "#/$defs/controller-port"
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    properties:
> +      port@0:
> +        $ref: "#/$defs/controller-port"
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description:
> +          A port node to link the FSA4480 to an audio codec in order to
> +          notify it whenever an analog audio adapter is plugged/unplugged.
> +        unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
> -  - port
>  
>  allOf:
>    - $ref: usb-switch.yaml#
>    - $ref: usb-switch-ports.yaml#
>  
> +oneOf:
> +  - required:
> +      - port
> +  - required:
> +      - ports

[Severity: Medium]
Does keeping usb-switch-ports.yaml# in allOf conflict with the custom ports
schema defined here?

In Documentation/devicetree/bindings/usb/usb-switch-ports.yaml, ports is
defined for SuperSpeed switches:

  ports:
    $ref: /schemas/graph.yaml#/properties/ports
    properties:
      port@0:
        $ref: /schemas/graph.yaml#/properties/port
        description:
          Super Speed (SS) Output endpoint to the Type-C connector

      port@1:
        $ref: /schemas/graph.yaml#/$defs/port-base
        description:
          Super Speed (SS) Input endpoint from the Super-Speed PHY
        ...
        properties:
          endpoint:
            $ref: /schemas/graph.yaml#/$defs/endpoint-base
            ...
            properties:
              data-lanes:
                minItems: 1
                maxItems: 8

Because allOf requires validation against both schemas, port@0 in
usb-switch-ports.yaml references /schemas/graph.yaml#/properties/port, which
enforces unevaluatedProperties: false on endpoint with no data-lanes property
(as noted in commit 11e80d371bbb ("dt-bindings: usb: fcs,fsa4480: Fix default
port schema reference")). That rejects valid data-lanes properties inside
ports/port@0/endpoint (such as on ocs,ocp96011 in
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts when converted to ports),
defeating #/$defs/controller-port on port@0.

At the same time, port@1 here uses /schemas/graph.yaml#/$defs/port-base
instead of /schemas/graph.yaml#/properties/port, which leaves port@1/endpoint
without unevaluatedProperties: false and inherits the SuperSpeed data-lanes
definition from usb-switch-ports.yaml, while ports does not require port@0.

Following commit bd8c3ce6d7a2 ("dt-bindings: usb: switch: split out ports
definition") for bindings with custom ports layouts, should
usb-switch-ports.yaml# be dropped from allOf, port@1 changed to reference
/schemas/graph.yaml#/properties/port, and required: [port@0] added under
ports?

> +
>  additionalProperties: false

[ ... ]

>
>
>

 base-commit: 90d9f76ccb72ec024ef476b4229f3dd1957d7b26
-- 
2.51.2

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018225906.1265299-1-val@packett.cool?part=1

[PATCH 2/2] usb: typec: fsa4480: Report switch & mux events to codec

[Val Packett]

From: Luca Weiss <luca.weiss@fairphone.com>

In the Type-C graph, the FSA4480 switch is in between the USB-C connector
and the audio codec, but the audio codec also needs to receive the
switch and mux events from the FSA4480.

Update the driver to get an Type-C mux on the next endpoint and
broadcast the recevied mux & switch state to it.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
[val: enable the SENSE/MIC bits used downstream, they do improve detection]
Signed-off-by: Val Packett <val@packett.cool>
---

Originaly in the WIP tree Luca sent me, the chunk with the two enable bits
was in a separate debug commit. I have tested the difference, without them
the codec does detect the headphones but complains in the process:

wcd937x_codec audio-codec: Impedance detect ramp error, c1=0, x1=0x0
wcd937x_codec audio-codec: Impedance detect ramp error, c1=0, x1=0x0
wcd937x_codec audio-codec: Impedance detect ramp error, c1=0, x1=0x0
wcd937x_codec audio-codec: wcd937x_mbhc_get_result_params: d1=30, c1=3, x1=0xb69, z_val=19776 (milliohm)

With FSA4480_ENABLE_SENSE | FSA4480_ENABLE_MIC set, there are no complaints,
the headset is detected on the first iteration:

wcd937x_codec audio-codec: wcd937x_mbhc_get_result_params: d1=5, c1=3, x1=0x2d31, z_val=33984 (milliohm)
wcd937x_codec audio-codec: wcd937x_mbhc_get_result_params: d1=5, c1=3, x1=0x2dc7, z_val=33380 (milliohm)
wcd937x_codec audio-codec: wcd937x_mbhc_get_result_params: d1=5, c1=3, x1=0x2d57, z_val=33829 (milliohm)

---
 drivers/usb/typec/mux/fsa4480.c | 54 ++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/typec/mux/fsa4480.c b/drivers/usb/typec/mux/fsa4480.c
index bea0c1deec94..d992a7e1887c 100644
--- a/drivers/usb/typec/mux/fsa4480.c
+++ b/drivers/usb/typec/mux/fsa4480.c
@@ -60,6 +60,9 @@ struct fsa4480 {
 
 	struct regmap *regmap;
 
+	struct typec_mux *codec;
+	struct typec_switch *codec_switch;
+
 	enum typec_orientation orientation;
 	unsigned long mode;
 	unsigned int svid;
@@ -115,7 +118,8 @@ static int fsa4480_set(struct fsa4480 *fsa)
 		}
 	} else if (fsa->mode == TYPEC_MODE_AUDIO) {
 		/* Audio Accessory Mode, setup to auto Jack Detection */
-		enable |= FSA4480_ENABLE_USB | FSA4480_ENABLE_AGND;
+		enable |= FSA4480_ENABLE_USB | FSA4480_ENABLE_AGND |
+			 FSA4480_ENABLE_SENSE | FSA4480_ENABLE_MIC;
 	} else
 		return -EOPNOTSUPP;
 
@@ -162,7 +166,11 @@ static int fsa4480_switch_set(struct typec_switch_dev *sw,
 
 	mutex_unlock(&fsa->lock);
 
-	return ret;
+	if (ret)
+		return ret;
+
+	/* Report orientation to codec after switch has been done */
+	return typec_switch_set(fsa->codec_switch, orientation);
 }
 
 static int fsa4480_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
@@ -185,7 +193,11 @@ static int fsa4480_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *st
 
 	mutex_unlock(&fsa->lock);
 
-	return ret;
+	if (ret)
+		return ret;
+
+	/* Report event to codec after switch has been done */
+	return typec_mux_set(fsa->codec, state);
 }
 
 enum {
@@ -287,6 +299,19 @@ static int fsa4480_probe(struct i2c_client *client)
 		FIELD_GET(FSA4480_DEVICE_ID_REV_ID, val),
 		FIELD_GET(FSA4480_DEVICE_ID_VENDOR_ID, val));
 
+	/* Get Codec's MUX & Switch devices */
+	fsa->codec = fwnode_typec_mux_get(dev->fwnode);
+	if (IS_ERR(fsa->codec))
+		return dev_err_probe(dev, PTR_ERR(fsa->codec),
+				     "failed to acquire codec mode-switch\n");
+
+	fsa->codec_switch = fwnode_typec_switch_get(dev->fwnode);
+	if (IS_ERR(fsa->codec_switch)) {
+		ret = dev_err_probe(dev, PTR_ERR(fsa->codec_switch),
+				    "failed to acquire codec orientation-switch\n");
+		goto err_put_mux;
+	}
+
 	/* Safe mode */
 	fsa->cur_enable = FSA4480_ENABLE_DEVICE | FSA4480_ENABLE_USB;
 	fsa->mode = TYPEC_STATE_SAFE;
@@ -310,8 +335,11 @@ static int fsa4480_probe(struct i2c_client *client)
 	sw_desc.set = fsa4480_switch_set;
 
 	fsa->sw = typec_switch_register(dev, &sw_desc);
-	if (IS_ERR(fsa->sw))
-		return dev_err_probe(dev, PTR_ERR(fsa->sw), "failed to register typec switch\n");
+	if (IS_ERR(fsa->sw)) {
+		ret = dev_err_probe(dev, PTR_ERR(fsa->sw),
+				    "failed to register typec switch\n");
+		goto err_put_switch;
+	}
 
 	mux_desc.drvdata = fsa;
 	mux_desc.fwnode = dev_fwnode(dev);
@@ -319,12 +347,24 @@ static int fsa4480_probe(struct i2c_client *client)
 
 	fsa->mux = typec_mux_register(dev, &mux_desc);
 	if (IS_ERR(fsa->mux)) {
-		typec_switch_unregister(fsa->sw);
-		return dev_err_probe(dev, PTR_ERR(fsa->mux), "failed to register typec mux\n");
+		ret = dev_err_probe(dev, PTR_ERR(fsa->mux),
+				    "failed to register typec mux\n");
+		goto err_unregister_switch;
 	}
 
 	i2c_set_clientdata(client, fsa);
 	return 0;
+
+err_unregister_switch:
+	typec_switch_unregister(fsa->sw);
+
+err_put_switch:
+	typec_switch_put(fsa->codec_switch);
+
+err_put_mux:
+	typec_mux_put(fsa->codec);
+
+	return ret;
 }
 
 static void fsa4480_remove(struct i2c_client *client)
-- 
2.55.0

Re: [PATCH 2/2] usb: typec: fsa4480: Report switch & mux events to codec

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: typec: fsa4480: leak of codec switch and mux in fsa4480_remove()
--

commit ec3a8ea75d1b120f5c00db8c4658dd7e1e5f7c65
Author: Luca Weiss <luca.weiss@fairphone.com>

usb: typec: fsa4480: Report switch & mux events to codec

This commit updates the fsa4480 driver to acquire a Type-C mux and
orientation switch for the connected audio codec and forward switch and
mux state updates to it. It also enables the SENSE and MIC bits when
entering audio accessory mode.

> diff --git a/drivers/usb/typec/mux/fsa4480.c b/drivers/usb/typec/mux/fsa4480.c
> index bea0c1deec94d..d992a7e1887c8 100644
> --- a/drivers/usb/typec/mux/fsa4480.c
> +++ b/drivers/usb/typec/mux/fsa4480.c

[ ... ]

> @@ -287,6 +299,19 @@ static int fsa4480_probe(struct i2c_client *client)
>  		FIELD_GET(FSA4480_DEVICE_ID_REV_ID, val),
>  		FIELD_GET(FSA4480_DEVICE_ID_VENDOR_ID, val));
>  
> +	/* Get Codec's MUX & Switch devices */
> +	fsa->codec = fwnode_typec_mux_get(dev->fwnode);
> +	if (IS_ERR(fsa->codec))
> +		return dev_err_probe(dev, PTR_ERR(fsa->codec),
> +				     "failed to acquire codec mode-switch\n");
> +
> +	fsa->codec_switch = fwnode_typec_switch_get(dev->fwnode);
> +	if (IS_ERR(fsa->codec_switch)) {
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->codec_switch),
> +				    "failed to acquire codec orientation-switch\n");
> +		goto err_put_mux;
> +	}
> +
>  	/* Safe mode */
>  	fsa->cur_enable = FSA4480_ENABLE_DEVICE | FSA4480_ENABLE_USB;
>  	fsa->mode = TYPEC_STATE_SAFE;
> @@ -310,8 +335,11 @@ static int fsa4480_probe(struct i2c_client *client)
>  	sw_desc.set = fsa4480_switch_set;
>  
>  	fsa->sw = typec_switch_register(dev, &sw_desc);
> -	if (IS_ERR(fsa->sw))
> -		return dev_err_probe(dev, PTR_ERR(fsa->sw), "failed to register typec switch\n");
> +	if (IS_ERR(fsa->sw)) {
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->sw),
> +				    "failed to register typec switch\n");
> +		goto err_put_switch;
> +	}
>  
>  	mux_desc.drvdata = fsa;
>  	mux_desc.fwnode = dev_fwnode(dev);
> @@ -319,12 +347,24 @@ static int fsa4480_probe(struct i2c_client *client)
>  
>  	fsa->mux = typec_mux_register(dev, &mux_desc);
>  	if (IS_ERR(fsa->mux)) {
> -		typec_switch_unregister(fsa->sw);
> -		return dev_err_probe(dev, PTR_ERR(fsa->mux), "failed to register typec mux\n");
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->mux),
> +				    "failed to register typec mux\n");
> +		goto err_unregister_switch;
>  	}
>  
>  	i2c_set_clientdata(client, fsa);
>  	return 0;
> +
> +err_unregister_switch:
> +	typec_switch_unregister(fsa->sw);
> +
> +err_put_switch:
> +	typec_switch_put(fsa->codec_switch);
> +
> +err_put_mux:
> +	typec_mux_put(fsa->codec);
> +
> +	return ret;
>  }
>  
>  static void fsa4480_remove(struct i2c_client *client)

[Severity: Medium]
Does fsa4480_remove() leak fsa->codec_switch and fsa->codec when the
device is unbound or the driver module is unloaded?

In fsa4480_probe(), fwnode_typec_mux_get() and fwnode_typec_switch_get()
allocate the wrapper structures and take device and module references on
the codec provider, which are not managed by devres. While the error path
in fsa4480_probe() cleans them up, fsa4480_remove() still only unregisters
fsa->mux and fsa->sw:

static void fsa4480_remove(struct i2c_client *client)
{
	struct fsa4480 *fsa = i2c_get_clientdata(client);

	typec_mux_unregister(fsa->mux);
	typec_switch_unregister(fsa->sw);
}

Should fsa4480_remove() also call typec_switch_put(fsa->codec_switch) and
typec_mux_put(fsa->codec)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018225906.1265299-1-val@packett.cool?part=2