Re: [PATCH v5 01/16] dt-bindings: mtd: gpmc-onenand: Update properties description

Re: [PATCH v5 01/16] dt-bindings: mtd: gpmc-onenand: Update properties description

[Boris Brezillon]

+Rob and the DT ML

On Wed, 15 Nov 2017 17:24:58 +0100
Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> wrote:

> Compatible property is required for OMAP2+ mtd driver. Also
> add INT pin gpio description and delete unused dma-channel
> property.
> 
> Signed-off-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Reviewed-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
> Acked-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
> 
> Changes in v5:
> - renamed R/B pin to INT pin
> 
> Changes in v4:
> - new patch
> 
> Changes in v3: None
> Changes in v2: None
> 
>  Documentation/devicetree/bindings/mtd/gpmc-onenand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> index b6e8bfd024f4..e9f01a963a0a 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> @@ -9,13 +9,14 @@ Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  Required properties:
>  
> + - compatible:		"ti,omap2-onenand"

Don't you break backward compat by adding this new requirement?

>   - reg:			The CS line the peripheral is connected to
> - - gpmc,device-width	Width of the ONENAND device connected to the GPMC
> + - gpmc,device-width:	Width of the ONENAND device connected to the GPMC
>  			in bytes. Must be 1 or 2.
>  
>  Optional properties:
>  
> - - dma-channel:		DMA Channel index
> + - int-gpios:		GPIO specifier for the INT pin.
>  
>  For inline partition table parsing (optional):
>  
> @@ -35,6 +36,7 @@ Example for an OMAP3430 board:
>  		#size-cells = <1>;
>  
>  		onenand@0 {
> +			compatible = "ti,omap2-onenand";
>  			reg = <0 0 0>; /* CS0, offset 0 */
>  			gpmc,device-width = <2>;
>  

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 01/16] dt-bindings: mtd: gpmc-onenand: Update properties description

[Ladislav Michl]

On Thu, Nov 30, 2017 at 11:25:44AM +0100, Boris Brezillon wrote:
> +Rob and the DT ML
> 
> On Wed, 15 Nov 2017 17:24:58 +0100
> Ladislav Michl <ladis@linux-mips.org> wrote:
> 
> > Compatible property is required for OMAP2+ mtd driver. Also
> > add INT pin gpio description and delete unused dma-channel
> > property.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > Acked-by: Roger Quadros <rogerq@ti.com>
> > ---
> > 
> > Changes in v5:
> > - renamed R/B pin to INT pin
> > 
> > Changes in v4:
> > - new patch
> > 
> > Changes in v3: None
> > Changes in v2: None
> > 
> >  Documentation/devicetree/bindings/mtd/gpmc-onenand.txt | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> > index b6e8bfd024f4..e9f01a963a0a 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> > @@ -9,13 +9,14 @@ Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
> >  
> >  Required properties:
> >  
> > + - compatible:		"ti,omap2-onenand"
> 
> Don't you break backward compat by adding this new requirement?

I was told it is okay in this case as OneNAND conversion to DT is unfinished.
There was compat fixup code in earlier versions of patchset and it was dropped
later. If others see it opposite way I can add it back.

> >   - reg:			The CS line the peripheral is connected to
> > - - gpmc,device-width	Width of the ONENAND device connected to the GPMC
> > + - gpmc,device-width:	Width of the ONENAND device connected to the GPMC
> >  			in bytes. Must be 1 or 2.
> >  
> >  Optional properties:
> >  
> > - - dma-channel:		DMA Channel index
> > + - int-gpios:		GPIO specifier for the INT pin.
> >  
> >  For inline partition table parsing (optional):
> >  
> > @@ -35,6 +36,7 @@ Example for an OMAP3430 board:
> >  		#size-cells = <1>;
> >  
> >  		onenand@0 {
> > +			compatible = "ti,omap2-onenand";
> >  			reg = <0 0 0>; /* CS0, offset 0 */
> >  			gpmc,device-width = <2>;
> >  

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v5 01/16] dt-bindings: mtd: gpmc-onenand: Update properties description

[Roger Quadros]

Ladislav,

