On Mon, Feb 26, 2024 at 1:06 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > 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 Okay, I will describe these properties in the bindings as well. > > > > >> > >>> + > >>> + 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. There already is an allOf below that says that ports is required for MST support and port is required otherwise. Isn't this enough?
