Hi Krzysztof, On Mon, Nov 28, 2022 at 5:02 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxxxx> > > > > --- > > > > 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@xxxxxxxxxx/ 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 >