Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Marek Vasut
On 12/05/2016 09:36 AM, Boris Brezillon wrote:
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut  wrote:
> 
>> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
>>> This patch adds the dts binding document for arasan nand flash
>>> controller.
>>>
>>> Signed-off-by: Punnaiah Choudary Kalluri 
>>> Acked-by: Rob Herring 
>>> ---
>>> changes in v6:
>>> - Removed num-cs property
>>> - Separated nandchip from nand controller
>>> changes in v5:
>>> - None
>>> Changes in v4:
>>> - Added num-cs property
>>> - Added clock support
>>> Changes in v3:
>>> - None
>>> Changes in v2:
>>> - None
>>> ---
>>>  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
>>> ++
>>>  1 file changed, 38 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
>>> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> new file mode 100644
>>> index 000..dcbe7ad
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> @@ -0,0 +1,38 @@
>>> +Arasan Nand Flash Controller with ONFI 3.1 support  
>>
>> Arasan NAND Flash ...
>>
>>> +Required properties:
>>> +- compatible: Should be "arasan,nfc-v3p10"  
>>
>> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
>> some fallback option which doesn't encode IP version in the compat
>> string ?
> 
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific
> compatible strings is a good solution to avoid breaking the DT ABI.

Can we detect it at runtime with this hardware ?

>>
>> Also, shouldn't quirks be handled by DT props instead of effectively
>> encoding them into the compatible string ?
> 
> Well, from my experience, it's better to hide as much as possible
> behind the compatible. This way, if new quirks are needed for a
> specific revision, you can update the driver without having to change
> the DT.

That's a good point, yes.

-- 
Best regards,
Marek Vasut


Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Marek Vasut
On 12/05/2016 09:36 AM, Boris Brezillon wrote:
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut  wrote:
> 
>> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
>>> This patch adds the dts binding document for arasan nand flash
>>> controller.
>>>
>>> Signed-off-by: Punnaiah Choudary Kalluri 
>>> Acked-by: Rob Herring 
>>> ---
>>> changes in v6:
>>> - Removed num-cs property
>>> - Separated nandchip from nand controller
>>> changes in v5:
>>> - None
>>> Changes in v4:
>>> - Added num-cs property
>>> - Added clock support
>>> Changes in v3:
>>> - None
>>> Changes in v2:
>>> - None
>>> ---
>>>  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
>>> ++
>>>  1 file changed, 38 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
>>> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> new file mode 100644
>>> index 000..dcbe7ad
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> @@ -0,0 +1,38 @@
>>> +Arasan Nand Flash Controller with ONFI 3.1 support  
>>
>> Arasan NAND Flash ...
>>
>>> +Required properties:
>>> +- compatible: Should be "arasan,nfc-v3p10"  
>>
>> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
>> some fallback option which doesn't encode IP version in the compat
>> string ?
> 
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific
> compatible strings is a good solution to avoid breaking the DT ABI.

Can we detect it at runtime with this hardware ?

>>
>> Also, shouldn't quirks be handled by DT props instead of effectively
>> encoding them into the compatible string ?
> 
> Well, from my experience, it's better to hide as much as possible
> behind the compatible. This way, if new quirks are needed for a
> specific revision, you can update the driver without having to change
> the DT.

That's a good point, yes.

-- 
Best regards,
Marek Vasut


RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Punnaiah Choudary Kalluri


> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@free-electrons.com]
> Sent: Monday, December 05, 2016 2:07 PM
> To: Marek Vasut <marek.va...@gmail.com>
> Cc: Punnaiah Choudary Kalluri <punn...@xilinx.com>;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> cyrille.pitc...@atmel.com; robh...@kernel.org; mark.rutl...@arm.com;
> linux-kernel@vger.kernel.org; linux-...@lists.infradead.org;
> devicet...@vger.kernel.org; Michal Simek <mich...@xilinx.com>;
> kalluripunnaiahchoud...@gmail.com; kpc...@gmail.com; Punnaiah
> Choudary Kalluri <punn...@xilinx.com>
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut <marek.va...@gmail.com> wrote:
> 
> > On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > > This patch adds the dts binding document for arasan nand flash
> > > controller.
> > >
> > > Signed-off-by: Punnaiah Choudary Kalluri <punn...@xilinx.com>
> > > Acked-by: Rob Herring <r...@kernel.org>
> > > ---
> > > changes in v6:
> > > - Removed num-cs property
> > > - Separated nandchip from nand controller changes in v5:
> > > - None
> > > Changes in v4:
> > > - Added num-cs property
> > > - Added clock support
> > > Changes in v3:
> > > - None
> > > Changes in v2:
> > > - None
> > > ---
> > >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38
> ++
> > >  1 file changed, 38 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > new file mode 100644
> > > index 000..dcbe7ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > @@ -0,0 +1,38 @@
> > > +Arasan Nand Flash Controller with ONFI 3.1 support
> >
> > Arasan NAND Flash ...
> >
> > > +Required properties:
> > > +- compatible: Should be "arasan,nfc-v3p10"
> >
> > This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> > some fallback option which doesn't encode IP version in the compat
> > string ?
> 
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific compatible
> strings is a good solution to avoid breaking the DT ABI.

Yes. I am also thinking the same. Arasan controller doesn't have the register
to indicate the IP version number.

> 
> >
> > Also, shouldn't quirks be handled by DT props instead of effectively
> > encoding them into the compatible string ?
> 
> Well, from my experience, it's better to hide as much as possible behind the
> compatible. This way, if new quirks are needed for a specific revision, you 
> can
> update the driver without having to change the DT.
> 

Agree.

Regards,
Punnaiah

> >
> > > +- reg: Memory map for module access
> > > +- interrupt-parent: Interrupt controller the interrupt is routed
> > > +through
> > > +- interrupts: Should contain the interrupt for the device
> > > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > > +   (See clock bindings for details)
> > > +- clocks: Clock phandles (see clock bindings for details)
> > > +
> > > +Optional properties:
> > > +- arasan,has-mdma: Enables Dma support
> >
> > 'Enables DMA support' , with DMA in caps.
> >
> > > +for nand partition information please refer the below file
> >
> > For NAND ...
> >
> > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > +
> > > +Example:
> > > + nand0: nand@ff10 {
> > > + compatible = "arasan,nfc-v3p10"
> > > + reg = <0x0 0xff10 0x1000>;
> > > + clock-name = "clk_sys", "clk_flash"
> > > + clocks = <_clk _clk>;
> > > + interrupt-parent = <>;
> > > + interrupts = <0 14 4>;
> > > + arasan,has-mdma;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>
> > > +
> > > + nand@0 {
> > > + reg = <0>
> > > + partition@0 {
> > > + label = "filesystem";
> > > + reg = <0x0 0x0 0x100>;
> > > + };
> > > + (...)
> > > + };
> > > + };
> > >
> >
> >



RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Punnaiah Choudary Kalluri


> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@free-electrons.com]
> Sent: Monday, December 05, 2016 2:07 PM
> To: Marek Vasut 
> Cc: Punnaiah Choudary Kalluri ;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> cyrille.pitc...@atmel.com; robh...@kernel.org; mark.rutl...@arm.com;
> linux-kernel@vger.kernel.org; linux-...@lists.infradead.org;
> devicet...@vger.kernel.org; Michal Simek ;
> kalluripunnaiahchoud...@gmail.com; kpc...@gmail.com; Punnaiah
> Choudary Kalluri 
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut  wrote:
> 
> > On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > > This patch adds the dts binding document for arasan nand flash
> > > controller.
> > >
> > > Signed-off-by: Punnaiah Choudary Kalluri 
> > > Acked-by: Rob Herring 
> > > ---
> > > changes in v6:
> > > - Removed num-cs property
> > > - Separated nandchip from nand controller changes in v5:
> > > - None
> > > Changes in v4:
> > > - Added num-cs property
> > > - Added clock support
> > > Changes in v3:
> > > - None
> > > Changes in v2:
> > > - None
> > > ---
> > >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38
> ++
> > >  1 file changed, 38 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > new file mode 100644
> > > index 000..dcbe7ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > @@ -0,0 +1,38 @@
> > > +Arasan Nand Flash Controller with ONFI 3.1 support
> >
> > Arasan NAND Flash ...
> >
> > > +Required properties:
> > > +- compatible: Should be "arasan,nfc-v3p10"
> >
> > This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> > some fallback option which doesn't encode IP version in the compat
> > string ?
> 
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific compatible
> strings is a good solution to avoid breaking the DT ABI.

