Re: [PATCH v2 1/3] dt-bindings: misc: add bindings for Intel HPS Copy Engine

[matthew . gerlach]

On Thu, 5 May 2022, Krzysztof Kozlowski wrote:

> On 05/05/2022 00:41, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
> > > Please find appropriate directory matching this hardware, not "misc". As
> > > a fallback SoC related bindings end up in "soc".
> >
> > I thought misc seemed appropriate because it is a very specific IP block
> > in the FPGA connected to the HPS.  It does perform a simple DMA function;
> > so I considered putting it in the dma directory, but it also has some
> > hand-shaking registers between the HPS and a host processor connected to the
> > FPGA via PCIe; so I thought misc.  Since the HPS "soc" accesses the
> > component, I can put it in the "soc" directory, unless there is a better
> > suggestion.
>
> So let it be "soc".

I will move it to the soc directory.

> > > $ git grep address-cell
> > > Do not use inconsistent coding. The same applies to your DTS.
> >
> > Is the inconsistency the use of '0x' in the values of #address-cells and
> > #size-cells, or is the consistency having different values for
> > #address-cells and #size-cells or both?
>
> It's about "0x".

Got it.  I will remove the 0x.

> > > > +        #size-cells = <0x1>;
> > > > +        ranges = <0x00000000 0x00000000 0xf9000000 0x00001000>;
> > >
> > > Why do you even need the simple-bus above and cannot put the device
> > > directly on the bus?
> >
> > On an Agilex chip, the HPS is connected to the FPGA via two bridges,
> > referred as the "HPS to FPGA bridge" and the "Lightweight HPS to FPGA
> > bridge".  An IP block in the FPGA could be connected to one or both of
> > these bridges.  I am anticipating device tree overlays being applied for
> > other IP blocks instantiated in the FPGA.
>
> OK
>
> > > > +
> > > > +        hps_cp_eng@0 {
> > >
> > > No underscores in node names. Generic node name.
> >
> > I understand.  I am considering dma@0 for the generic node name.
>
> Let's keep the same as in DTS.

Yes, we want the example and the DTS to be the same.  It will be called 
dma-controller@0.

Thank you for the feedback,
Matthew

> Best regards,
> Krzysztof