lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <875yowfafu.fsf@bang-olufsen.dk>
Date:   Wed, 2 Mar 2022 18:58:45 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Rob Herring <robh@...nel.org>
CC:     Alvin Šipraga <alvin@...s.dk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] dt-bindings: usb: add TS5USBA224 USB/Audio switch mux

Rob Herring <robh@...nel.org> writes:

> On Tue, Mar 01, 2022 at 02:20:06PM +0100, Alvin Šipraga wrote:
>> From: Alvin Šipraga <alsi@...g-olufsen.dk>
>> 
>> The TS5USBA224 is a USB High Speed/Audio switch mux IC controlled via
>> GPIO. It is typically composed with a Type-C port controller with Audio
>> Accessory mode detection.
>> 
>> Signed-off-by: Alvin Šipraga <alsi@...g-olufsen.dk>
>> ---
>>  .../bindings/usb/ti,ts5usba224.yaml           | 56 +++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> new file mode 100644
>> index 000000000000..0a488b961906
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/ti,ts5usba224.yaml
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/ti,ts5usba224.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TS5USBA224 USB 2.0 High Speed and Audio mux DT bindings
>> +
>> +description:
>> +  The Texas Instruments TS5USBA224 is a double-pole, double throw
>> +  (DPDT) multiplexer that includes a low-distortion audio switch and a
>> +  USB 2.0 High Speed switch in the same package.
>> +
>> +maintainers:
>> +  - Alvin Šipraga <alsi@...g-olufsen.dk>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,ts5usba224
>> +
>> +  asel-gpio:
>> +    description: Output GPIO for A_SEL signal
>> +    maxItems: 1
>> +
>> +  accessory:
>> +    type: boolean
>> +    description:
>> +      Indicates that this is an Accessory Mode mux.
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description:
>> +      OF graph bindings modelling a Type-C port controller.
>
> What does that mean?
>
> There's a couple of Type-C bindings on the list ATM. Without block 
> diagrams of all the relevant components and the corresponding graph, I 
> can't say whether any of this graph usage makes sense.

Thanks for your review. Maybe I need some help with this but I'll try
and explain according to my understanding.

The series adds two drivers:

- tusb320xa - a USB Type-C port controller driver
- ts5usba224 - a USB Type-C multiplexer (for Audio Accessory mode)

These handle different classes of device: typec (the port controller)
and typec_mux (the multiplexer) respectively.

The typec driver is responsible for determining what is connected on the
other end of the Type-C port. It informs the typec subsystem of what
role is detected (UFP or DFP), the orientation, and whether or not
certain accessories (Audio or Debug) are connected.

The typec_mux driver is interested in whether an Audio Accessory has
been connected to the Type-C port. Depending on whether it is or not, it
will assert a GPIO controlling the TS5USBA224 mux chip.

Additionally, an OTG capable USB controller might like to be informed so
that it can configure itself correctly as a host (and enable VBUS etc.)
or peripheral.

Here is a simplified block diagram with my i.MX8MM SoC:

     ┌─────────────────────┐                    ┌───────────────────┐
     │                     │          I2C       │                   │
     │                     ├────────────────────┤ I2C               │
     │      TUSB320LAI     │         INT_N      │                   │
     │                     ├────────────────────┤         i.MX8MM   │
     │                     │                    │                   │
     │ CC1 CC2             │           ┌────────┤ GPIO              │
     └──┬───┬──────────────┘           │        │                   │
        │   │                          │        │       USBOTG      │
        │   │                          │        │    VBUS   D+  D-  │
        │   │                          │        └──────┬─────┬───┬──┘
        │   │   ┌──────────────────────│───────────────┘     │   │
        │   │   │          ┌───────────┴──┐                  │   │
        │   │   │          │        A_SEL │                  │   │
        │   │   │          │              │                  │   │
        │   │   │          │           D+ ├──────────────────┘   │
        │   │   │          │              │                      │
        │   │   │   ┌──────┤ D+/L      D- ├──────────────────────┘
        │   │   │   │      │              │
        │   │   │   │   ┌──┤ D-/R         │                 Line_L
        │   │   │   │   │  │            L ├───────────────────────
        ┴   ┴   ┴   ┴   ┴  │              │                 Line_R
         USB Type-C port   │            R ├───────────────────────
                           │              │
                           │  TS5USBA224  │
                           └──────────────┘


Line_{L,R} here are handled by some alternative circuitry which is not
relevant to this discussion.

Now back to the graph: I noticed while reading the generic typec code
that it is walking the child nodes and looking for particular endpoints
when registering a typec port. If a remote endpoint has the 'accessory'
property, it will pick up the corresponding device and inform it when
the port mode changes. If the new mode is TYPEC_MODE_AUDIO (as a result
of an Audio Accessory being detected by the port), then the typec_mux
driver will assert the A_SEL GPIO, otherwise it will deassert it.

Since this worked pretty automatically, my understanding was that this
was the correct way to "connect" these two devices in the device tree.

For the USB host/peripheral role switching, I use the
usb_role_switch_get() API in the probe function of the tusb320xa driver
to similarly walk the device tree and find a remote endpoint with the
'usb-role-switch' property.

So it could be a misunderstanding on my part, but what I'm trying to do
here with the bindings is offer the option to connect the various
devices together via the graph bindings. They are strictly optional
though - one might have a non-OTG capable USB controller, or perhaps not
care about Audio Accessories, etc. - so I did not specify specific port
numbering. I can of course do that, but it will be an arbitrary choice.

I hope that explains a bit better what I'm trying to do here. I don't
claim to have got it right though. Do you have any comments? Maybe
Heikki can chip in and explain if this is the right way to look at
things.

Kind regards,
Alvin

>
>> +
>> +required:
>> +  - compatible
>> +  - asel-gpio
>> +  - accessory
>> +  - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    usbaudiomux@0 {
>> +      compatible = "ti,ts5usba224";
>> +      asel-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
>> +      accessory;
>> +
>> +      port {
>> +        usb_audio_mux1: endpoint {
>> +          remote-endpoint = <&typec1_mux>;
>> +        };
>> +      };
>> +    };
>> -- 
>> 2.35.1
>> 

Powered by blists - more mailing lists