| Message ID | 20200612141903.2391044-9-thierry.reding@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: json-schema conversions and cleanups | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/checkpatch | success |
Hello Thierry, 12.06.2020 17:18, Thierry Reding пишет: > From: Thierry Reding <treding@nvidia.com> Commit description is missing, checkpatch should warn about it. > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > .../display/tegra/nvidia,tegra20-host1x.yaml | 52 ++++++++++++++++--- > 1 file changed, 46 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > index 3347e1b3c8f0..684fe25641f1 100644 > --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > @@ -97,8 +97,17 @@ properties: > iommus: > $ref: "/schemas/iommu/iommu.yaml#/properties/iommus" > > - memory-controllers: > - $ref: /schemas/types.yaml#/definitions/phandle-array Why memory-controllers property is removed? > + interconnects: > + description: Description of the interconnect paths for the host1x > + controller; see ../interconnect/interconnect.txt for details. > + items: > + - description: memory read client for host1x > + > + interconnect-names: > + description: A list of names identifying each entry listed in the > + "interconnects" property. > + items: > + - const: dma-mem # read Please notice that Host1x has two memory clients: one for DMA engine and second I don't know what's for, maybe for indirect memory accesses. Why you skipped the second path? > required: > - compatible > @@ -489,6 +498,26 @@ allOf: > iommus: > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > + #interconnects: > + # items: > + # - description: memory read client for window A > + # - description: memory read client for window B > + # - description: memory read client for window C > + # - description: memory read client for cursor > + # # disp only > + # - description: memory read client for window T > + # - description: memory read client for window D > + > + #interconnect-names: > + # items: > + # - const: wina > + # - const: winb > + # - const: winc > + # - const: cursor > + # # disp only > + # - const: wint > + # - const: wind Is this really intended to be commented out? Looks like this is an unfinished patch. In the patch [1] I used memory client names for the interconnect paths. I like yours variant of the naming, it is more intuitive. [1] https://patchwork.ozlabs.org/project/linux-tegra/patch/20200609131404.17523-23-digetx@gmail.com/ I'll rebase my series on top of yours patches once you'll get them into linux-next. Looking forward to v2!
On Fri, Jun 12, 2020 at 06:52:44PM +0300, Dmitry Osipenko wrote: > Hello Thierry, > > 12.06.2020 17:18, Thierry Reding пишет: > > From: Thierry Reding <treding@nvidia.com> > Commit description is missing, checkpatch should warn about it. > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > .../display/tegra/nvidia,tegra20-host1x.yaml | 52 ++++++++++++++++--- > > 1 file changed, 46 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > > index 3347e1b3c8f0..684fe25641f1 100644 > > --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > > @@ -97,8 +97,17 @@ properties: > > iommus: > > $ref: "/schemas/iommu/iommu.yaml#/properties/iommus" > > > > - memory-controllers: > > - $ref: /schemas/types.yaml#/definitions/phandle-array > > Why memory-controllers property is removed? Because it was accidentally added in patch 7. This is leftover from an earlier set of patches. > > + interconnects: > > + description: Description of the interconnect paths for the host1x > > + controller; see ../interconnect/interconnect.txt for details. > > + items: > > + - description: memory read client for host1x > > + > > + interconnect-names: > > + description: A list of names identifying each entry listed in the > > + "interconnects" property. > > + items: > > + - const: dma-mem # read > > Please notice that Host1x has two memory clients: one for DMA engine and > second I don't know what's for, maybe for indirect memory accesses. Why > you skipped the second path? I'm primarily targetting Tegra186 and Tegra194 with these patches because we need the interconnects properties on those platforms in order to correctly set the DMA masks at a global scope. For Tegra186 and Tegra194 I can only see a single memory client for host1x. Looking at the register documentation, the host1xw and host1xr clients do exist on Tegra210 and earlier and they are used for indirect writes and reads, respectively. I don't think we use indirect reads/writes, so we can probably do without the corresponding memory clients. Alternatively we could conditionally add them on Tegra210 and earlier. Thierry > > required: > > - compatible > > @@ -489,6 +498,26 @@ allOf: > > iommus: > > $ref: "/schemas/types.yaml#/definitions/phandle-array" > > > > + #interconnects: > > + # items: > > + # - description: memory read client for window A > > + # - description: memory read client for window B > > + # - description: memory read client for window C > > + # - description: memory read client for cursor > > + # # disp only > > + # - description: memory read client for window T > > + # - description: memory read client for window D > > + > > + #interconnect-names: > > + # items: > > + # - const: wina > > + # - const: winb > > + # - const: winc > > + # - const: cursor > > + # # disp only > > + # - const: wint > > + # - const: wind > > Is this really intended to be commented out? Looks like this is an > unfinished patch. I started adding this after finishing Tegra186 and Tegra194, but then I ran into various other issues and wanted to do this step by step, so I plan to go back to Tegra210 and earlier later on and stub them out. However, since I had already done the research I wanted to leave these in here for later reference, so I didn't have to go back and look this up again. > In the patch [1] I used memory client names for the interconnect paths. > I like yours variant of the naming, it is more intuitive. I only noticed after posting this series that you had sent out the series for interconnects and that it conflicted in the bindings. But yeah, I do also like the contextual names better because they are more intuitive. > [1] > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200609131404.17523-23-digetx@gmail.com/ > > I'll rebase my series on top of yours patches once you'll get them into > linux-next. Looking forward to v2! There's quite a bit to review here, but I also have a couple of patches that depend on these, so yeah, it'd be great if this could go in sooner rather than later. Thierry
diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml index 3347e1b3c8f0..684fe25641f1 100644 --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml @@ -97,8 +97,17 @@ properties: iommus: $ref: "/schemas/iommu/iommu.yaml#/properties/iommus" - memory-controllers: - $ref: /schemas/types.yaml#/definitions/phandle-array + interconnects: + description: Description of the interconnect paths for the host1x + controller; see ../interconnect/interconnect.txt for details. + items: + - description: memory read client for host1x + + interconnect-names: + description: A list of names identifying each entry listed in the + "interconnects" property. + items: + - const: dma-mem # read required: - compatible @@ -489,6 +498,26 @@ allOf: iommus: $ref: "/schemas/types.yaml#/definitions/phandle-array" + #interconnects: + # items: + # - description: memory read client for window A + # - description: memory read client for window B + # - description: memory read client for window C + # - description: memory read client for cursor + # # disp only + # - description: memory read client for window T + # - description: memory read client for window D + + #interconnect-names: + # items: + # - const: wina + # - const: winb + # - const: winc + # - const: cursor + # # disp only + # - const: wint + # - const: wind + nvidia,head: description: The number of the display controller head. This is used to setup the various types of output to receive video @@ -634,8 +663,15 @@ allOf: master interface of this display controller. $ref: "/schemas/types.yaml#/definitions/phandle-array" - memory-controllers: - $ref: /schemas/types.yaml#/definitions/phandle-array + interconnects: + description: Description of the interconnect paths for the + display controller; see ../interconnect/interconnect.txt + for details. + + interconnect-names: + items: + - const: dma-mem # read-0 + - const: read-1 nvidia,outputs: description: A list of phandles of outputs that this display @@ -1027,8 +1063,12 @@ allOf: iommus: $ref: "/schemas/types.yaml#/definitions/phandle-array" - memory-controllers: - $ref: /schemas/types.yaml#/definitions/phandle-array + interconnects: true + + interconnect-names: + items: + - const: dma-mem # read + - const: write unevaluatedProperties: false