Yes. I am also thinking the same. Arasan controller doesn't have the register
to indicate the IP version number.

> 
> >
> > Also, shouldn't quirks be handled by DT props instead of effectively
> > encoding them into the compatible string ?
> 
> Well, from my experience, it's better to hide as much as possible behind the
> compatible. This way, if new quirks are needed for a specific revision, you 
> can
> update the driver without having to change the DT.
> 

Agree.

Regards,
Punnaiah

> >
> > > +- reg: Memory map for module access
> > > +- interrupt-parent: Interrupt controller the interrupt is routed
> > > +through
> > > +- interrupts: Should contain the interrupt for the device
> > > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > > +   (See clock bindings for details)
> > > +- clocks: Clock phandles (see clock bindings for details)
> > > +
> > > +Optional properties:
> > > +- arasan,has-mdma: Enables Dma support
> >
> > 'Enables DMA support' , with DMA in caps.
> >
> > > +for nand partition information please refer the below file
> >
> > For NAND ...
> >
> > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > +
> > > +Example:
> > > + nand0: nand@ff10 {
> > > + compatible = "arasan,nfc-v3p10"
> > > + reg = <0x0 0xff10 0x1000>;
> > > + clock-name = "clk_sys", "clk_flash"
> > > + clocks = <_clk _clk>;
> > > + interrupt-parent = <>;
> > > + interrupts = <0 14 4>;
> > > + arasan,has-mdma;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>
> > > +
> > > + nand@0 {
> > > + reg = <0>
> > > + partition@0 {
> > > + label = "filesystem";
> > > + reg = <0x0 0x0 0x100>;
> > > + };
> > > + (...)
> > > + };
> > > + };
> > >
> >
> >



RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Punnaiah Choudary Kalluri
Hi Marek,

 Thanks for the review.

> -Original Message-
> From: Marek Vasut [mailto:marek.va...@gmail.com]
> Sent: Monday, December 05, 2016 9:56 AM
> To: Punnaiah Choudary Kalluri <punn...@xilinx.com>;
> dw...@infradead.org; computersforpe...@gmail.com;
> boris.brezil...@free-electrons.com; rich...@nod.at;
> cyrille.pitc...@atmel.com; robh...@kernel.org; mark.rutl...@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org;
> devicet...@vger.kernel.org; Michal Simek <mich...@xilinx.com>;
> kalluripunnaiahchoud...@gmail.com; kpc...@gmail.com; Punnaiah
> Choudary Kalluri <punn...@xilinx.com>
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > This patch adds the dts binding document for arasan nand flash
> > controller.
> >
> > Signed-off-by: Punnaiah Choudary Kalluri <punn...@xilinx.com>
> > Acked-by: Rob Herring <r...@kernel.org>
> > ---
> > changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller
> > changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38
> ++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > new file mode 100644
> > index 000..dcbe7ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > @@ -0,0 +1,38 @@
> > +Arasan Nand Flash Controller with ONFI 3.1 support
> 
> Arasan NAND Flash ...
> 
> > +Required properties:
> > +- compatible: Should be "arasan,nfc-v3p10"
> 
> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> some fallback option which doesn't encode IP version in the compat
> string ?
> 

This is a third-party IP and v3p10 is the version that we are using in our SOC.
Also this IP doesn’t have the IP version information in the register space to
read dynamically and having generic compatible name. So, any new versions
can be added through  of_match_table with config data inside the driver.

> Also, shouldn't quirks be handled by DT props instead of effectively
> encoding them into the compatible string ?
>
I feel the above mentioned method will be more appropriate rather than defining
the quirks through DT properties.

I will fix all other comments

