Messages in this thread | | | Date | Wed, 20 Apr 2022 19:55:54 +0800 | Subject | Re: [PATCH v4 2/2] dt-bindings: nand: meson: refine Amlogic NAND controller driver | From | Liang Yang <> |
| |
Hi Miquel,
On 2022/4/20 15:41, Miquel Raynal wrote: > [ EXTERNAL EMAIL ] > > Hi Liang, > > liang.yang@amlogic.com wrote on Sat, 2 Apr 2022 15:49:20 +0800: > >> convert txt to yaml and refine the meson NFC clock document. > > We generally prefer to split this into two changes (yaml conversion > then modifications). You need to be very explicit on the changes you > bring to this file afterward. Also you may s/refine/fix/ in your title > if this really is a correction of something that does not work at all as > you suggest. ok > > Please mention that due to the other series about the clock changes > never being accepted the current binding was never valid/working ok > (again, I'm not sure it's the case on all Amlogic SoCs, so please be > very careful about that). of course. > > And please use a Link: tag to point to the discussion with Neil and > Jerome on your MMC/NAND subclock final discussion. ok > >> Signed-off-by: Liang Yang <liang.yang@amlogic.com> >> --- >> .../bindings/mtd/amlogic,meson-nand.txt | 60 -------------- >> .../bindings/mtd/amlogic,meson-nand.yaml | 80 +++++++++++++++++++ >> 2 files changed, 80 insertions(+), 60 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt >> create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml >> >> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt >> deleted file mode 100644 >> index 5794ab1147c1..000000000000 >> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt >> +++ /dev/null >> @@ -1,60 +0,0 @@ >> -Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs >> - >> -This file documents the properties in addition to those available in >> -the MTD NAND bindings. >> - >> -Required properties: >> -- compatible : contains one of: >> - - "amlogic,meson-gxl-nfc" >> - - "amlogic,meson-axg-nfc" >> -- clocks : >> - A list of phandle + clock-specifier pairs for the clocks listed >> - in clock-names. >> - >> -- clock-names: Should contain the following: >> - "core" - NFC module gate clock >> - "device" - device clock from eMMC sub clock controller >> - "rx" - rx clock phase >> - "tx" - tx clock phase >> - >> -- amlogic,mmc-syscon : Required for NAND clocks, it's shared with SD/eMMC >> - controller port C >> - >> -Optional children nodes: >> -Children nodes represent the available nand chips. >> - >> -Other properties: >> -see Documentation/devicetree/bindings/mtd/nand-controller.yaml for generic bindings. >> - >> -Example demonstrate on AXG SoC: >> - >> - sd_emmc_c_clkc: mmc@7000 { >> - compatible = "amlogic,meson-axg-mmc-clkc", "syscon"; >> - reg = <0x0 0x7000 0x0 0x800>; >> - }; >> - >> - nand-controller@7800 { >> - compatible = "amlogic,meson-axg-nfc"; >> - reg = <0x0 0x7800 0x0 0x100>; >> - #address-cells = <1>; >> - #size-cells = <0>; >> - interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>; >> - >> - clocks = <&clkc CLKID_SD_EMMC_C>, >> - <&sd_emmc_c_clkc CLKID_MMC_DIV>, >> - <&sd_emmc_c_clkc CLKID_MMC_PHASE_RX>, >> - <&sd_emmc_c_clkc CLKID_MMC_PHASE_TX>; >> - clock-names = "core", "device", "rx", "tx"; >> - amlogic,mmc-syscon = <&sd_emmc_c_clkc>; >> - >> - pinctrl-names = "default"; >> - pinctrl-0 = <&nand_pins>; >> - >> - nand@0 { >> - reg = <0>; >> - #address-cells = <1>; >> - #size-cells = <1>; >> - >> - nand-on-flash-bbt; >> - }; >> - }; >> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml >> new file mode 100644 >> index 000000000000..965a2dd20645 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml >> @@ -0,0 +1,80 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mtd/amlogic,meson-nand.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs > > Maybe you need to inherit from nand-controller.yaml. ok > >> + >> +maintainers: >> + - liang.yang@amlogic.com >> + >> +properties: >> + compatible: >> + enum: >> + - "amlogic,meson-gxl-nfc" >> + - "amlogic,meson-axg-nfc" >> + >> + reg: >> + maxItems: 2 >> + >> + '#address-cells': >> + const: 1 > > Not sure this property is needed. this is for the subnode, such as nand@0. > >> + >> + '#size-cells': >> + const: 0 > > Ditto. Plus, this one looks wrong anyway. this is for the subnode, such as nand@0. do you mean s/''/""/? > >> + >> + reg-names: >> + items: >> + - const: nfc >> + - const: emmc > > Why do you need the emmc register map? Do you really need to perform a > register access there? yes, we have to access the emmc register map. because the NFC clock comes from SDEMMC_CLOCK register. > >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 2 >> + >> + clock-names: >> + items: >> + - const: core >> + - const: device >> + >> + "#clock-cells": >> + const: 1 > > ? en, it should be deleted here. > >> + >> +required: >> + - compatible >> + - reg >> + - '#address-cells' >> + - '#size-cells' >> + - reg-names >> + - interrupts >> + - clocks >> + - clock-names >> + >> +additionalProperties: false > > I will let Rob check that but I think what you need is > > unevaluatedProperties: false ok > >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/axg-clkc.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + apb { >> + #address-cells = <2>; >> + #size-cells = <2>; > > Not sure you need this upper node in the example. use the upper node to indicate the "#address-cells" and "#size-cells". if i do not do that, dt_binding_check will report: ".....reg:0: [0, 30720, 0, 256] is too long" and ".....reg:1: [0, 28672, 0, 2048] is too long". > >> + nand-controller@7800 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "amlogic,meson-axg-nfc"; >> + reg = <0x0 0x7800 0x0 0x100>, >> + <0x0 0x7000 0x0 0x800>; >> + reg-names = "nfc", "emmc"; >> + >> + interrupts = <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>; >> + clocks = <&clkc CLKID_SD_EMMC_C>, >> + <&clkc CLKID_FCLK_DIV2>; >> + clock-names = "core", "device"; >> + >> + }; >> + }; >> +... > > > Thanks, > Miquèl > > .
| |