diff mbox series

[v3,1/7] dt-bindings: connector: add GE SUNH hotplug addon connector

Message ID 20240809-hotplug-drm-bridge-v3-1-b4c178380bc9@bootlin.com (mailing list archive)
State New, archived
Headers show
Series Add support for GE SUNH hot-pluggable connector | expand

Commit Message

Luca Ceresoli Aug. 9, 2024, 3:34 p.m. UTC
Add bindings for the GE SUNH add-on connector. This is a physical,
hot-pluggable connector that allows to attach and detach at runtime an
add-on adding peripherals on non-discoverable busses.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changed in v3:
 - change the layout to only add subnodes, not properties
 - add the 'nobus-devices' node description to hold devices not on any bus
 - add 'i2c-*' nodes for the I2C busses, using a i2c-parent phandle
 - and the 'dsi' node for the DSI bus
 - move the entire port@1 node to the overlay (not only the remote-endpoint
   property)
 - remove the overlay examples (Overlays in examples are not supported)
 - add more clarifying descriptions and comments for examples
 - some rewording

This patch was added in v2.
---
 .../connector/ge,sunh-addon-connector.yaml         | 185 +++++++++++++++++++++
 MAINTAINERS                                        |   5 +
 2 files changed, 190 insertions(+)

Comments

Rob Herring (Arm) Aug. 9, 2024, 4:33 p.m. UTC | #1
On Fri, 09 Aug 2024 17:34:49 +0200, Luca Ceresoli wrote:
> Add bindings for the GE SUNH add-on connector. This is a physical,
> hot-pluggable connector that allows to attach and detach at runtime an
> add-on adding peripherals on non-discoverable busses.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changed in v3:
>  - change the layout to only add subnodes, not properties
>  - add the 'nobus-devices' node description to hold devices not on any bus
>  - add 'i2c-*' nodes for the I2C busses, using a i2c-parent phandle
>  - and the 'dsi' node for the DSI bus
>  - move the entire port@1 node to the overlay (not only the remote-endpoint
>    property)
>  - remove the overlay examples (Overlays in examples are not supported)
>  - add more clarifying descriptions and comments for examples
>  - some rewording
> 
> This patch was added in v2.
> ---
>  .../connector/ge,sunh-addon-connector.yaml         | 185 +++++++++++++++++++++
>  MAINTAINERS                                        |   5 +
>  2 files changed, 190 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dtb: /: 'compatible' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dtb: /: '#address-cells' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.example.dtb: /: '#size-cells' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240809-hotplug-drm-bridge-v3-1-b4c178380bc9@bootlin.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Luca Ceresoli Aug. 13, 2024, 9:06 a.m. UTC | #2
Hello,

On Fri, 09 Aug 2024 17:34:49 +0200
Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:

> Add bindings for the GE SUNH add-on connector. This is a physical,
> hot-pluggable connector that allows to attach and detach at runtime an
> add-on adding peripherals on non-discoverable busses.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

...

> +examples:
> +  # This is the description of the connector as it should appear in the
> +  # main DTS describing the "main" board up to the connector.

The comment from now on has outdated content referring to v2 and which I
forgot to remove before sending v3:

>      This is
> +  # supposed to be used together with the overlays in the two following
> +  # examples. The addon_connector and hotplug_conn_dsi_out labels are
> +  # referenced by the overlays in those examples.

Fixed locally.

