| Message ID | 1561443056-13766-3-git-send-email-masonccyang@mxic.com.tw |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | Add Macronix Raw NAND controller driver | expand |
Hi Mason, Mason Yang <masonccyang@mxic.com.tw> wrote on Tue, 25 Jun 2019 14:10:56 +0800: > Document the bindings used by the Macronix raw NAND controller. > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > --- > .../devicetree/bindings/mtd/mxic-nand.txt | 26 ++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/mxic-nand.txt > > diff --git a/Documentation/devicetree/bindings/mtd/mxic-nand.txt b/Documentation/devicetree/bindings/mtd/mxic-nand.txt > new file mode 100644 > index 0000000..3d198e4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/mxic-nand.txt > @@ -0,0 +1,26 @@ > +Macronix Raw NAND Controller Device Tree Bindings > +------------------------------------------------- > + > +Required properties: > +- compatible: should be "mxic,raw-nand-ctlr" I would prefer "macronix,nand-controller" > +- reg: should contain 1 entrie for the registers entry > +- reg-names: should contain "regs" Not sure you need that? > +- 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/ ? > + > +Example: > + > + nand: mxic-nfc@43c30000 { > + compatible = "mxic,raw-nand-ctlr"; > + reg = <0x43c30000 0x10000>; > + reg-names = "regs"; > + clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>; > + clock-names = "send_clk", "send_dly_clk", "ps_clk"; > + > + nand-ecc-mode = "soft"; > + nand-ecc-algo = "bch"; > + nand-ecc-step-size = <512>; > + nand-ecc-strength = <8>; The last 4 lines are probably not needed. > + }; Thanks, Miquèl
Hi Miquel, > > Document the bindings used by the Macronix raw NAND controller. > > > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > > --- > > .../devicetree/bindings/mtd/mxic-nand.txt | 26 ++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mtd/mxic-nand.txt > > > > diff --git a/Documentation/devicetree/bindings/mtd/mxic-nand.txt b/ > Documentation/devicetree/bindings/mtd/mxic-nand.txt > > new file mode 100644 > > index 0000000..3d198e4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/mxic-nand.txt > > @@ -0,0 +1,26 @@ > > +Macronix Raw NAND Controller Device Tree Bindings > > +------------------------------------------------- > > + > > +Required properties: > > +- compatible: should be "mxic,raw-nand-ctlr" > > I would prefer "macronix,nand-controller" okay, will patch it. > > > +- 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. > > > +- 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. > > > + > > +Example: > > + > > + nand: mxic-nfc@43c30000 { > > + compatible = "mxic,raw-nand-ctlr"; > > + reg = <0x43c30000 0x10000>; > > + reg-names = "regs"; > > + clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>; > > + clock-names = "send_clk", "send_dly_clk", "ps_clk"; > > + > > + nand-ecc-mode = "soft"; > > + nand-ecc-algo = "bch"; > > + nand-ecc-step-size = <512>; > > + nand-ecc-strength = <8>; > > The last 4 lines are probably not needed. okay, will remove them. thanks for your review. best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =====================================================================
Hi Mason, Please always Cc: Rob (robh+dt@kernel.org) when you send bindings related patches. masonccyang@mxic.com.tw wrote on Fri, 28 Jun 2019 14:48:02 +0800: > Hi Miquel, > > > > Document the bindings used by the Macronix raw NAND controller. > > > > > > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> > > > --- > > > .../devicetree/bindings/mtd/mxic-nand.txt | 26 > ++++++++++++++++++++++ > > > 1 file changed, 26 insertions(+) > > > create mode 100644 > Documentation/devicetree/bindings/mtd/mxic-nand.txt > > > > > > diff --git a/Documentation/devicetree/bindings/mtd/mxic-nand.txt b/ > > Documentation/devicetree/bindings/mtd/mxic-nand.txt > > > new file mode 100644 > > > index 0000000..3d198e4 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mtd/mxic-nand.txt > > > @@ -0,0 +1,26 @@ > > > +Macronix Raw NAND Controller Device Tree Bindings > > > +------------------------------------------------- > > > + > > > +Required properties: > > > +- compatible: should be "mxic,raw-nand-ctlr" > > > > I would prefer "macronix,nand-controller" > > okay, will patch it. > > > > > > +- 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. > > > +- 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. Thanks, Miquèl
Hi Miquel, > > Please always Cc: Rob (robh+dt@kernel.org) 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()? thanks & best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =====================================================================
Hi Mason, masonccyang@mxic.com.tw wrote on Fri, 28 Jun 2019 17:09:16 +0800: > Hi Miquel, > > > > > Please always Cc: Rob (robh+dt@kernel.org) 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
Hi Miquel, > > > > > > > > > +- 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". Oops, I misunderstood your meaning. thanks for your interpretation. best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =====================================================================
diff --git a/Documentation/devicetree/bindings/mtd/mxic-nand.txt b/Documentation/devicetree/bindings/mtd/mxic-nand.txt new file mode 100644 index 0000000..3d198e4 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/mxic-nand.txt @@ -0,0 +1,26 @@ +Macronix Raw NAND Controller Device Tree Bindings +------------------------------------------------- + +Required properties: +- compatible: should be "mxic,raw-nand-ctlr" +- reg: should contain 1 entrie for the registers +- reg-names: should contain "regs" +- 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 + +Example: + + nand: mxic-nfc@43c30000 { + compatible = "mxic,raw-nand-ctlr"; + reg = <0x43c30000 0x10000>; + reg-names = "regs"; + clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>; + clock-names = "send_clk", "send_dly_clk", "ps_clk"; + + nand-ecc-mode = "soft"; + nand-ecc-algo = "bch"; + nand-ecc-step-size = <512>; + nand-ecc-strength = <8>; + };
Document the bindings used by the Macronix raw NAND controller. Signed-off-by: Mason Yang <masonccyang@mxic.com.tw> --- .../devicetree/bindings/mtd/mxic-nand.txt | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/mxic-nand.txt