Re: [[LINUX PATCH v10] 1/4] Devicetree: Add pl353 smc controller devicetree binding information

[Linus Walleij]

On Thu, Jun 21, 2018 at 8:43 AM Naga Sureshkumar Relli
<naga.sureshkumar.relli@xxxxxxxxxx> wrote:

Thanks for your patch!

> Add pl353 static memory controller devicetree binding information.
>
> Signed-off-by: Naga Sureshkumar Relli <naga.sureshkumar.relli@xxxxxxxxxx>
> +Device tree bindings for ARM PL353 static memory controller
> +
> +PL353 static memory controller supports two kinds of memory
> +interfaces.i.e NAND and SRAM/NOR interfaces.
> +The actual devices are instantiated from the child nodes of pl353 smc node.

See and reference the special PrimeCell DT bindings:
Documentation/devicetree/bindings/arm/primecell.txt

> +Required properties:
> +- compatible           : Must be "arm,pl353-smc-r2p1"

Should be:

"arm,pl353-smc-r2p1", "arm,primecell";

It will be possible for the Linux AMBA core to probe the device
from just reading out the primecell ID registers, so the first
compatible string will not even be used by many OS:es.

> +- reg                  : Controller registers map and length.
> +- clock-names          : List of input clock names - "ref_clk", "aper_clk"
> +                         (See clock bindings for details).

The AMBA primecell peripheral clock (clocking the silicon)
must be named "apb_pclk".

> +- clocks               : Clock phandles (see clock bindings for details).
> +- address-cells                : Must be 1.
> +- size-cells           : Must be 1.
> +
> +Child nodes:
> + For NAND the "arm,pl353-nand-r2p1" and for NOR the "cfi-flash" drivers are
> +supported as child nodes.
> +
> +for NAND partition information please refer the below file
> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> +       smcc: memory-controller@e000e000
> +                       compatible = "arm,pl353-smc-r2p1"

Add , "arm,primecell";

> +                       clock-names = "memclk", "aclk";

This doesn't even have the same name as in the bindings
above, I think the latter should be named "apb_pclk".

Otherwise it looks good!

Yours,
Linus Walleij