Messages in this thread | | | Date | Wed, 20 Apr 2022 14:16:20 +0200 | From | Miquel Raynal <> | Subject | Re: [PATCH v4 2/2] dt-bindings: nand: meson: refine Amlogic NAND controller driver |
| |
Hi Liang,
> >> +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.
Yes but if you refer to nand-controller.yaml you no longer need these.
> > > >> + > >> + '#size-cells': > >> + const: 0 > > > > Ditto. Plus, this one looks wrong anyway. > this is for the subnode, such as nand@0. do you mean s/''/""/?
Sorry, this is not "wrong anyway", my fault. But still, you don't need this property for the same reason as above.
> > > >> + > >> + 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.
But if it's a clock you should get the clock and call clk_prepare_enable(), you don't need to poke directly in the registers. Do you?
> >> +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".
ok, maybe, I'll let bindings maintainer review that.
> > > >> + 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
| |