[PATCH v5 3/3] dt/bindings: qcom_nandc: Add DT bindings

[Rob Herring]

On Wed, Jan 6, 2016 at 9:37 AM, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> On Wed, 6 Jan 2016 09:14:44 -0600
> Rob Herring <robh at 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 at vger.kernel.org
>> > Cc: Rob Herring <robh at kernel.org>
>> > Signed-off-by: Archit Taneja <architt at 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