Re: [PATCH 01/14] qcom: mtd: nand: Add driver data for QPIC DMA

From: Abhishek Sahu
Date: Mon Jul 17 2017 - 04:49:54 EST


On 2017-07-17 12:52, Boris Brezillon wrote:
On Mon, 17 Jul 2017 11:41:01 +0530
Abhishek Sahu <absahu@xxxxxxxxxxxxxx> wrote:

>> > +
>> > +nand@79b0000 {
>
> nand-controller@xxxx {
>
> BTW, glad to see another driver moving to the new DT representation
> :-).
>
>> > + compatible = "qcom,qpic-nandc-v1.4.0";
>> > + reg = <0x79b0000 0x1000>;
>> > +
>> > + clocks = <&gcc GCC_QPIC_CLK>,
>> > + <&gcc GCC_QPIC_AHB_CLK>;
>> > + clock-names = "core", "aon";
>> > +
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > +
>> > + nandcs@0 {
>
> nand@0 {
>
>> > + compatible = "qcom,nandcs";
>
> Why do you need a compatible here?
It is the part of original driver. We can connect multiple
NAND devices in the same bus and qcom,nandcs is being used
for each connected NAND device. Each NAND device can use
different chip select, ecc strength etc which we can specify
under this sub node.


Still don't understand why you need a compatible? Is this a memory bus
where you can connect other kind of memories (parallel NORs,
SRAMs, ...)?

If that's not the case, then considering all subnodes of the
nand-controller node containing a reg property as NAND devices is fine,
you don't need this compatible = "nand,cs" (see sunxi-nand bindings
[1]).


Thanks Boris for giving detailed references.

We can connect other parallel devices also but we have
different hardware wrappers over generic EBI2/QPIC which
will MUX/arbitrate the device access from the hardware itself.
So this NAND driver will only control multiple NAND devices.

We can remove this compatible and use the bindings
similar to sunxi-nand. I will do the required changes and
will post in v2 of the same patch series.

If the bus is generic and can be attached non-NAND devices, I'd
recommend looking at atmel's binding [2], because you're likely to
have one instance of the NAND controller logic for all NAND devices
connected on this bus.
And more importantly, if the bus a generic, the node should not be
named nand or nand-controller, and the compatible should not contain
'nandc' in it.

[1]http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt#L34
[2]http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mtd/atmel-nand.txt#L70

--
Abhishek Sahu