diff mbox series

[18/22] dt-bindings: chrome: Add binding for ChromeOS Pogo pin connector

Message ID 20240210070934.2549994-19-swboyd@chromium.org
State New
Headers
Series platform/chrome: Add DT USB/DP muxing/topology to Trogdor |

Commit Message

Stephen Boyd Feb. 10, 2024, 7:09 a.m. UTC 
  Describe the set of pins used to connect the detachable keyboard on
detachable ChromeOS devices. The set of pins is called the "pogo pins".
It's basically USB 2.0 with an extra pin for base detection. We expect
to find a keyboard on the other side of this connector with a specific
vid/pid, so describe that as a child device at the port of the usb
device connected upstream.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Pin-yen Lin <treapking@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../chrome/google,pogo-pin-connector.yaml     | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml

Comments

Krzysztof Kozlowski Feb. 11, 2024, 1:39 p.m. UTC | #1 
On 10/02/2024 08:09, Stephen Boyd wrote:
> Describe the set of pins used to connect the detachable keyboard on
> detachable ChromeOS devices. The set of pins is called the "pogo pins".
> It's basically USB 2.0 with an extra pin for base detection. We expect
> to find a keyboard on the other side of this connector with a specific
> vid/pid, so describe that as a child device at the port of the usb
> device connected upstream.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Pin-yen Lin <treapking@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../chrome/google,pogo-pin-connector.yaml     | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> 
> diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> new file mode 100644
> index 000000000000..5ba68fd95fcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/chrome/google,pogo-pin-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Pogo Pin Connector
> +
> +maintainers:
> +  - Stephen Boyd <swboyd@chromium.org>
> +

Missing description describing the hardware.

> +properties:
> +  compatible:
> +    const: google,pogo-pin-connector
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: Connection to USB2 port providing USB signals
> +    required:
> +      - endpoint

Drop required.


> +
> +patternProperties:
> +  "^keyboard@[0-9a-f]{1,2}$":
> +    description: The detachable keyboard

If this is detachable why do you define it in DT? Only hard-wired USB
devices, which need some sort of special handling. are described in DT.

> +    type: object
> +    $ref: /schemas/usb/usb-device.yaml

On this level:
unevaluatedProperties: false

> +
> +required:
> +  - compatible
> +  - '#address-cells'
> +  - '#size-cells'

Use one type of quotes, either ' or ".

> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    connector {
> +        compatible = "google,pogo-pin-connector";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        keyboard@2 {
> +          compatible = "usb18d1,504c";

Messed indentation.

> +          reg = <2>;
> +        };
> +
Best regards,
Krzysztof
Doug Anderson Feb. 14, 2024, 1:17 a.m. UTC | #2 
Hi,

On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Describe the set of pins used to connect the detachable keyboard on
> detachable ChromeOS devices. The set of pins is called the "pogo pins".
> It's basically USB 2.0 with an extra pin for base detection. We expect
> to find a keyboard on the other side of this connector with a specific
> vid/pid, so describe that as a child device at the port of the usb
> device connected upstream.

Can you remind me what the side effects would be if a different
VID/PID shows up there? I know it's not an end-user scenario, but I
have a pre-production "coachz" keyboard that's actually programmed
incorrectly and shows up as the wrong PID. Presumably I could either
throw the old hardware away or figure out a way to re-program it and
it's really not a big deal, but just curious what happens...


-Doug
Stephen Boyd Feb. 15, 2024, 12:07 a.m. UTC | #3 
Quoting Krzysztof Kozlowski (2024-02-11 05:39:36)
> On 10/02/2024 08:09, Stephen Boyd wrote:
> > diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> > new file mode 100644
> > index 000000000000..5ba68fd95fcd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
[...]
>
> > +properties:
> > +  compatible:
> > +    const: google,pogo-pin-connector
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description: Connection to USB2 port providing USB signals
> > +    required:
> > +      - endpoint
>
> Drop required.

Why? I'd like to make it so you can't have the node defined without
connecting it up to the rest of the system. Is that bad?

>
>
> > +
> > +patternProperties:
> > +  "^keyboard@[0-9a-f]{1,2}$":
> > +    description: The detachable keyboard
>
> If this is detachable why do you define it in DT? Only hard-wired USB
> devices, which need some sort of special handling. are described in DT.

From the commit text:

 We expect to find a keyboard on the other side of this connector with a
 specific vid/pid, so describe that as a child device at the port of the
 usb device connected upstream.

ChromeOS userspace is checking that the connected device downstream of
this port has the expected vid/pid to quickly rule out USB keyboards
that aren't the detachable keyboard. I wanted to express this in DT so
that it didn't live in ChromeOS userspace forever.
Stephen Boyd Feb. 15, 2024, 12:10 a.m. UTC | #4 
Quoting Doug Anderson (2024-02-13 17:17:34)
> Hi,
>
> On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Describe the set of pins used to connect the detachable keyboard on
> > detachable ChromeOS devices. The set of pins is called the "pogo pins".
> > It's basically USB 2.0 with an extra pin for base detection. We expect
> > to find a keyboard on the other side of this connector with a specific
> > vid/pid, so describe that as a child device at the port of the usb
> > device connected upstream.
>
> Can you remind me what the side effects would be if a different
> VID/PID shows up there? I know it's not an end-user scenario, but I
> have a pre-production "coachz" keyboard that's actually programmed
> incorrectly and shows up as the wrong PID. Presumably I could either
> throw the old hardware away or figure out a way to re-program it and
> it's really not a big deal, but just curious what happens...

As far as I know nothing happens besides ChromeOS userspace treats the
keyboard as "external" so things like smarter base detection, e.g.
wraparound keyboard detection or kickstand mode, may not work. I think
you get a popup box telling you the keyboard isn't the trusted one.
Krzysztof Kozlowski Feb. 15, 2024, 8:39 a.m. UTC | #5 
On 15/02/2024 01:07, Stephen Boyd wrote:
> Quoting Krzysztof Kozlowski (2024-02-11 05:39:36)
>> On 10/02/2024 08:09, Stephen Boyd wrote:
>>> diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
>>> new file mode 100644
>>> index 000000000000..5ba68fd95fcd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
>>> @@ -0,0 +1,61 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
> [...]
>>
>>> +properties:
>>> +  compatible:
>>> +    const: google,pogo-pin-connector
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +
>>> +  port:
>>> +    $ref: /schemas/graph.yaml#/properties/port
>>> +    description: Connection to USB2 port providing USB signals
>>> +    required:
>>> +      - endpoint
>>
>> Drop required.
> 
> Why? I'd like to make it so you can't have the node defined without
> connecting it up to the rest of the system. Is that bad?

Hm, I double checked and you're right. I thought endpoint is required
anyway by graph.yaml in dtschema, but it seems it is not.

> 
>>
>>
>>> +
>>> +patternProperties:
>>> +  "^keyboard@[0-9a-f]{1,2}$":
>>> +    description: The detachable keyboard
>>
>> If this is detachable why do you define it in DT? Only hard-wired USB
>> devices, which need some sort of special handling. are described in DT.
> 
> From the commit text:
> 
>  We expect to find a keyboard on the other side of this connector with a
>  specific vid/pid, so describe that as a child device at the port of the
>  usb device connected upstream.
> 
> ChromeOS userspace is checking that the connected device downstream of
> this port has the expected vid/pid to quickly rule out USB keyboards
> that aren't the detachable keyboard. I wanted to express this in DT so
> that it didn't live in ChromeOS userspace forever.

OK,

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
new file mode 100644
index 000000000000..5ba68fd95fcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
@@ -0,0 +1,61 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/chrome/google,pogo-pin-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Pogo Pin Connector
+
+maintainers:
+  - Stephen Boyd <swboyd@chromium.org>
+
+properties:
+  compatible:
+    const: google,pogo-pin-connector
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description: Connection to USB2 port providing USB signals
+    required:
+      - endpoint
+
+patternProperties:
+  "^keyboard@[0-9a-f]{1,2}$":
+    description: The detachable keyboard
+    type: object
+    $ref: /schemas/usb/usb-device.yaml
+
+required:
+  - compatible
+  - '#address-cells'
+  - '#size-cells'
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    connector {
+        compatible = "google,pogo-pin-connector";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        keyboard@2 {
+          compatible = "usb18d1,504c";
+          reg = <2>;
+        };
+
+        port {
+            pogo_connector_in: endpoint {
+                remote-endpoint = <&usb_hub_dsp3_hs>;
+            };
+        };
+    };
+
+...