Messages in this thread | | | Date | Wed, 20 Apr 2022 20:35:41 +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 20:16, Miquel Raynal wrote: > [ EXTERNAL EMAIL ] > > 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. ok, i will try it. > >>> >>>> + >>>> + '#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. ok. > >>> >>>> + >>>> + 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?
no, it doesn't. it is special and the reason why need to implement a MMC sub clock driver previously. also we don't implement it as a clock provider in drivers/clk/meson/, because the SDEMMC_CLOCK register is internal in MCI controller. > >>>> +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.
ok. thanks.
> >>> >>>> + 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 > > .
| |