On 30/11/17 15:09, Ladislav Michl wrote:
> On Thu, Nov 30, 2017 at 11:25:44AM +0100, Boris Brezillon wrote:
>> +Rob and the DT ML
>>
>> On Wed, 15 Nov 2017 17:24:58 +0100
>> Ladislav Michl <ladis@linux-mips.org> wrote:
>>
>>> Compatible property is required for OMAP2+ mtd driver. Also
>>> add INT pin gpio description and delete unused dma-channel
>>> property.
>>>
>>> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>>> Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> Acked-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>
>>> Changes in v5:
>>> - renamed R/B pin to INT pin
>>>
>>> Changes in v4:
>>> - new patch
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>  Documentation/devicetree/bindings/mtd/gpmc-onenand.txt | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>> index b6e8bfd024f4..e9f01a963a0a 100644
>>> --- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>> @@ -9,13 +9,14 @@ Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>>>  
>>>  Required properties:
>>>  
>>> + - compatible:		"ti,omap2-onenand"
>>
>> Don't you break backward compat by adding this new requirement?
> 
> I was told it is okay in this case as OneNAND conversion to DT is unfinished.
> There was compat fixup code in earlier versions of patchset and it was dropped
> later. If others see it opposite way I can add it back.
> 

If it is possible not to break backward compatibility then we shouldn't.
Only if things are getting real messy because of the poor way the earlier DT
implementation was done we can consider it as an exception.

I thought we rely on GPMC timings entirely from the DT now and so things don't
work with older DT nodes?

>>>   - reg:			The CS line the peripheral is connected to
>>> - - gpmc,device-width	Width of the ONENAND device connected to the GPMC
>>> + - gpmc,device-width:	Width of the ONENAND device connected to the GPMC
>>>  			in bytes. Must be 1 or 2.
>>>  
>>>  Optional properties:
>>>  
>>> - - dma-channel:		DMA Channel index
>>> + - int-gpios:		GPIO specifier for the INT pin.
>>>  
>>>  For inline partition table parsing (optional):
>>>  
>>> @@ -35,6 +36,7 @@ Example for an OMAP3430 board:
>>>  		#size-cells = <1>;
>>>  
>>>  		onenand@0 {
>>> +			compatible = "ti,omap2-onenand";
>>>  			reg = <0 0 0>; /* CS0, offset 0 */
>>>  			gpmc,device-width = <2>;
>>>  

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v5 01/16] dt-bindings: mtd: gpmc-onenand: Update properties description

[Ladislav Michl]

Roger,

On Fri, Dec 15, 2017 at 12:18:59PM +0200, Roger Quadros wrote:
> Ladislav,
> 
> On 30/11/17 15:09, Ladislav Michl wrote:
> > On Thu, Nov 30, 2017 at 11:25:44AM +0100, Boris Brezillon wrote:
> >> +Rob and the DT ML
> >>
> >> On Wed, 15 Nov 2017 17:24:58 +0100
> >> Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> wrote:
> >>
> >>> Compatible property is required for OMAP2+ mtd driver. Also
> >>> add INT pin gpio description and delete unused dma-channel
> >>> property.
> >>>
> >>> Signed-off-by: Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> >>> Reviewed-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
> >>> Acked-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> >>> ---
> >>>
> >>> Changes in v5:
> >>> - renamed R/B pin to INT pin
> >>>
> >>> Changes in v4:
> >>> - new patch
> >>>
> >>> Changes in v3: None
> >>> Changes in v2: None
> >>>
> >>>  Documentation/devicetree/bindings/mtd/gpmc-onenand.txt | 6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>> index b6e8bfd024f4..e9f01a963a0a 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>> @@ -9,13 +9,14 @@ Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
> >>>  
> >>>  Required properties:
> >>>  
> >>> + - compatible:		"ti,omap2-onenand"
> >>
> >> Don't you break backward compat by adding this new requirement?
> > 
> > I was told it is okay in this case as OneNAND conversion to DT is unfinished.
> > There was compat fixup code in earlier versions of patchset and it was dropped
> > later. If others see it opposite way I can add it back.
> > 
> 
> If it is possible not to break backward compatibility then we shouldn't.
> Only if things are getting real messy because of the poor way the earlier DT
> implementation was done we can consider it as an exception.

Well, it is possible to reintroduce fixup. Based on other posts on linux-omap
mailing list, it seems that n900, etc. support is still under heavy development,
so people are updating both kernel and dtb. That's why I happily agreed on
removing fixup.

Of course I'd like to hear opinion from n8x0 and n900 people and also would be
very happy if someone could finally test on OMAP2 device.

> I thought we rely on GPMC timings entirely from the DT now and so things don't
> work with older DT nodes?

Well, they do work as long as DT contains correct timings. Mainline driver just
ignores them a does calculations on its own, so it works even with wrong DT
timings as long as bootloader configured something reasonable (otherwise it
doe not work at all).

This is no longer the case. Now, driver is expecting reasonable timings, enough
good to get device probed. Driver then calculates optimized timings based on
actual OneNAND chip ised.

Thank you,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/16] OMAP2+ OneNAND driver update

[Ladislav Michl]

+Rob and the DT ML

On Wed, Jan 10, 2018 at 07:35:19PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Wed, Jan 10, 2018 at 10:20:58AM +0100, Ladislav Michl wrote:
> > On Tue, Jan 09, 2018 at 11:01:14PM +0200, Aaro Koskinen wrote:
> > > On Tue, Jan 09, 2018 at 02:08:12PM +0100, Ladislav Michl wrote:
> > > > Merge window is really close now and patches seems still to be untested on
> > > > n8x0. So unless someone is going to test it soon, I'll pick used N800 on
> > > > ebay :-/
> > > 
> > > Today I tried to test them on Nokia N810, and I get:
> > > 
> > > [    0.174804] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> > > [    0.174865] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > > [    0.177459] omap-gpmc 6800a000.gpmc: /ocp/gpmc@6800a000/onenand@0,0 has no 'bank-width' property
> > > [    0.177612] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22
> > > 
> > > With plain v4.15-rc7+ it probes fine.
> > 
> > Thank you for testing. As stated in cover letter, you need also
> > https://patchwork.kernel.org/patch/10043259/
> > which can be also found in Roger's branch
> > https://github.com/rogerq/linux/commits/for-v4.16/gpmc-omap-immutable
> > (see previous replies in this thread and also discussion about OneNAND
> > DT node)
> 
> I applied it, and now get:
> 
> [    0.174987] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> [    0.175048] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> [    0.177703] omap-gpmc 6800a000.gpmc: Incompatible OneNAND node: missing compatible
> [    0.177764] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22

As expected without reading discussion about OneNAND DT node :)

In short, this version expects "ti,omap2-onenand" compatible.

In earlier version of this patchset there was fixup present, so driver
worked even without compatible. Later on (17 Oct 2017 11:46:51 +0300)
Roger objected (https://www.spinics.net/lists/linux-omap/msg139576.html):
"I don't want to fixup broken DTs like this as they will go silently without
being fixed. Let's instead just error out after the dev_warn(). This will
force us to fix all DT nodes."
So fixup was removed. I know N900 support is under heavy development
in mainline, but if n8x0 is considered finished and stable, we should
reintroduce fixup. Thoughts?

Thank you for your patience,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/16] OMAP2+ OneNAND driver update

[Aaro Koskinen]

Hi,

On Wed, Jan 10, 2018 at 10:13:25PM +0100, Ladislav Michl wrote:
> On Wed, Jan 10, 2018 at 07:35:19PM +0200, Aaro Koskinen wrote:
> > On Wed, Jan 10, 2018 at 10:20:58AM +0100, Ladislav Michl wrote:
> > > On Tue, Jan 09, 2018 at 11:01:14PM +0200, Aaro Koskinen wrote:
> > > > On Tue, Jan 09, 2018 at 02:08:12PM +0100, Ladislav Michl wrote:
> > > > > Merge window is really close now and patches seems still to be untested on
> > > > > n8x0. So unless someone is going to test it soon, I'll pick used N800 on
> > > > > ebay :-/
> > > > 
> > > > Today I tried to test them on Nokia N810, and I get:
> > > > 
> > > > [    0.174804] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> > > > [    0.174865] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > > > [    0.177459] omap-gpmc 6800a000.gpmc: /ocp/gpmc@6800a000/onenand@0,0 has no 'bank-width' property
> > > > [    0.177612] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22
> > > > 
> > > > With plain v4.15-rc7+ it probes fine.
> > > 
> > > Thank you for testing. As stated in cover letter, you need also
> > > https://patchwork.kernel.org/patch/10043259/
> > > which can be also found in Roger's branch
> > > https://github.com/rogerq/linux/commits/for-v4.16/gpmc-omap-immutable
> > > (see previous replies in this thread and also discussion about OneNAND
> > > DT node)
> > 
> > I applied it, and now get:
> > 
> > [    0.174987] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> > [    0.175048] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > [    0.177703] omap-gpmc 6800a000.gpmc: Incompatible OneNAND node: missing compatible
> > [    0.177764] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22
> 
> As expected without reading discussion about OneNAND DT node :)
> 
> In short, this version expects "ti,omap2-onenand" compatible.