Regards,
Punnaiah
> > +- reg: Memory map for module access
> > +- interrupt-parent: Interrupt controller the interrupt is routed through
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > + (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Optional properties:
> > +- arasan,has-mdma: Enables Dma support
> 
> 'Enables DMA support' , with DMA in caps.
> 
> > +for nand partition information please refer the below file
> 
> For NAND ...
> 
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > +   nand0: nand@ff10 {
> > +   compatible = "arasan,nfc-v3p10"
> > +   reg = <0x0 0xff10 0x1000>;
> > +   clock-name = "clk_sys", "clk_flash"
> > +   clocks = <_clk _clk>;
> > +   interrupt-parent = <>;
> > +   interrupts = <0 14 4>;
> > +   arasan,has-mdma;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>
> > +
> > +   nand@0 {
> > +   reg = <0>
> > +   partition@0 {
> > +   label = "filesystem";
> > +   reg = <0x0 0x0 0x100>;
> > +   };
> > +   (...)
> > +   };
> > +   };
> >
> 
> 
> --
> Best regards,
> Marek Vasut


RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Punnaiah Choudary Kalluri
Hi Marek,

 Thanks for the review.

> -Original Message-
> From: Marek Vasut [mailto:marek.va...@gmail.com]
> Sent: Monday, December 05, 2016 9:56 AM
> To: Punnaiah Choudary Kalluri ;
> dw...@infradead.org; computersforpe...@gmail.com;
> boris.brezil...@free-electrons.com; rich...@nod.at;
> cyrille.pitc...@atmel.com; robh...@kernel.org; mark.rutl...@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-...@lists.infradead.org;
> devicet...@vger.kernel.org; Michal Simek ;
> kalluripunnaiahchoud...@gmail.com; kpc...@gmail.com; Punnaiah
> Choudary Kalluri 
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > This patch adds the dts binding document for arasan nand flash
> > controller.
> >
> > Signed-off-by: Punnaiah Choudary Kalluri 
> > Acked-by: Rob Herring 
> > ---
> > changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller
> > changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38
> ++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > new file mode 100644
> > index 000..dcbe7ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > @@ -0,0 +1,38 @@
> > +Arasan Nand Flash Controller with ONFI 3.1 support
> 
> Arasan NAND Flash ...
> 
> > +Required properties:
> > +- compatible: Should be "arasan,nfc-v3p10"
> 
> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> some fallback option which doesn't encode IP version in the compat
> string ?
> 

This is a third-party IP and v3p10 is the version that we are using in our SOC.
Also this IP doesn’t have the IP version information in the register space to
read dynamically and having generic compatible name. So, any new versions
can be added through  of_match_table with config data inside the driver.

> Also, shouldn't quirks be handled by DT props instead of effectively
> encoding them into the compatible string ?
>
I feel the above mentioned method will be more appropriate rather than defining
the quirks through DT properties.

I will fix all other comments

Regards,
Punnaiah
> > +- reg: Memory map for module access
> > +- interrupt-parent: Interrupt controller the interrupt is routed through
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > + (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Optional properties:
> > +- arasan,has-mdma: Enables Dma support
> 
> 'Enables DMA support' , with DMA in caps.
> 
> > +for nand partition information please refer the below file
> 
> For NAND ...
> 
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > +   nand0: nand@ff10 {
> > +   compatible = "arasan,nfc-v3p10"
> > +   reg = <0x0 0xff10 0x1000>;
> > +   clock-name = "clk_sys", "clk_flash"
> > +   clocks = <_clk _clk>;
> > +   interrupt-parent = <>;
> > +   interrupts = <0 14 4>;
> > +   arasan,has-mdma;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>
> > +
> > +   nand@0 {
> > +   reg = <0>
> > +   partition@0 {
> > +   label = "filesystem";
> > +   reg = <0x0 0x0 0x100>;
> > +   };
> > +   (...)
> > +   };
> > +   };
> >
> 
> 
> --
> Best regards,
> Marek Vasut


Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Boris Brezillon
On Mon, 5 Dec 2016 05:25:54 +0100
Marek Vasut  wrote:

> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > This patch adds the dts binding document for arasan nand flash
> > controller.
> > 
> > Signed-off-by: Punnaiah Choudary Kalluri 
> > Acked-by: Rob Herring 
> > ---
> > changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller
> > changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
> > ++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
> > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > new file mode 100644
> > index 000..dcbe7ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > @@ -0,0 +1,38 @@
> > +Arasan Nand Flash Controller with ONFI 3.1 support  
> 
> Arasan NAND Flash ...
> 
> > +Required properties:
> > +- compatible: Should be "arasan,nfc-v3p10"  
> 
> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> some fallback option which doesn't encode IP version in the compat
> string ?

Not necessarily. Usually you define a generic compatible when you have
other reliable means to detect the IP version (a version register for
example).
If you can't detect that at runtime, then providing only specific
compatible strings is a good solution to avoid breaking the DT ABI.

> 
> Also, shouldn't quirks be handled by DT props instead of effectively
> encoding them into the compatible string ?

Well, from my experience, it's better to hide as much as possible
behind the compatible. This way, if new quirks are needed for a
specific revision, you can update the driver without having to change
the DT.

> 
> > +- reg: Memory map for module access
> > +- interrupt-parent: Interrupt controller the interrupt is routed through
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > + (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Optional properties:
> > +- arasan,has-mdma: Enables Dma support  
> 
> 'Enables DMA support' , with DMA in caps.
> 
> > +for nand partition information please refer the below file  
> 
> For NAND ...
> 
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > +   nand0: nand@ff10 {
> > +   compatible = "arasan,nfc-v3p10"
> > +   reg = <0x0 0xff10 0x1000>;
> > +   clock-name = "clk_sys", "clk_flash"
> > +   clocks = <_clk _clk>;
> > +   interrupt-parent = <>;
> > +   interrupts = <0 14 4>;
> > +   arasan,has-mdma;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>
> > +
> > +   nand@0 {
> > +   reg = <0>
> > +   partition@0 {
> > +   label = "filesystem";
> > +   reg = <0x0 0x0 0x100>;
> > +   };
> > +   (...)
> > +   };
> > +   };
> >   
> 
> 



Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-05 Thread Boris Brezillon
On Mon, 5 Dec 2016 05:25:54 +0100
Marek Vasut  wrote:

> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > This patch adds the dts binding document for arasan nand flash
> > controller.
> > 
> > Signed-off-by: Punnaiah Choudary Kalluri 
> > Acked-by: Rob Herring 
> > ---
> > changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller
> > changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> >  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
> > ++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
> > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > new file mode 100644
> > index 000..dcbe7ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > @@ -0,0 +1,38 @@
> > +Arasan Nand Flash Controller with ONFI 3.1 support  
> 
> Arasan NAND Flash ...
> 
> > +Required properties:
> > +- compatible: Should be "arasan,nfc-v3p10"  
> 
> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> some fallback option which doesn't encode IP version in the compat
> string ?

Not necessarily. Usually you define a generic compatible when you have
other reliable means to detect the IP version (a version register for
example).
If you can't detect that at runtime, then providing only specific
compatible strings is a good solution to avoid breaking the DT ABI.

> 
> Also, shouldn't quirks be handled by DT props instead of effectively
> encoding them into the compatible string ?

Well, from my experience, it's better to hide as much as possible
behind the compatible. This way, if new quirks are needed for a
specific revision, you can update the driver without having to change
the DT.

> 
> > +- reg: Memory map for module access
> > +- interrupt-parent: Interrupt controller the interrupt is routed through
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > + (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Optional properties:
> > +- arasan,has-mdma: Enables Dma support  
> 
> 'Enables DMA support' , with DMA in caps.
> 
> > +for nand partition information please refer the below file  
> 
> For NAND ...
> 
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > +   nand0: nand@ff10 {
> > +   compatible = "arasan,nfc-v3p10"
> > +   reg = <0x0 0xff10 0x1000>;
> > +   clock-name = "clk_sys", "clk_flash"
> > +   clocks = <_clk _clk>;
> > +   interrupt-parent = <>;
> > +   interrupts = <0 14 4>;
> > +   arasan,has-mdma;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>
> > +
> > +   nand@0 {
> > +   reg = <0>
> > +   partition@0 {
> > +   label = "filesystem";
> > +   reg = <0x0 0x0 0x100>;
> > +   };
> > +   (...)
> > +   };
> > +   };
> >   
> 
> 



Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-04 Thread Marek Vasut
On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> This patch adds the dts binding document for arasan nand flash
> controller.
> 
> Signed-off-by: Punnaiah Choudary Kalluri 
> Acked-by: Rob Herring 
> ---
> changes in v6:
> - Removed num-cs property
> - Separated nandchip from nand controller
> changes in v5:
> - None
> Changes in v4:
> - Added num-cs property
> - Added clock support
> Changes in v3:
> - None
> Changes in v2:
> - None
> ---
>  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> new file mode 100644
> index 000..dcbe7ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> @@ -0,0 +1,38 @@
> +Arasan Nand Flash Controller with ONFI 3.1 support

Arasan NAND Flash ...

> +Required properties:
> +- compatible: Should be "arasan,nfc-v3p10"

This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
some fallback option which doesn't encode IP version in the compat
string ?

Also, shouldn't quirks be handled by DT props instead of effectively
encoding them into the compatible string ?

> +- reg: Memory map for module access
> +- interrupt-parent: Interrupt controller the interrupt is routed through
> +- interrupts: Should contain the interrupt for the device
> +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> +   (See clock bindings for details)
> +- clocks: Clock phandles (see clock bindings for details)
> +
> +Optional properties:
> +- arasan,has-mdma: Enables Dma support

'Enables DMA support' , with DMA in caps.

> +for nand partition information please refer the below file

For NAND ...

> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> + nand0: nand@ff10 {
> + compatible = "arasan,nfc-v3p10"
> + reg = <0x0 0xff10 0x1000>;
> + clock-name = "clk_sys", "clk_flash"
> + clocks = <_clk _clk>;
> + interrupt-parent = <>;
> + interrupts = <0 14 4>;
> + arasan,has-mdma;
> + #address-cells = <1>;
> + #size-cells = <0>
> +
> + nand@0 {
> + reg = <0>
> + partition@0 {
> + label = "filesystem";
> + reg = <0x0 0x0 0x100>;
> + };
> + (...)
> + };
> + };
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation

2016-12-04 Thread Marek Vasut
On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> This patch adds the dts binding document for arasan nand flash
> controller.
> 
> Signed-off-by: Punnaiah Choudary Kalluri 
> Acked-by: Rob Herring 
> ---
> changes in v6:
> - Removed num-cs property
> - Separated nandchip from nand controller
> changes in v5:
> - None
> Changes in v4:
> - Added num-cs property
> - Added clock support
> Changes in v3:
> - None
> Changes in v2:
> - None
> ---
>  .../devicetree/bindings/mtd/arasan_nfc.txt | 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt 
> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> new file mode 100644
> index 000..dcbe7ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> @@ -0,0 +1,38 @@
> +Arasan Nand Flash Controller with ONFI 3.1 support

Arasan NAND Flash ...

> +Required properties:
> +- compatible: Should be "arasan,nfc-v3p10"

This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
some fallback option which doesn't encode IP version in the compat
string ?

Also, shouldn't quirks be handled by DT props instead of effectively
encoding them into the compatible string ?

> +- reg: Memory map for module access
> +- interrupt-parent: Interrupt controller the interrupt is routed through
> +- interrupts: Should contain the interrupt for the device
> +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> +   (See clock bindings for details)
> +- clocks: Clock phandles (see clock bindings for details)
> +
> +Optional properties:
> +- arasan,has-mdma: Enables Dma support

'Enables DMA support' , with DMA in caps.

> +for nand partition information please refer the below file

For NAND ...

> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> + nand0: nand@ff10 {
> + compatible = "arasan,nfc-v3p10"
> + reg = <0x0 0xff10 0x1000>;
> + clock-name = "clk_sys", "clk_flash"
> + clocks = <_clk _clk>;
> + interrupt-parent = <>;
> + interrupts = <0 14 4>;
> + arasan,has-mdma;
> + #address-cells = <1>;
> + #size-cells = <0>
> +
> + nand@0 {
> + reg = <0>
> + partition@0 {
> + label = "filesystem";
> + reg = <0x0 0x0 0x100>;
> + };
> + (...)
> + };
> + };
> 


-- 
Best regards,
Marek Vasut