Hi Mason, masonccyang@xxxxxxxxxxx wrote on Fri, 28 Jun 2019 17:09:16 +0800: > Hi Miquel, > > > > > Please always Cc: Rob (robh+dt@xxxxxxxxxx) when you send bindings > > related patches. > > Understood. thanks for your remind. > > > > > > > > > > > > > > +- reg: should contain 1 entrie for the registers > > > > > > > > entry > > > > > > > > > +- reg-names: should contain "regs" > > > > > > > > Not sure you need that? > > > > > > for a base address of ctlr registers. > > > > Yes I know, I mean: you don't necessarily need the 'reg-names' property > > as it is supposed that the only entry will be the IP registers (unless > > there are more). I don't know what's Rob preference here but I would > > either drop the reg-names property or enhance the name, "regs" is > > terribly not descriptive. > > Got it, any comment is appreciated for either drop the reg-names property > or enhance the name. > > > > > > > > +- interrupts: interrupt line connected to this NAND controller > > > > > +- clock-names: should contain "ps_clk", "send_clk" and > "send_dly_clk" > > > > > +- clocks: should contain 3 entries for the "ps_clk", "send_clk" > and > > > > > + "send_dly_clk" clocks > > > > > > > > s/entries/phandles/ ? > > > > > > ? > > > as I know that kernel views the phandle values as device tree > structure > > > information instead of device tree data and thus does not store them > as > > > properties. > > > > The bindings have nothing to do with the kernel views. They might > > actually be merged in a different project, out of the kernel. > > > > if patch to phandle, should we also patch driver to of_xxx_phandle()? I don't understand your question. <&clk 1> is a phandle, you already use phandles, it's just more precise than the word "entries". Thanks, Miquèl