Right. I didn't realize by bootloader was passing the DTB from an old
kernel.

So after fixing my boot I got the patches working N800 and N810: I was
able to read and write on old JFFS2 file systems without any issues. So
you can add for the series:

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>

> In earlier version of this patchset there was fixup present, so driver
> worked even without compatible. Later on (17 Oct 2017 11:46:51 +0300)
> Roger objected (https://www.spinics.net/lists/linux-omap/msg139576.html):
> "I don't want to fixup broken DTs like this as they will go silently without
> being fixed. Let's instead just error out after the dev_warn(). This will
> force us to fix all DT nodes."
> So fixup was removed. I know N900 support is under heavy development
> in mainline, but if n8x0 is considered finished and stable, we should
> reintroduce fixup. Thoughts?

N8x0 support is also "under development", and we are fine with updating
and always using latest in-tree DTs.

A.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v5 00/16] OMAP2+ OneNAND driver update

[Ladislav Michl]

On Thu, Jan 11, 2018 at 09:29:31PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Wed, Jan 10, 2018 at 10:13:25PM +0100, Ladislav Michl wrote:
> > On Wed, Jan 10, 2018 at 07:35:19PM +0200, Aaro Koskinen wrote:
> > > On Wed, Jan 10, 2018 at 10:20:58AM +0100, Ladislav Michl wrote:
> > > > On Tue, Jan 09, 2018 at 11:01:14PM +0200, Aaro Koskinen wrote:
> > > > > On Tue, Jan 09, 2018 at 02:08:12PM +0100, Ladislav Michl wrote:
> > > > > > Merge window is really close now and patches seems still to be untested on
> > > > > > n8x0. So unless someone is going to test it soon, I'll pick used N800 on
> > > > > > ebay :-/
> > > > > 
> > > > > Today I tried to test them on Nokia N810, and I get:
> > > > > 
> > > > > [    0.174804] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> > > > > [    0.174865] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > > > > [    0.177459] omap-gpmc 6800a000.gpmc: /ocp/gpmc@6800a000/onenand@0,0 has no 'bank-width' property
> > > > > [    0.177612] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22
> > > > > 
> > > > > With plain v4.15-rc7+ it probes fine.
> > > > 
> > > > Thank you for testing. As stated in cover letter, you need also
> > > > https://patchwork.kernel.org/patch/10043259/
> > > > which can be also found in Roger's branch
> > > > https://github.com/rogerq/linux/commits/for-v4.16/gpmc-omap-immutable
> > > > (see previous replies in this thread and also discussion about OneNAND
> > > > DT node)
> > > 
> > > I applied it, and now get:
> > > 
> > > [    0.174987] omap-gpmc 6800a000.gpmc: GPMC revision 2.0
> > > [    0.175048] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > > [    0.177703] omap-gpmc 6800a000.gpmc: Incompatible OneNAND node: missing compatible
> > > [    0.177764] omap-gpmc 6800a000.gpmc: failed to probe DT child 'onenand': -22
> > 
> > As expected without reading discussion about OneNAND DT node :)
> > 
> > In short, this version expects "ti,omap2-onenand" compatible.
> 
> Right. I didn't realize by bootloader was passing the DTB from an old
> kernel.
> 
> So after fixing my boot I got the patches working N800 and N810: I was
> able to read and write on old JFFS2 file systems without any issues. So
> you can add for the series:
> 
> Tested-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>

Thank you!

> > In earlier version of this patchset there was fixup present, so driver
> > worked even without compatible. Later on (17 Oct 2017 11:46:51 +0300)
> > Roger objected (https://www.spinics.net/lists/linux-omap/msg139576.html):
> > "I don't want to fixup broken DTs like this as they will go silently without
> > being fixed. Let's instead just error out after the dev_warn(). This will
> > force us to fix all DT nodes."
> > So fixup was removed. I know N900 support is under heavy development
> > in mainline, but if n8x0 is considered finished and stable, we should
> > reintroduce fixup. Thoughts?
> 
> N8x0 support is also "under development", and we are fine with updating
> and always using latest in-tree DTs.

That makes things simpler :) I'll post v6 based on -next with patches 13 and
14 squashed.

Thanks to everyone involved for cooperation,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html