| Message ID | 1451971501-18160-4-git-send-email-architt@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Hi Archit, On Tue, 5 Jan 2016 10:55:01 +0530 Archit Taneja <architt@codeaurora.org> wrote: > Add DT bindings document for the Qualcomm NAND controller driver. > > Cc: devicetree@vger.kernel.org > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Archit Taneja <architt@codeaurora.org> > --- > v5: > - Make changes to incorporate chip select sub nodes (brcmnand taken as > reference) > > v3: > - Don't use '0x' when specifying nand controller address space > - Add optional property for on-flash bbt usage > > .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > new file mode 100644 > index 0000000..b2cf2d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -0,0 +1,84 @@ > +* Qualcomm NAND controller > + > +Required properties: > +- compatible: should be "qcom,ebi2-nand" for IPQ806x > +- reg: MMIO address range > +- clocks: must contain core clock and always on clock > +- clock-names: must contain "core" for the core clock and "aon" for the > + always on clock > +- dmas: DMA specifier, consisting of a phandle to the ADM DMA > + controller node and the channel number to be used for > + NAND. Refer to dma.txt and qcom_adm.txt for more details > +- dma-names: must be "rxtx" > +- qcom,cmd-crci: must contain the ADM command type CRCI block instance > + number specified for the NAND controller on the given > + platform > +- qcom,data-crci: must contain the ADM data type CRCI block instance > + number specified for the NAND controller on the given > + platform > +- #address-cells: <1> - subnodes give the chip-select number > +- #size-cells: <0> > + > +* NAND chip-select > + > +Each controller may contain one or more subnodes to represent enabled > +chip-selects which (may) contain NAND flash chips. Their properties are as > +follows. > + > +Required properties: > +- compatible: should contain "qcom,nandcs" > +- reg: a single integer representing the chip-select > + number (e.g., 0, 1, 2, etc.) > +- #address-cells: see partition.txt > +- #size-cells: see partition.txt > +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 > + bits. > +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. > + > +Optional properties: > +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen > + as default You should probably reference Documentation/devicetree/bindings/mtd/nand.txt which is documenting generic nand-xxx properties. > + > +Each nandcs device node may optionally contain sub-nodes describing the flash > +partition mapping. See partition.txt for more detail. > + > +Example: > + > +nand@1ac00000 { > + compatible = "qcom,ebi2-nandc"; > + reg = <0x1ac00000 0x800>; > + > + clocks = <&gcc EBI2_CLK>, > + <&gcc EBI2_AON_CLK>; > + clock-names = "core", "aon"; > + > + dmas = <&adm_dma 3>; > + dma-names = "rxtx"; > + qcom,cmd-crci = <15>; > + qcom,data-crci = <3>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + nandcs@0 { > + compatible = "qcom,nandcs"; > + reg = <0>; > + > + nand-ecc-strength = <4>; > + nand-ecc-step-size = <512>; > + nand-bus-width = <8>; > + It's now recommended to define a 'partitions' subnode to store those partition nodes. > + #address-cells = <1>; > + #size-cells = <1>; > + > + partition@0 { > + label = "boot-nand"; > + reg = <0 0x58a0000>; > + }; > + > + partition@58a0000 { > + label = "fs-nand"; > + reg = <0x58a0000 0x4000000>; > + }; > + }; > +}; The rest looks good to me. Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com> Best Regards, Boris
On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote: > Add DT bindings document for the Qualcomm NAND controller driver. > > Cc: devicetree@vger.kernel.org > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Archit Taneja <architt@codeaurora.org> > --- > v5: > - Make changes to incorporate chip select sub nodes (brcmnand taken as > reference) > > v3: > - Don't use '0x' when specifying nand controller address space > - Add optional property for on-flash bbt usage > > .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > new file mode 100644 > index 0000000..b2cf2d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > @@ -0,0 +1,84 @@ > +* Qualcomm NAND controller > + > +Required properties: > +- compatible: should be "qcom,ebi2-nand" for IPQ806x More specific name please. > +- reg: MMIO address range > +- clocks: must contain core clock and always on clock > +- clock-names: must contain "core" for the core clock and "aon" for the > + always on clock > +- dmas: DMA specifier, consisting of a phandle to the ADM DMA > + controller node and the channel number to be used for > + NAND. Refer to dma.txt and qcom_adm.txt for more details > +- dma-names: must be "rxtx" > +- qcom,cmd-crci: must contain the ADM command type CRCI block instance > + number specified for the NAND controller on the given > + platform > +- qcom,data-crci: must contain the ADM data type CRCI block instance > + number specified for the NAND controller on the given > + platform > +- #address-cells: <1> - subnodes give the chip-select number > +- #size-cells: <0> > + > +* NAND chip-select > + > +Each controller may contain one or more subnodes to represent enabled > +chip-selects which (may) contain NAND flash chips. Their properties are as > +follows. > + > +Required properties: > +- compatible: should contain "qcom,nandcs" > +- reg: a single integer representing the chip-select > + number (e.g., 0, 1, 2, etc.) > +- #address-cells: see partition.txt > +- #size-cells: see partition.txt > +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 > + bits. > +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. > + > +Optional properties: > +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen > + as default > + > +Each nandcs device node may optionally contain sub-nodes describing the flash > +partition mapping. See partition.txt for more detail. Can't the partitioning span across chip selects? After all, interleaving is how you get high performance. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 6 Jan 2016 09:14:44 -0600 Rob Herring <robh@kernel.org> wrote: > On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote: > > Add DT bindings document for the Qualcomm NAND controller driver. > > > > Cc: devicetree@vger.kernel.org > > Cc: Rob Herring <robh@kernel.org> > > Signed-off-by: Archit Taneja <architt@codeaurora.org> > > --- > > v5: > > - Make changes to incorporate chip select sub nodes (brcmnand taken as > > reference) > > > > v3: > > - Don't use '0x' when specifying nand controller address space > > - Add optional property for on-flash bbt usage > > > > .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ > > 1 file changed, 84 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt > > > > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > > new file mode 100644 > > index 0000000..b2cf2d9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > > @@ -0,0 +1,84 @@ > > +* Qualcomm NAND controller > > + > > +Required properties: > > +- compatible: should be "qcom,ebi2-nand" for IPQ806x > > More specific name please. > > > +- reg: MMIO address range > > +- clocks: must contain core clock and always on clock > > +- clock-names: must contain "core" for the core clock and "aon" for the > > + always on clock > > +- dmas: DMA specifier, consisting of a phandle to the ADM DMA > > + controller node and the channel number to be used for > > + NAND. Refer to dma.txt and qcom_adm.txt for more details > > +- dma-names: must be "rxtx" > > +- qcom,cmd-crci: must contain the ADM command type CRCI block instance > > + number specified for the NAND controller on the given > > + platform > > +- qcom,data-crci: must contain the ADM data type CRCI block instance > > + number specified for the NAND controller on the given > > + platform > > +- #address-cells: <1> - subnodes give the chip-select number > > +- #size-cells: <0> > > + > > +* NAND chip-select > > + > > +Each controller may contain one or more subnodes to represent enabled > > +chip-selects which (may) contain NAND flash chips. Their properties are as > > +follows. > > + > > +Required properties: > > +- compatible: should contain "qcom,nandcs" > > +- reg: a single integer representing the chip-select > > + number (e.g., 0, 1, 2, etc.) > > +- #address-cells: see partition.txt > > +- #size-cells: see partition.txt > > +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 > > + bits. > > +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. > > + > > +Optional properties: > > +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen > > + as default > > + > > +Each nandcs device node may optionally contain sub-nodes describing the flash > > +partition mapping. See partition.txt for more detail. > > Can't the partitioning span across chip selects? After all, interleaving > is how you get high performance. Hm, defining aggregated mtd devices in the DT is not supported, and since, as I've been told many times ;), DT is supposed to represent the HW not what we want to do with it, I'm not sure that's such a good idea. Note that the infrastructure to concatenate several MTD devices already exists, but it's not used by the NAND layer. Also note that some NAND chips embed several dies and expose multiple CS lines. The NAND framework already supports assigning different CS lines to a single NAND device, so you could abuse this feature and expose your different NAND devices as a single one (that will only work for a cluster of identical chips connected to the same controller), but I'd really like to avoid this kind of things.
On Wed, Jan 6, 2016 at 9:37 AM, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Wed, 6 Jan 2016 09:14:44 -0600 > Rob Herring <robh@kernel.org> wrote: > >> On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote: >> > Add DT bindings document for the Qualcomm NAND controller driver. >> > >> > Cc: devicetree@vger.kernel.org >> > Cc: Rob Herring <robh@kernel.org> >> > Signed-off-by: Archit Taneja <architt@codeaurora.org> >> > --- >> > v5: >> > - Make changes to incorporate chip select sub nodes (brcmnand taken as >> > reference) >> > >> > v3: >> > - Don't use '0x' when specifying nand controller address space >> > - Add optional property for on-flash bbt usage >> > >> > .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ >> > 1 file changed, 84 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> > >> > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> > new file mode 100644 >> > index 0000000..b2cf2d9 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt >> > @@ -0,0 +1,84 @@ >> > +* Qualcomm NAND controller >> > + >> > +Required properties: >> > +- compatible: should be "qcom,ebi2-nand" for IPQ806x >> >> More specific name please. >> >> > +- reg: MMIO address range >> > +- clocks: must contain core clock and always on clock >> > +- clock-names: must contain "core" for the core clock and "aon" for the >> > + always on clock >> > +- dmas: DMA specifier, consisting of a phandle to the ADM DMA >> > + controller node and the channel number to be used for >> > + NAND. Refer to dma.txt and qcom_adm.txt for more details >> > +- dma-names: must be "rxtx" >> > +- qcom,cmd-crci: must contain the ADM command type CRCI block instance >> > + number specified for the NAND controller on the given >> > + platform >> > +- qcom,data-crci: must contain the ADM data type CRCI block instance >> > + number specified for the NAND controller on the given >> > + platform >> > +- #address-cells: <1> - subnodes give the chip-select number >> > +- #size-cells: <0> >> > + >> > +* NAND chip-select >> > + >> > +Each controller may contain one or more subnodes to represent enabled >> > +chip-selects which (may) contain NAND flash chips. Their properties are as >> > +follows. >> > + >> > +Required properties: >> > +- compatible: should contain "qcom,nandcs" >> > +- reg: a single integer representing the chip-select >> > + number (e.g., 0, 1, 2, etc.) >> > +- #address-cells: see partition.txt >> > +- #size-cells: see partition.txt >> > +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 >> > + bits. >> > +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. >> > + >> > +Optional properties: >> > +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen >> > + as default >> > + >> > +Each nandcs device node may optionally contain sub-nodes describing the flash >> > +partition mapping. See partition.txt for more detail. >> >> Can't the partitioning span across chip selects? After all, interleaving >> is how you get high performance. > > Hm, defining aggregated mtd devices in the DT is not supported, and > since, as I've been told many times ;), DT is supposed to represent the > HW not what we want to do with it, I'm not sure that's such a good idea. What are partitions then? > Note that the infrastructure to concatenate several MTD devices > already exists, but it's not used by the NAND layer. > > Also note that some NAND chips embed several dies and expose multiple > CS lines. The NAND framework already supports assigning different CS > lines to a single NAND device, so you could abuse this feature and > expose your different NAND devices as a single one (that will only work > for a cluster of identical chips connected to the same controller), but > I'd really like to avoid this kind of things. What exactly the kernel supports ATM is irrelevant. I'm fully aware that the kernel's NAND support has huge gaps in its ability to support raw NAND at typical SSD speeds. My last employer had delusions about doing that. Fortunately, NAND manufacturers don't really want to sell you raw NAND anyway. My point here was only that the partitions node may not be under the CS nodes, but at the same level. Or maybe partitions in DT just doesn't make sense at all for interleaved cases. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 6 Jan 2016 10:13:57 -0600 Rob Herring <robh@kernel.org> wrote: > On Wed, Jan 6, 2016 at 9:37 AM, Boris Brezillon > <boris.brezillon@free-electrons.com> wrote: > > On Wed, 6 Jan 2016 09:14:44 -0600 > > Rob Herring <robh@kernel.org> wrote: > > > >> On Tue, Jan 05, 2016 at 10:55:01AM +0530, Archit Taneja wrote: > >> > Add DT bindings document for the Qualcomm NAND controller driver. > >> > > >> > Cc: devicetree@vger.kernel.org > >> > Cc: Rob Herring <robh@kernel.org> > >> > Signed-off-by: Archit Taneja <architt@codeaurora.org> > >> > --- > >> > v5: > >> > - Make changes to incorporate chip select sub nodes (brcmnand taken as > >> > reference) > >> > > >> > v3: > >> > - Don't use '0x' when specifying nand controller address space > >> > - Add optional property for on-flash bbt usage > >> > > >> > .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ > >> > 1 file changed, 84 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt > >> > > >> > diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > >> > new file mode 100644 > >> > index 0000000..b2cf2d9 > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt > >> > @@ -0,0 +1,84 @@ > >> > +* Qualcomm NAND controller > >> > + > >> > +Required properties: > >> > +- compatible: should be "qcom,ebi2-nand" for IPQ806x > >> > >> More specific name please. > >> > >> > +- reg: MMIO address range > >> > +- clocks: must contain core clock and always on clock > >> > +- clock-names: must contain "core" for the core clock and "aon" for the > >> > + always on clock > >> > +- dmas: DMA specifier, consisting of a phandle to the ADM DMA > >> > + controller node and the channel number to be used for > >> > + NAND. Refer to dma.txt and qcom_adm.txt for more details > >> > +- dma-names: must be "rxtx" > >> > +- qcom,cmd-crci: must contain the ADM command type CRCI block instance > >> > + number specified for the NAND controller on the given > >> > + platform > >> > +- qcom,data-crci: must contain the ADM data type CRCI block instance > >> > + number specified for the NAND controller on the given > >> > + platform > >> > +- #address-cells: <1> - subnodes give the chip-select number > >> > +- #size-cells: <0> > >> > + > >> > +* NAND chip-select > >> > + > >> > +Each controller may contain one or more subnodes to represent enabled > >> > +chip-selects which (may) contain NAND flash chips. Their properties are as > >> > +follows. > >> > + > >> > +Required properties: > >> > +- compatible: should contain "qcom,nandcs" > >> > +- reg: a single integer representing the chip-select > >> > + number (e.g., 0, 1, 2, etc.) > >> > +- #address-cells: see partition.txt > >> > +- #size-cells: see partition.txt > >> > +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 > >> > + bits. > >> > +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. > >> > + > >> > +Optional properties: > >> > +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen > >> > + as default > >> > + > >> > +Each nandcs device node may optionally contain sub-nodes describing the flash > >> > +partition mapping. See partition.txt for more detail. > >> > >> Can't the partitioning span across chip selects? After all, interleaving > >> is how you get high performance. > > > > Hm, defining aggregated mtd devices in the DT is not supported, and > > since, as I've been told many times ;), DT is supposed to represent the > > HW not what we want to do with it, I'm not sure that's such a good idea. > > What are partitions then? It's definitely not describing the hardware, and I never said describing partitions in the DT was following the number one rule: "only describe your HW" ;-). I'm generally not in favor of this kind of restrictions, I'm just pointing the irony of the situation here :p. > > > Note that the infrastructure to concatenate several MTD devices > > already exists, but it's not used by the NAND layer. > > > > Also note that some NAND chips embed several dies and expose multiple > > CS lines. The NAND framework already supports assigning different CS > > lines to a single NAND device, so you could abuse this feature and > > expose your different NAND devices as a single one (that will only work > > for a cluster of identical chips connected to the same controller), but > > I'd really like to avoid this kind of things. > > What exactly the kernel supports ATM is irrelevant. I'm fully aware > that the kernel's NAND support has huge gaps in its ability to support > raw NAND at typical SSD speeds. My last employer had delusions about > doing that. Fortunately, NAND manufacturers don't really want to sell > you raw NAND anyway. > > My point here was only that the partitions node may not be under the > CS nodes, but at the same level. Or maybe partitions in DT just > doesn't make sense at all for interleaved cases. IMHO, if we had to support this aggregation feature, the NAND cluster should be represented using a different node, outside of the nand controller node. Theoretically, you can perfectly have several NAND chips connected to different NAND controllers, but still want to aggregate those chips into a single entity. Anyway, that's not the topic here, and since other bindings are already describing partitions under the nand device node and not the nand controller node, I think we can keep doing like this until we find a better solution. Best Regards, Boris
diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt new file mode 100644 index 0000000..b2cf2d9 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt @@ -0,0 +1,84 @@ +* Qualcomm NAND controller + +Required properties: +- compatible: should be "qcom,ebi2-nand" for IPQ806x +- reg: MMIO address range +- clocks: must contain core clock and always on clock +- clock-names: must contain "core" for the core clock and "aon" for the + always on clock +- dmas: DMA specifier, consisting of a phandle to the ADM DMA + controller node and the channel number to be used for + NAND. Refer to dma.txt and qcom_adm.txt for more details +- dma-names: must be "rxtx" +- qcom,cmd-crci: must contain the ADM command type CRCI block instance + number specified for the NAND controller on the given + platform +- qcom,data-crci: must contain the ADM data type CRCI block instance + number specified for the NAND controller on the given + platform +- #address-cells: <1> - subnodes give the chip-select number +- #size-cells: <0> + +* NAND chip-select + +Each controller may contain one or more subnodes to represent enabled +chip-selects which (may) contain NAND flash chips. Their properties are as +follows. + +Required properties: +- compatible: should contain "qcom,nandcs" +- reg: a single integer representing the chip-select + number (e.g., 0, 1, 2, etc.) +- #address-cells: see partition.txt +- #size-cells: see partition.txt +- nand-ecc-strength: number of bits to correct per ECC step. Must be 4 or 8 + bits. +- nand-ecc-step-size: bytes of data per ECC step. Must be 512. + +Optional properties: +- nand-bus-width: bus width. Must be 8 or 16. If not present, 8 is chosen + as default + +Each nandcs device node may optionally contain sub-nodes describing the flash +partition mapping. See partition.txt for more detail. + +Example: + +nand@1ac00000 { + compatible = "qcom,ebi2-nandc"; + reg = <0x1ac00000 0x800>; + + clocks = <&gcc EBI2_CLK>, + <&gcc EBI2_AON_CLK>; + clock-names = "core", "aon"; + + dmas = <&adm_dma 3>; + dma-names = "rxtx"; + qcom,cmd-crci = <15>; + qcom,data-crci = <3>; + + #address-cells = <1>; + #size-cells = <0>; + + nandcs@0 { + compatible = "qcom,nandcs"; + reg = <0>; + + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + nand-bus-width = <8>; + + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "boot-nand"; + reg = <0 0x58a0000>; + }; + + partition@58a0000 { + label = "fs-nand"; + reg = <0x58a0000 0x4000000>; + }; + }; +};
Add DT bindings document for the Qualcomm NAND controller driver. Cc: devicetree@vger.kernel.org Cc: Rob Herring <robh@kernel.org> Signed-off-by: Archit Taneja <architt@codeaurora.org> --- v5: - Make changes to incorporate chip select sub nodes (brcmnand taken as reference) v3: - Don't use '0x' when specifying nand controller address space - Add optional property for on-flash bbt usage .../devicetree/bindings/mtd/qcom_nandc.txt | 84 ++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt