RE: [PATCH 1/3] dt-bindings: memory-controllers: Add support for Versal NET EDAC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Sent: Saturday, November 23, 2024 10:15 PM
> To: Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> edac@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>; krzk@xxxxxxxxx;
> robh@xxxxxxxxxx; conor+dt@xxxxxxxxxx; bp@xxxxxxxxx; tony.luck@xxxxxxxxx;
> james.morse@xxxxxxx; mchehab@xxxxxxxxxx; rric@xxxxxxxxxx
> Subject: Re: [PATCH 1/3] dt-bindings: memory-controllers: Add support for Versal
> NET EDAC
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Fri, Nov 22, 2024 at 03:36:23PM +0530, Shubhrajyoti Datta wrote:
> > Add device tree bindings for AMD Versal NET EDAC for DDR controller.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
> > ---
> >
>
> Use tools to create cc-list, like b4 or:
> https://github.com/krzk/tools/blob/master/linux/.bash_aliases_linux#L92
> so you won't make a typo in my email.
>
> >  .../amd,versalnet-edac.yaml                   | 56 +++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/memory-controllers/amd,versalnet-eda
> > c.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/memory-controllers/amd,versalnet-e
> > dac.yaml
> > b/Documentation/devicetree/bindings/memory-controllers/amd,versalnet-e
> > dac.yaml
> > new file mode 100644
> > index 000000000000..22a4669c46b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/memory-controllers/amd,versaln
> > +++ et-edac.yaml
> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +http://devicetree.org/schemas/memory-controllers/amd,versalnet-edac.y
> > +aml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMD Versal NET EDAC
>
> s/EDAC/Memory Controller
> or something similar, I guess.

Will do

>
> > +
> > +maintainers:
> > +  - Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
> > +
> > +description:
> > +  The integrated DDR Memory Controllers (DDRMCs) support both DDR4
> > +and LPDDR4/
> > +  4X memory interfaces. Versal NET DDR memory controller has an
> > +optional ECC support
> > +  which correct single bit ECC errors and detect double bit ECC errors.
> > +  It also has support for reporting other errors like MMCM
> > +(Mixed-Mode Clock
> > +  Manager) errors and General software errors.
> > +
> > +properties:
> > +  compatible:
> > +    const: amd,versalnet-edac
>
> Why using different name than all others? Keep consistent stuff for your SoCs.
>
> Also, s/edac/memory-controller/, depending what this stuff really is.

Will do
>
> > +
> > +  amd,dwidth:
> > +    description:
> > +      DDR memory controller device width.
>
> Use existing properties.
>
>
> > +    enum: [16, 32]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  amd,num-chans:
> > +    description:
> > +      Number of channels.
>
> Use existing properties, e.g. some of the DDR schemas describing memory.
> Look how other bindings describe actual chips.
>
> > +    enum: [1, 2]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  amd,num-rank:
> > +    description:
> > +      Number of rank.
> > +    enum: [1, 2, 4]
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +required:
> > +  - compatible
>
> Eh, no resources? How do you talk with the hardware? This looks way too Linuxy...

The address space is secure, making it inaccessible to Linux. In this setup, the secure firmware (NMC)
 communicates the necessary information to Linux through RPMsg.
>
> Best regards,
> Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux