On 09/06/2021 15:34, Miquel Raynal wrote: > Hi Krzysztof, > > Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> wrote on Wed, 9 > Jun 2021 14:12:40 +0200: > >> On 09/06/2021 10:01, Miquel Raynal wrote: >>> Convert this binding file to yaml schema. >>> >>> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> >>> --- >>> .../memory-controllers/arm,pl353-smc.yaml | 133 ++++++++++++++++++ >>> .../bindings/memory-controllers/pl353-smc.txt | 45 ------ >>> 2 files changed, 133 insertions(+), 45 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml >>> delete mode 100644 Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml >>> new file mode 100644 >>> index 000000000000..1de6f87d4986 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml >>> @@ -0,0 +1,133 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/memory-controllers/arm,pl353-smc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: ARM PL353 Static Memory Controller (SMC) device-tree bindings >>> + >>> +maintainers: >>> + - Miquel Raynal <miquel.raynal@xxxxxxxxxxx> >>> + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xxxxxxxxxx> >>> + >>> +description: >>> + The PL353 Static Memory Controller is a bus where you can connect two kinds >>> + of memory interfaces, which are NAND and memory mapped interfaces (such as >>> + SRAM or NOR). >>> + >>> +# We need a select here so we don't match all nodes with 'arm,primecell' >>> +select: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - arm,pl353-smc-r2p1 >> >> That's a const... but also I don't get the need for select. > > I think this is needed to ensure this binding is not enforced against > arm,primecell compatible nodes which are not featuring the > arm,pl353-smc-r2p1 compatible. Which seems to be result of unusual compatible match, so once you convert to regular match, this select is not needed. > >> >>> + required: >>> + - compatible >>> + >>> +properties: >>> + $nodename: >>> + pattern: "^memory-controller@[0-9a-f]+$" >>> + >>> + compatible: >>> + oneOf: >>> + - items: >>> + - enum: >>> + - arm,pl353-smc-r2p1 >>> + - enum: >>> + - arm,primecell >> >> This looks unusual. Basically you change the bindings, because before >> they required "arm,pl353-smc-r2p1", "arm,primecell". > > That is precisely what I try to match and I think it works. Perhaps > this version is easier to extend when a new compatible comes in. > However, I am fine using an alternative formula, like below if you > think it's better: > > compatible: > items: > - const: arm,pl353-smc-r2p1 > - const: arm,primecell That's the common, easiest and actually expected. > >> Don't you want here items: >> - const: ... >> - const: ... >> ? >> >>> + >>> + "#address-cells": >>> + const: 2 >>> + >>> + "#size-cells": >>> + const: 1 >>> + >>> + reg: >>> + items: >>> + - description: configuration registers for the host and sub-controllers >> >> Just maxItems. Description is obvious. > > I don't think it is that obvious because there are actually 4 areas > and, because of the yaml language, we only describe one in the reg > property while the others and defined in the ranges property, but > that's fine by me, I'll drop the description and stick to a > maxItems entry. The explanation of all four areas could have sense, but now it states the obvious - these are configuration registers :) > >> >>> + >>> + clocks: >>> + items: >>> + - description: the clock for the memory device bus >>> + - description: the main clock of the controller >> >> Isn't apb_pclk the bus clock (so second item below)? > > The SMC has two clock domains referred as aclk and mclk. In the TRM, > aclk is described as "Clock for the AXI domain". The AXI interface is > used to trigger CMD/ADDR/DATA cycles on the memory bus. There is also > an APB interface used to reach the SMC registers. I *think* that > both APB and AXI domains are fed by the same apb_pclk source but I > might be wrong. Hence memclk would just feed the memory bus that bonds > the memory device (eg. the NAND flash) to the host controller. > > This is my current understanding but if you think it works differently > I'm all ears because this part is not 100% clear to me. I was just wondering... your explanation is fine to me, thanks! Best regards, Krzysztof
