| Message ID | 20221124102056.393220-7-treapking@chromium.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Register Type-C mode-switch in DP bridge endpoints | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On Thu, 24 Nov 2022 18:20:55 +0800, Pin-yen Lin wrote: > ITE IT6505 can be used in systems to switch the DP traffic between > two downstreams, which can be USB Type-C DisplayPort alternate mode > lane or regular DisplayPort output ports. > > Update the binding to accommodate this usage by introducing a > data-lanes and a mode-switch property on endpoints. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > --- > > Changes in v6: > - Remove switches node and use endpoints and data-lanes property to > describe the connections. > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++++++++++++++++++- > 1 file changed, 90 insertions(+), 4 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts' Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml: ignoring, error parsing file make: *** [Makefile:1492: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-7-treapking@chromium.org This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
Sorry for accidentally using the tab characters. Will fix this in v7. On Fri, Nov 25, 2022 at 1:39 AM Rob Herring <robh@kernel.org> wrote: > > > On Thu, 24 Nov 2022 18:20:55 +0800, Pin-yen Lin wrote: > > ITE IT6505 can be used in systems to switch the DP traffic between > > two downstreams, which can be USB Type-C DisplayPort alternate mode > > lane or regular DisplayPort output ports. > > > > Update the binding to accommodate this usage by introducing a > > data-lanes and a mode-switch property on endpoints. > > > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > > > --- > > > > Changes in v6: > > - Remove switches node and use endpoints and data-lanes property to > > describe the connections. > > > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++++++++++++++++++- > > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: [error] syntax error: found character '\t' that cannot start any token (syntax) > > dtschema/dtc warnings/errors: > make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts' > Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token > make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts] Error 1 > make[1]: *** Waiting for unfinished jobs.... > ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml: ignoring, error parsing file > make: *** [Makefile:1492: dt_binding_check] Error 2 > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-7-treapking@chromium.org > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > 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. >
On 24/11/2022 11:20, Pin-yen Lin wrote: > ITE IT6505 can be used in systems to switch the DP traffic between > two downstreams, which can be USB Type-C DisplayPort alternate mode > lane or regular DisplayPort output ports. > > Update the binding to accommodate this usage by introducing a > data-lanes and a mode-switch property on endpoints. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > --- > > Changes in v6: > - Remove switches node and use endpoints and data-lanes property to > describe the connections. > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++++++++++++++++++- > 1 file changed, 90 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > index 833d11b2303a..b4b9881c7759 100644 > --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > @@ -52,9 +52,53 @@ properties: > maxItems: 1 > description: extcon specifier for the Power Delivery > > - port: > - $ref: /schemas/graph.yaml#/properties/port > - description: A port node pointing to DPI host port node > + data-lanes: > + maxItems: 1 > + description: restrict the dp output data-lanes with value of 1-4 Hm, where is the definition of this type? For example it comes with video-interfaces, which you did not reference here. > + > + max-pixel-clock-khz: There is no such unit accepted: https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml > + maxItems: 1 maxItems of what type? What is this? > + description: restrict max pixel clock > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports This is incompatible change... how do you handle now ABI break? > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/$defs/port-base Why changing the ref? > + unevaluatedProperties: false > + description: A port node pointing to DPI host port node > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port-base > + description: > + Video port for panel or connector. > + > + patternProperties: > + "^endpoint@[01]$": > + $ref: /schemas/media/video-interfaces.yaml# > + type: object > + unevaluatedProperties: false > + > + properties: > + reg: > + maxItems: 1 > + > + remote-endpoint: true > + > + data-lanes: > + minItems: 1 > + uniqueItems: true > + items: > + - enum: [ 0, 1, 2, 3] Same problem as your previouspatch. > + > + mode-switch: > + type: boolean > + description: Register this node as a Type-C mode switch or not. > + > + required: > + - reg > + - remote-endpoint > > required: > - compatible > @@ -62,7 +106,7 @@ required: > - pwr18-supply > - interrupts > - reset-gpios > - - extcon > + - ports > > additionalProperties: false > > @@ -92,3 +136,45 @@ examples: > }; > }; > }; > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + &i2c3 { > + clock-frequency = <100000>; > + > + it6505dptx: it6505dptx@5c { Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ite,it6505"; > + interrupts = <8 IRQ_TYPE_LEVEL_LOW 8 0>; > + reg = <0x5c>; > + pinctrl-names = "default"; > + pinctrl-0 = <&it6505_pins>; > + ovdd-supply = <&mt6366_vsim2_reg>; > + pwr18-supply = <&pp1800_dpbrdg_dx>; > + reset-gpios = <&pio 177 0>; > + hpd-gpios = <&pio 10 0>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + it6505_in: endpoint { > + remote-endpoint = <&dpi_out>; > + }; > + }; > + port@1 { > + reg = <1>; > + ite_typec0: endpoint@0 { > + mode-switch; > + data-lanes = <0 1>; Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Best regards, Krzysztof
Hi Krzysztof, On Mon, Nov 28, 2022 at 5:02 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 24/11/2022 11:20, Pin-yen Lin wrote: > > ITE IT6505 can be used in systems to switch the DP traffic between > > two downstreams, which can be USB Type-C DisplayPort alternate mode > > lane or regular DisplayPort output ports. > > > > Update the binding to accommodate this usage by introducing a > > data-lanes and a mode-switch property on endpoints. > > > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > > > --- > > > > Changes in v6: > > - Remove switches node and use endpoints and data-lanes property to > > describe the connections. > > > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++++++++++++++++++- > > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > index 833d11b2303a..b4b9881c7759 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml > > @@ -52,9 +52,53 @@ properties: > > maxItems: 1 > > description: extcon specifier for the Power Delivery > > > > - port: > > - $ref: /schemas/graph.yaml#/properties/port > > - description: A port node pointing to DPI host port node > > + data-lanes: > > + maxItems: 1 > > + description: restrict the dp output data-lanes with value of 1-4 > > Hm, where is the definition of this type? For example it comes with > video-interfaces, which you did not reference here. > Actually I messed up here with another accepted patch: https://lore.kernel.org/all/20221103091243.96036-2-allen.chen@ite.com.tw/ This and the next new property have been added in that patch. > > + > > + max-pixel-clock-khz: > > There is no such unit accepted: > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml > > > + maxItems: 1 > > maxItems of what type? What is this? > > > + description: restrict max pixel clock > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > This is incompatible change... how do you handle now ABI break? > This is also added in another patch, and currently we don't have any upstream it6505 users now. > > + > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/$defs/port-base > > Why changing the ref? The `unevaluatedProperties: false` in `/schemas/graph.yaml#/properties/port` does not allow me to add new properties here. > > > + unevaluatedProperties: false > > + description: A port node pointing to DPI host port node > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port-base > > + description: > > + Video port for panel or connector. > > + > > + patternProperties: > > + "^endpoint@[01]$": > > + $ref: /schemas/media/video-interfaces.yaml# > > + type: object > > + unevaluatedProperties: false > > + > > + properties: > > + reg: > > + maxItems: 1 > > + > > + remote-endpoint: true > > + > > + data-lanes: > > + minItems: 1 > > + uniqueItems: true > > + items: > > + - enum: [ 0, 1, 2, 3] > > Same problem as your previouspatch. > > > + > > + mode-switch: > > + type: boolean > > + description: Register this node as a Type-C mode switch or not. > > + > > + required: > > + - reg > > + - remote-endpoint > > > > required: > > - compatible > > @@ -62,7 +106,7 @@ required: > > - pwr18-supply > > - interrupts > > - reset-gpios > > - - extcon > > + - ports > > > > additionalProperties: false > > > > @@ -92,3 +136,45 @@ examples: > > }; > > }; > > }; > > + - | > > + #include <dt-bindings/interrupt-controller/irq.h> > > + > > + &i2c3 { > > + clock-frequency = <100000>; > > + > > + it6505dptx: it6505dptx@5c { > > Node names should be generic. > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > I'll fix this in v7. > > + compatible = "ite,it6505"; > > + interrupts = <8 IRQ_TYPE_LEVEL_LOW 8 0>; > > + reg = <0x5c>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&it6505_pins>; > > + ovdd-supply = <&mt6366_vsim2_reg>; > > + pwr18-supply = <&pp1800_dpbrdg_dx>; > > + reset-gpios = <&pio 177 0>; > > + hpd-gpios = <&pio 10 0>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + it6505_in: endpoint { > > + remote-endpoint = <&dpi_out>; > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + ite_typec0: endpoint@0 { > > + mode-switch; > > + data-lanes = <0 1>; > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). Sorry for not checking the documentation and testing the patches before submitting this. I'll fix the errors in v7. Best regards, Pin-yen > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml index 833d11b2303a..b4b9881c7759 100644 --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml @@ -52,9 +52,53 @@ properties: maxItems: 1 description: extcon specifier for the Power Delivery - port: - $ref: /schemas/graph.yaml#/properties/port - description: A port node pointing to DPI host port node + data-lanes: + maxItems: 1 + description: restrict the dp output data-lanes with value of 1-4 + + max-pixel-clock-khz: + maxItems: 1 + description: restrict max pixel clock + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + description: A port node pointing to DPI host port node + + port@1: + $ref: /schemas/graph.yaml#/properties/port-base + description: + Video port for panel or connector. + + patternProperties: + "^endpoint@[01]$": + $ref: /schemas/media/video-interfaces.yaml# + type: object + unevaluatedProperties: false + + properties: + reg: + maxItems: 1 + + remote-endpoint: true + + data-lanes: + minItems: 1 + uniqueItems: true + items: + - enum: [ 0, 1, 2, 3] + + mode-switch: + type: boolean + description: Register this node as a Type-C mode switch or not. + + required: + - reg + - remote-endpoint required: - compatible @@ -62,7 +106,7 @@ required: - pwr18-supply - interrupts - reset-gpios - - extcon + - ports additionalProperties: false @@ -92,3 +136,45 @@ examples: }; }; }; + - | + #include <dt-bindings/interrupt-controller/irq.h> + + &i2c3 { + clock-frequency = <100000>; + + it6505dptx: it6505dptx@5c { + compatible = "ite,it6505"; + interrupts = <8 IRQ_TYPE_LEVEL_LOW 8 0>; + reg = <0x5c>; + pinctrl-names = "default"; + pinctrl-0 = <&it6505_pins>; + ovdd-supply = <&mt6366_vsim2_reg>; + pwr18-supply = <&pp1800_dpbrdg_dx>; + reset-gpios = <&pio 177 0>; + hpd-gpios = <&pio 10 0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + it6505_in: endpoint { + remote-endpoint = <&dpi_out>; + }; + }; + port@1 { + reg = <1>; + ite_typec0: endpoint@0 { + mode-switch; + data-lanes = <0 1>; + remote-endpoint = <&typec_port0>; + }; + ite_typec1: endpoint@1 { + mode-switch; + data-lanes = <2 3>; + remote-endpoint = <&typec_port1>; + }; + }; + }; + }; + };
ITE IT6505 can be used in systems to switch the DP traffic between two downstreams, which can be USB Type-C DisplayPort alternate mode lane or regular DisplayPort output ports. Update the binding to accommodate this usage by introducing a data-lanes and a mode-switch property on endpoints. Signed-off-by: Pin-yen Lin <treapking@chromium.org> --- Changes in v6: - Remove switches node and use endpoints and data-lanes property to describe the connections. .../bindings/display/bridge/ite,it6505.yaml | 94 ++++++++++++++++++- 1 file changed, 90 insertions(+), 4 deletions(-)