Re: [PATCH 08/38] dt-bindings: display: tegra: Document interconnect paths

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]