> +  - |
> +    / {
> +        #include <dt-bindings/gpio/gpio.h>

I also removed the root node which is not needed and cause bot warnings,
and with that done the #include went back to a correct position.

The above are queued for v4.

Luca
Rob Herring (Arm) Aug. 13, 2024, 3:19 p.m. UTC | #3
On Fri, Aug 09, 2024 at 05:34:49PM +0200, Luca Ceresoli wrote:
> Add bindings for the GE SUNH add-on connector. This is a physical,
> hot-pluggable connector that allows to attach and detach at runtime an
> add-on adding peripherals on non-discoverable busses.

Overall, looks pretty good.

> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changed in v3:
>  - change the layout to only add subnodes, not properties
>  - add the 'nobus-devices' node description to hold devices not on any bus
>  - add 'i2c-*' nodes for the I2C busses, using a i2c-parent phandle
>  - and the 'dsi' node for the DSI bus
>  - move the entire port@1 node to the overlay (not only the remote-endpoint
>    property)
>  - remove the overlay examples (Overlays in examples are not supported)
>  - add more clarifying descriptions and comments for examples
>  - some rewording
> 
> This patch was added in v2.
> ---
>  .../connector/ge,sunh-addon-connector.yaml         | 185 +++++++++++++++++++++
>  MAINTAINERS                                        |   5 +
>  2 files changed, 190 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
> new file mode 100644
> index 000000000000..2a0b4e0fd089
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
> @@ -0,0 +1,185 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/ge,sunh-addon-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GE SUNH hotplug add-on connector
> +
> +maintainers:
> +  - Luca Ceresoli <luca.ceresoli@bootlin.com>
> +
> +description:

You need '|' or '>' to preserve line breaks. Elsewhere too.

> +  Represent the physical connector present on GE SUNH devices that allows
> +  to attach and detach at runtime an add-on adding peripherals on
> +  non-discoverable busses. Peripherals on the add-on include I2C sensors
> +  and a video bridge controlled via I2C.
> +
> +  The connector has status GPIOs to notify the connection status to the CPU
> +  and a reset GPIO to allow the CPU to reset all the peripherals on the
> +  add-on. It also has I2C busses and a 4-lane MIPI DSI bus.
> +
> +  Different add-on models can be connected, each having different
> +  peripherals. For this reason each add-on has a model ID stored in a
> +  non-volatile memory, which is accessed in the same way on all add-ons.
> +
> +  Add-on removal can happen at any moment under user control and without
> +  prior notice to the CPU, making all of its components not usable
> +  anymore. Later on, the same or a different add-on model can be connected.
> +
> +properties:
> +  compatible:
> +    const: ge,sunh-addon-connector
> +
> +  reset-gpios:
> +    description: An output GPIO to reset the peripherals on the add-on.

Active state is?

> +    maxItems: 1
> +
> +  plugged-gpios:
> +    description: An input GPIO that is asserted if and only if an add-on is
> +      physically connected.

Asserted is high or low?

> +    maxItems: 1
> +
> +  powergood-gpios:
> +    description: An input GPIO that is asserted if and only if power rails
> +      on the add-on are stable.

Active state is?

> +    maxItems: 1
> +
> +  nobus-devices:

Just 'devices'.

> +    description:
> +      A container for devices not accessible via any data bus. Common use
> +      cases include fixed and GPIO regulators, simple video panels and LED
> +      or GPIO backlight devices. When not hot-pluggable, nodes such devices
> +      are children of the root node.
> +
> +      This node should not be present in the connector description in the
> +      base device tree. It should be added by overlays along with a subnode
> +      per device.
> +
> +    type: object
> +    additionalProperties: false

The schema needs to work with the overlay applied too. 'true' is fine 
here.

> +
> +  dsi:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +
> +        description:
> +          OF graph bindings modeling the MIPI DSI bus across the connector. The
> +          connector splits the video pipeline in a fixed part and a removable
> +          part.
> +
> +          The fixed part of the video pipeline includes all components up to
> +          the display controller and 0 or more bridges. The removable part
> +          includes any bridges and any other components up to the panel.
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              The last point of the non-removable part of the MIPI DSI bus
> +              line. The remote-endpoint sub-node must point to the last
> +              non-removable video component of the video pipeline.
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              The first point of the removable part of the MIPI DSI bus
> +              line.  The remote-endpoint sub-node must point to the first
> +              video pipeline component on the add-on. As it describes the
> +              hot-pluggable hardware, the endpoint node cannot be filled
> +              until an add-on is detected, so this node needs to be added
> +              by a device tree overlay at runtime.
> +
> +        required:
> +          - port@0
> +          # port@1 is added by the overlay for any add-on using the DSI lines
> +
> +    required:
> +      - ports
> +
> +patternProperties:
> +  '^i2c-(dbat|gp|btp)$':
> +    description:
> +      An I2C bus that goes through the connector. The adapter (and possibly
> +      some clients) are on the fixed side. Add-ons that have any clients on
> +      this bus have to be added by the add-on overlay, inside this node.
> +
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +    type: object
> +
> +    properties:
> +      i2c-parent:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description:
> +          Phandle pointing to the I2C bus controller on the fixed side that
> +          drives this bus
> +
> +required:
> +  - compatible
> +  - i2c-dbat
> +  - i2c-gp
> +  - i2c-btp
> +  - dsi
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  # This is the description of the connector as it should appear in the
> +  # main DTS describing the "main" board up to the connector. This is
> +  # supposed to be used together with the overlays in the two following
> +  # examples. The addon_connector and hotplug_conn_dsi_out labels are
> +  # referenced by the overlays in those examples.
> +  - |
> +    / {
> +        #include <dt-bindings/gpio/gpio.h>
> +
> +        addon_connector: addon-connector {
> +            compatible = "ge,sunh-addon-connector";
> +            reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> +            plugged-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
> +            powergood-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> +
> +            i2c-dbat {
> +                i2c-parent = <&i2c5_ch2>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                // device subnodes to be added by overlays
> +            };
> +
> +            i2c-gp {
> +                i2c-parent = <&i2c4>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                // device subnodes to be added by overlays
> +            };
> +
> +            i2c-btp {
> +                i2c-parent = <&i2c3>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                // device subnodes to be added by overlays
> +            };
> +
> +            dsi {
> +                ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    port@0 {
> +                        reg = <0>;
> +
> +                        endpoint {
> +                            remote-endpoint = <&previous_bridge_out>;
> +                        };
> +                    };
> +
> +                    // port@1 to be added by overlay
> +                };
> +            };
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 42decde38320..9e902db825d7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10254,6 +10254,11 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
>  F:	drivers/iio/pressure/mprls0025pa*
>  
> +HOTPLUG CONNECTOR FOR GE SUNH ADDONS
> +M:	Luca Ceresoli <luca.ceresoli@bootlin.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
> +
>  HP BIOSCFG DRIVER
>  M:	Jorge Lopez <jorge.lopez2@hp.com>
>  L:	platform-driver-x86@vger.kernel.org
> 
> -- 
> 2.34.1
>
Luca Ceresoli Aug. 14, 2024, 2:42 p.m. UTC | #4
Hello Rob,

On Tue, 13 Aug 2024 09:19:01 -0600
Rob Herring <robh@kernel.org> wrote:

> On Fri, Aug 09, 2024 at 05:34:49PM +0200, Luca Ceresoli wrote:
> > Add bindings for the GE SUNH add-on connector. This is a physical,
> > hot-pluggable connector that allows to attach and detach at runtime an
> > add-on adding peripherals on non-discoverable busses.  
> 
> Overall, looks pretty good.

Thanks, I'm very glad it does.

> > +    maxItems: 1
> > +
> > +  nobus-devices:  
> 
> Just 'devices'.

Sure, simple enough.

> > +    description:
> > +      A container for devices not accessible via any data bus. Common use
> > +      cases include fixed and GPIO regulators, simple video panels and LED
> > +      or GPIO backlight devices. When not hot-pluggable, nodes such devices
> > +      are children of the root node.
> > +
> > +      This node should not be present in the connector description in the
> > +      base device tree. It should be added by overlays along with a subnode
> > +      per device.
> > +
> > +    type: object
> > +    additionalProperties: false  
> 
> The schema needs to work with the overlay applied too. 'true' is fine 
> here.

Does additionalProperties apply to nodes as well? No properties are
supposed to be added inside this node, only nodes, so I'm not sure what
to do about additionalProperties.

Queued all other changes for v4.

Luca
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
new file mode 100644
index 000000000000..2a0b4e0fd089
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
@@ -0,0 +1,185 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/connector/ge,sunh-addon-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GE SUNH hotplug add-on connector
+
+maintainers:
+  - Luca Ceresoli <luca.ceresoli@bootlin.com>
+
+description:
+  Represent the physical connector present on GE SUNH devices that allows
+  to attach and detach at runtime an add-on adding peripherals on
+  non-discoverable busses. Peripherals on the add-on include I2C sensors
+  and a video bridge controlled via I2C.
+
+  The connector has status GPIOs to notify the connection status to the CPU
+  and a reset GPIO to allow the CPU to reset all the peripherals on the
+  add-on. It also has I2C busses and a 4-lane MIPI DSI bus.
+
+  Different add-on models can be connected, each having different
+  peripherals. For this reason each add-on has a model ID stored in a
+  non-volatile memory, which is accessed in the same way on all add-ons.
+
+  Add-on removal can happen at any moment under user control and without
+  prior notice to the CPU, making all of its components not usable
+  anymore. Later on, the same or a different add-on model can be connected.
+
+properties:
+  compatible:
+    const: ge,sunh-addon-connector
+
+  reset-gpios:
+    description: An output GPIO to reset the peripherals on the add-on.
+    maxItems: 1
+
+  plugged-gpios:
+    description: An input GPIO that is asserted if and only if an add-on is
+      physically connected.
+    maxItems: 1
+
+  powergood-gpios:
+    description: An input GPIO that is asserted if and only if power rails
+      on the add-on are stable.
+    maxItems: 1
+
+  nobus-devices:
+    description:
+      A container for devices not accessible via any data bus. Common use
+      cases include fixed and GPIO regulators, simple video panels and LED
+      or GPIO backlight devices. When not hot-pluggable, nodes such devices
+      are children of the root node.
+
+      This node should not be present in the connector description in the
+      base device tree. It should be added by overlays along with a subnode
+      per device.
+
+    type: object
+    additionalProperties: false
+
+  dsi:
+    type: object
+    additionalProperties: false
+
+    properties:
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+
+        description:
+          OF graph bindings modeling the MIPI DSI bus across the connector. The
+          connector splits the video pipeline in a fixed part and a removable
+          part.
+
+          The fixed part of the video pipeline includes all components up to
+          the display controller and 0 or more bridges. The removable part
+          includes any bridges and any other components up to the panel.
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              The last point of the non-removable part of the MIPI DSI bus
+              line. The remote-endpoint sub-node must point to the last
+              non-removable video component of the video pipeline.
+
+          port@1:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              The first point of the removable part of the MIPI DSI bus
+              line.  The remote-endpoint sub-node must point to the first
+              video pipeline component on the add-on. As it describes the
+              hot-pluggable hardware, the endpoint node cannot be filled
+              until an add-on is detected, so this node needs to be added
+              by a device tree overlay at runtime.
+
+        required:
+          - port@0
+          # port@1 is added by the overlay for any add-on using the DSI lines
+
+    required:
+      - ports
+
+patternProperties:
+  '^i2c-(dbat|gp|btp)$':
+    description:
+      An I2C bus that goes through the connector. The adapter (and possibly
+      some clients) are on the fixed side. Add-ons that have any clients on
+      this bus have to be added by the add-on overlay, inside this node.
+
+    $ref: /schemas/i2c/i2c-controller.yaml#
+    unevaluatedProperties: false
+    type: object
+
+    properties:
+      i2c-parent:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description:
+          Phandle pointing to the I2C bus controller on the fixed side that
+          drives this bus
+
+required:
+  - compatible
+  - i2c-dbat
+  - i2c-gp
+  - i2c-btp
+  - dsi
+
+unevaluatedProperties: false
+
+examples:
+  # This is the description of the connector as it should appear in the
+  # main DTS describing the "main" board up to the connector. This is
+  # supposed to be used together with the overlays in the two following
+  # examples. The addon_connector and hotplug_conn_dsi_out labels are
+  # referenced by the overlays in those examples.
+  - |
+    / {
+        #include <dt-bindings/gpio/gpio.h>
+
+        addon_connector: addon-connector {
+            compatible = "ge,sunh-addon-connector";
+            reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+            plugged-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
+            powergood-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+
+            i2c-dbat {
+                i2c-parent = <&i2c5_ch2>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                // device subnodes to be added by overlays
+            };
+
+            i2c-gp {
+                i2c-parent = <&i2c4>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                // device subnodes to be added by overlays
+            };
+
+            i2c-btp {
+                i2c-parent = <&i2c3>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                // device subnodes to be added by overlays
+            };
+
+            dsi {
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+
+                        endpoint {
+                            remote-endpoint = <&previous_bridge_out>;
+                        };
+                    };
+
+                    // port@1 to be added by overlay
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 42decde38320..9e902db825d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10254,6 +10254,11 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
 F:	drivers/iio/pressure/mprls0025pa*
 
+HOTPLUG CONNECTOR FOR GE SUNH ADDONS
+M:	Luca Ceresoli <luca.ceresoli@bootlin.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml
+
 HP BIOSCFG DRIVER
 M:	Jorge Lopez <jorge.lopez2@hp.com>
 L:	platform-driver-x86@vger.kernel.org