On 26/02/2024 11:59, Paweł Anikiel wrote: >>> +properties: >>> + compatible: >>> + const: intel,dprx-20.0.1 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + intel,max-link-rate: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Max link rate configuration parameter >> >> Please do not duplicate property name in description. It's useless. >> Instead explain what is this responsible for. >> >> Why max-link-rate would differ for the same dprx-20.0.1? And why >> standard properties cannot be used? >> >> Same for all questions below. > > These four properties are the IP configuration parameters mentioned in > the device description. When generating the IP core you can set these > parameters, which could make them differ for the same dprx-20.0.1. > They are documented in the user guide, for which I also put a link in > the description. Is that enough? Or should I also document these > parameters here? user-guide is something for users, like user-space programmers or end-users. I would never open it to look for any information related to hardware. Anyway, external resources are a no-go. We have it clearly in submitting patches: https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/process/submitting-patches.rst#L130 > >> >>> + >>> + intel,max-lane-count: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Max lane count configuration parameter >>> + >>> + intel,multi-stream-support: >>> + type: boolean >>> + description: Multi-Stream Transport support configuration parameter >>> + >>> + intel,max-stream-count: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Max stream count configuration parameter >>> + >>> + port: >>> + $ref: /schemas/graph.yaml#/properties/port >>> + description: SST main link >> >> I don't understand why you have both port and ports. Shouldn't this be >> under ports? > > I put both so that you can use the shorter port property when the > device only has one port (i.e. no MST support). It would work fine > without it. If you think that's unnecessary, I can remove it (and use > the ports property even if there is only one). No, it is fine, but then you need allOf: which will restrict to only one of them: either port or ports. Best regards, Krzysztof