| Message ID | 20230119121639.226729-2-jonathanh@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add device-tree support for Cypress CYPD4226 | expand |
On 19/01/2023 13:16, Jon Hunter wrote: > From: Wayne Chang <waynec@nvidia.com> > > Add the device-tree binding documentation for Cypress cypd4226 dual > Type-C controller. > > Signed-off-by: Wayne Chang <waynec@nvidia.com> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > V5 -> V6: no changes > V4 -> V5: updated subject and updated binding to use 'firmware-name'. > V3 -> V4: no changes > V2 -> V3: fix additionalProperties warning on new schema > V1 -> V2: based on the review comments. Fix some addressed issues on > > .../bindings/usb/cypress,cypd4226.yaml | 86 +++++++++++++++++++ > 1 file changed, 86 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml > > diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml > new file mode 100644 > index 000000000000..5d87c9f09913 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml > @@ -0,0 +1,86 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cypress cypd4226 Type-C Controller > + > +maintainers: > + - Wayne Chang <waynec@nvidia.com> > + > +description: > + The Cypress cypd4226 is a dual Type-C controller that is controlled > + via an I2C interface. > + > +properties: > + compatible: > + const: cypress,cypd4226 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + reg: > + const: 0x08 > + > + interrupts: > + items: > + - description: cypd4226 host interrupt > + > + firmware-name: > + enum: > + - nvidia,gpu > + - nvidia,jetson-agx-xavier > + description: | > + The name of the CCGx firmware built for product series. > + should be set one of following: > + - "nvidia,gpu" for the NVIDIA RTX product series > + - "nvidia,jetson-agx-xavier" for the NVIDIA Jetson product series > + > +patternProperties: > + '^connector@[0-1]+$': > + $ref: /schemas/connector/usb-connector.yaml# On this level: unevaluatedProperties: false > + properties: > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/tegra194-gpio.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + #interrupt-cells = <2>; Drop, does not look like relevant or used here. > + > + ucsi-ccg@8 { Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "cypress,cypd4226"; > + reg = <0x08>; > + interrupt-parent = <&gpio_aon>; > + interrupts = <TEGRA194_AON_GPIO(BB, 2) IRQ_TYPE_LEVEL_LOW>; > + firmware-name = "nvidia,jetson-agx-xavier"; > + #address-cells = <1>; > + #size-cells = <0>; > + ccg_typec_con0: connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + label = "USB-C"; > + data-role = "dual"; > + port { > + ucsi_ccg_p0: endpoint { > + remote-endpoint = <&usb_role_switch0>; > + }; > + }; > + }; > + }; > + }; Best regards, Krzysztof
On 20/01/2023 08:32, Krzysztof Kozlowski wrote: ... >> +examples: >> + - | >> + #include <dt-bindings/gpio/tegra194-gpio.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + #interrupt-cells = <2>; > > Drop, does not look like relevant or used here. Without the above I get ... Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:25.13-26: Warning (reg_format): /example-0/i2c/typec-controller@8:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #address-cells value Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #size-cells value Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: typec-controller@8: interrupts: [[10], [8]] is too long > >> + >> + ucsi-ccg@8 { > > Node names should be generic. > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Thanks. I don't see anything there is would fit here, so would 'typec-controller' for the node name be OK? Cheers Jon
On 23/01/2023 14:28, Jon Hunter wrote: ... >>> + >>> + ucsi-ccg@8 { >> >> Node names should be generic. >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > Thanks. I don't see anything there is would fit here, so would > 'typec-controller' for the node name be OK? Looks like the st,typec-stm32g0 and google,cros-ec-typec both use just 'typec'. So we can use the same as this looks quite similar to the ST device. Jon
On 23/01/2023 15:28, Jon Hunter wrote: > > On 20/01/2023 08:32, Krzysztof Kozlowski wrote: > > ... > >>> +examples: >>> + - | >>> + #include <dt-bindings/gpio/tegra194-gpio.h> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + i2c { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + #interrupt-cells = <2>; >> >> Drop, does not look like relevant or used here. > > > Without the above I get ... Didn't you drop too much? I meant only that one line above my comment, so only interrupt-cells. > > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:25.13-26: Warning (reg_format): /example-0/i2c/typec-controller@8:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #address-cells value > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #size-cells value > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: typec-controller@8: interrupts: [[10], [8]] is too long > >> >>> + >>> + ucsi-ccg@8 { >> >> Node names should be generic. >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > Thanks. I don't see anything there is would fit here, so would 'typec-controller' for the node name be OK? Yeah, pretty often we miss a generic example. Can be just "typec". > Best regards, Krzysztof
On 23/01/2023 15:54, Krzysztof Kozlowski wrote: ... >>>> +examples: >>>> + - | >>>> + #include <dt-bindings/gpio/tegra194-gpio.h> >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>>> + i2c { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + #interrupt-cells = <2>; >>> >>> Drop, does not look like relevant or used here. >> >> >> Without the above I get ... > > Didn't you drop too much? I meant only that one line above my comment, > so only interrupt-cells. Yes, I was not sure if you meant just interrupt-cells or all of them. However ... >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:25.13-26: Warning (reg_format): /example-0/i2c/typec-controller@8:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:22.13-43.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #address-cells value >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dts:23.30-42.13: Warning (avoid_default_addr_size): /example-0/i2c/typec-controller@8: Relying on default #size-cells value >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' >> Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: typec-controller@8: interrupts: [[10], [8]] is too long Per the above if we remove interrupt-cells, then we get ... Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: typec@8: interrupts: [[10], [8]] is too long From schema: /home/jonathanh/workdir/tegra/korg-linux-next.git/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml So it seems that we need this as well. Jon
On 23/01/2023 23:16, Jon Hunter wrote: > Per the above if we remove interrupt-cells, then we get ... > > Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb: typec@8: interrupts: [[10], [8]] is too long > From schema: /home/jonathanh/workdir/tegra/korg-linux-next.git/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml > > So it seems that we need this as well. > Indeed. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml new file mode 100644 index 000000000000..5d87c9f09913 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/cypress,cypd4226.yaml @@ -0,0 +1,86 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/cypress,cypd4226.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cypress cypd4226 Type-C Controller + +maintainers: + - Wayne Chang <waynec@nvidia.com> + +description: + The Cypress cypd4226 is a dual Type-C controller that is controlled + via an I2C interface. + +properties: + compatible: + const: cypress,cypd4226 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + reg: + const: 0x08 + + interrupts: + items: + - description: cypd4226 host interrupt + + firmware-name: + enum: + - nvidia,gpu + - nvidia,jetson-agx-xavier + description: | + The name of the CCGx firmware built for product series. + should be set one of following: + - "nvidia,gpu" for the NVIDIA RTX product series + - "nvidia,jetson-agx-xavier" for the NVIDIA Jetson product series + +patternProperties: + '^connector@[0-1]+$': + $ref: /schemas/connector/usb-connector.yaml# + properties: + reg: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/tegra194-gpio.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + #interrupt-cells = <2>; + + ucsi-ccg@8 { + compatible = "cypress,cypd4226"; + reg = <0x08>; + interrupt-parent = <&gpio_aon>; + interrupts = <TEGRA194_AON_GPIO(BB, 2) IRQ_TYPE_LEVEL_LOW>; + firmware-name = "nvidia,jetson-agx-xavier"; + #address-cells = <1>; + #size-cells = <0>; + ccg_typec_con0: connector@0 { + compatible = "usb-c-connector"; + reg = <0>; + label = "USB-C"; + data-role = "dual"; + port { + ucsi_ccg_p0: endpoint { + remote-endpoint = <&usb_role_switch0>; + }; + }; + }; + }; + };