diff mbox

[v3,1/7] memory: omap-gpmc: Refactor OneNAND support

Message ID 20171109181004.g55vx4iveo5sulpt@lenoch (mailing list archive)
State New, archived
Headers show

Commit Message

Ladislav Michl Nov. 9, 2017, 6:10 p.m. UTC
On Thu, Nov 09, 2017 at 09:56:26AM -0800, Tony Lindgren wrote:
> Hi,
> 
> * Ladislav Michl <ladis@linux-mips.org> [171109 09:14]:
> > Use generic probe function to deal with OneNAND node and remove now useless
> > gpmc_probe_onenand_child function.
> > Import sync mode timing calculation function from mach-omap2/gpmc-onenand.c
> > and prepare for MTD driver DTfication.
> 
> I tried giving this series a try on n900, but looks like onenand is no longer
> seen on n900 after this first patch.

This first patch makes original driver stop working as it removes special
OneNAND handling. If it doesn't work even after applying whole serie, then:
- verify onenand node has compatible 'ti,omap2-onenand' property
- verify timings

On IGEPv2 bootlog shows:
[    1.544464] omap-gpmc 6e000000.gpmc: GPMC revision 5.0
[    1.550415] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
[    1.560485] omap2-onenand 30000000.onenand: initializing on CS0, phys base 0x30000000, virtual base e0080000
[    1.571014] Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58)
[    1.576507] OneNAND version = 0x0031
[    1.583435] Scanning device for bad blocks
[    1.620971] OneNAND eraseblock 597 is an initial bad block
[    1.657470] OneNAND eraseblock 1159 is an initial bad block
[    1.754577] OneNAND eraseblock 2812 is an initial bad block
[    1.832214] omap2-onenand 30000000.onenand: optimized timings for 83 MHz
[    1.842620] 2 ofpart partitions found on MTD device 30000000.onenand
[    1.849426] Creating 2 MTD partitions on "30000000.onenand":
[    1.855651] 0x000000000000-0x000000080000 : "SPL"
[    1.863464] 0x000000080000-0x000020000000 : "UBI"

For a start:

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

Comments

Tony Lindgren Nov. 9, 2017, 6:26 p.m. UTC | #1
* Ladislav Michl <ladis@linux-mips.org> [171109 18:11]:
> On Thu, Nov 09, 2017 at 09:56:26AM -0800, Tony Lindgren wrote:
> > Hi,
> > 
> > * Ladislav Michl <ladis@linux-mips.org> [171109 09:14]:
> > > Use generic probe function to deal with OneNAND node and remove now useless
> > > gpmc_probe_onenand_child function.
> > > Import sync mode timing calculation function from mach-omap2/gpmc-onenand.c
> > > and prepare for MTD driver DTfication.
> > 
> > I tried giving this series a try on n900, but looks like onenand is no longer
> > seen on n900 after this first patch.
> 
> This first patch makes original driver stop working as it removes special
> OneNAND handling. If it doesn't work even after applying whole serie, then:
> - verify onenand node has compatible 'ti,omap2-onenand' property
> - verify timings
> 
> On IGEPv2 bootlog shows:
> [    1.544464] omap-gpmc 6e000000.gpmc: GPMC revision 5.0
> [    1.550415] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> [    1.560485] omap2-onenand 30000000.onenand: initializing on CS0, phys base 0x30000000, virtual base e0080000
> [    1.571014] Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58)
> [    1.576507] OneNAND version = 0x0031
> [    1.583435] Scanning device for bad blocks
> [    1.620971] OneNAND eraseblock 597 is an initial bad block
> [    1.657470] OneNAND eraseblock 1159 is an initial bad block
> [    1.754577] OneNAND eraseblock 2812 is an initial bad block
> [    1.832214] omap2-onenand 30000000.onenand: optimized timings for 83 MHz
> [    1.842620] 2 ofpart partitions found on MTD device 30000000.onenand
> [    1.849426] Creating 2 MTD partitions on "30000000.onenand":
> [    1.855651] 0x000000000000-0x000000080000 : "SPL"
> [    1.863464] 0x000000080000-0x000020000000 : "UBI"
> 
> For a start:
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index 4acd32a1c4ef..aa5b1a439564 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -838,6 +838,7 @@
>  	onenand@0,0 {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> +		compatible = "ti,omap2-onenand";
>  		reg = <0 0 0x20000>;	/* CS0, offset 0, IO size 128K */
>  
>  		gpmc,sync-read;

Well we should have the dependencies merged first to avoid breaking
git bisect. After applying this and the first patch I see:

omap-gpmc 6e000000.gpmc: /ocp@68000000/gpmc@6e000000/onenand@0,0 has no 'bank-width' property
omap-gpmc 6e000000.gpmc: failed to probe DT child 'onenand': -22

So seems like more dts changes are needed to test this.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ladislav Michl Nov. 9, 2017, 6:34 p.m. UTC | #2
On Thu, Nov 09, 2017 at 10:26:45AM -0800, Tony Lindgren wrote:
> * Ladislav Michl <ladis@linux-mips.org> [171109 18:11]:
> > On Thu, Nov 09, 2017 at 09:56:26AM -0800, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * Ladislav Michl <ladis@linux-mips.org> [171109 09:14]:
> > > > Use generic probe function to deal with OneNAND node and remove now useless
> > > > gpmc_probe_onenand_child function.
> > > > Import sync mode timing calculation function from mach-omap2/gpmc-onenand.c
> > > > and prepare for MTD driver DTfication.
> > > 
> > > I tried giving this series a try on n900, but looks like onenand is no longer
> > > seen on n900 after this first patch.
> > 
> > This first patch makes original driver stop working as it removes special
> > OneNAND handling. If it doesn't work even after applying whole serie, then:
> > - verify onenand node has compatible 'ti,omap2-onenand' property
> > - verify timings
> > 
> > On IGEPv2 bootlog shows:
> > [    1.544464] omap-gpmc 6e000000.gpmc: GPMC revision 5.0
> > [    1.550415] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> > [    1.560485] omap2-onenand 30000000.onenand: initializing on CS0, phys base 0x30000000, virtual base e0080000
> > [    1.571014] Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58)
> > [    1.576507] OneNAND version = 0x0031
> > [    1.583435] Scanning device for bad blocks
> > [    1.620971] OneNAND eraseblock 597 is an initial bad block
> > [    1.657470] OneNAND eraseblock 1159 is an initial bad block
> > [    1.754577] OneNAND eraseblock 2812 is an initial bad block
> > [    1.832214] omap2-onenand 30000000.onenand: optimized timings for 83 MHz
> > [    1.842620] 2 ofpart partitions found on MTD device 30000000.onenand
> > [    1.849426] Creating 2 MTD partitions on "30000000.onenand":
> > [    1.855651] 0x000000000000-0x000000080000 : "SPL"
> > [    1.863464] 0x000000080000-0x000020000000 : "UBI"
> > 
> > For a start:
> > 
> > diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> > index 4acd32a1c4ef..aa5b1a439564 100644
> > --- a/arch/arm/boot/dts/omap3-n900.dts
> > +++ b/arch/arm/boot/dts/omap3-n900.dts
> > @@ -838,6 +838,7 @@
> >  	onenand@0,0 {
> >  		#address-cells = <1>;
> >  		#size-cells = <1>;
> > +		compatible = "ti,omap2-onenand";
> >  		reg = <0 0 0x20000>;	/* CS0, offset 0, IO size 128K */
> >  
> >  		gpmc,sync-read;
> 
> Well we should have the dependencies merged first to avoid breaking

Yes, that's what cover letter says :-)
Also, I still count on your suggestion to merge it via mtd tree.

> git bisect. After applying this and the first patch I see:
> 
> omap-gpmc 6e000000.gpmc: /ocp@68000000/gpmc@6e000000/onenand@0,0 has no 'bank-width' property
> omap-gpmc 6e000000.gpmc: failed to probe DT child 'onenand': -22
> 
> So seems like more dts changes are needed to test this.

Argh... You are right, I should add this into serie:
https://patchwork.kernel.org/patch/10043259/

> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Nov. 9, 2017, 6:48 p.m. UTC | #3
* Ladislav Michl <ladis@linux-mips.org> [171109 18:35]:
> On Thu, Nov 09, 2017 at 10:26:45AM -0800, Tony Lindgren wrote:
> > Well we should have the dependencies merged first to avoid breaking
> 
> Yes, that's what cover letter says :-)
> Also, I still count on your suggestion to merge it via mtd tree.

Well only after it has been tested :) At this point I'd say
we're best off waiting for v4.16 merge window on the clean
up too as the merge window is about to open.

> > git bisect. After applying this and the first patch I see:
> > 
> > omap-gpmc 6e000000.gpmc: /ocp@68000000/gpmc@6e000000/onenand@0,0 has no 'bank-width' property
> > omap-gpmc 6e000000.gpmc: failed to probe DT child 'onenand': -22
> > 
> > So seems like more dts changes are needed to test this.
> 
> Argh... You are right, I should add this into serie:
> https://patchwork.kernel.org/patch/10043259/

Well don't we still need the related dts changes posted
and merged first?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ladislav Michl Nov. 9, 2017, 7:10 p.m. UTC | #4
On Thu, Nov 09, 2017 at 10:48:28AM -0800, Tony Lindgren wrote:
> * Ladislav Michl <ladis@linux-mips.org> [171109 18:35]:
> > On Thu, Nov 09, 2017 at 10:26:45AM -0800, Tony Lindgren wrote:
> > > Well we should have the dependencies merged first to avoid breaking
> > 
> > Yes, that's what cover letter says :-)
> > Also, I still count on your suggestion to merge it via mtd tree.
> 
> Well only after it has been tested :) At this point I'd say
> we're best off waiting for v4.16 merge window on the clean
> up too as the merge window is about to open.
> 
> > > git bisect. After applying this and the first patch I see:
> > > 
> > > omap-gpmc 6e000000.gpmc: /ocp@68000000/gpmc@6e000000/onenand@0,0 has no 'bank-width' property
> > > omap-gpmc 6e000000.gpmc: failed to probe DT child 'onenand': -22
> > > 
> > > So seems like more dts changes are needed to test this.
> > 
> > Argh... You are right, I should add this into serie:
> > https://patchwork.kernel.org/patch/10043259/
> 
> Well don't we still need the related dts changes posted
> and merged first?

Well, except Roger's feedback, there were complete silence so far.
Note that simple fix was posted in February, this was rejected as
we should aim towards clean DT only driver. And here we are with
some doubts.

As I have no hardware nor any special knowledge going prior
initial driver commit it is rather hard to send "related dts changes"
given simple fact, that I'm unsure whenever we need to distinguish
between OMAP2 and OMAP3. Based on that we need either one or
two compatible strings.

Having two is safe, but u-boot will be unable to bring onenand equipped
igep up.

	ladis

> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Nov. 9, 2017, 9:59 p.m. UTC | #5
* Ladislav Michl <ladis@linux-mips.org> [171109 19:12]:
> On Thu, Nov 09, 2017 at 10:48:28AM -0800, Tony Lindgren wrote:
> > Well don't we still need the related dts changes posted
> > and merged first?
> 
> Well, except Roger's feedback, there were complete silence so far.
> Note that simple fix was posted in February, this was rejected as
> we should aim towards clean DT only driver. And here we are with
> some doubts.
> 
> As I have no hardware nor any special knowledge going prior
> initial driver commit it is rather hard to send "related dts changes"
> given simple fact, that I'm unsure whenever we need to distinguish
> between OMAP2 and OMAP3. Based on that we need either one or
> two compatible strings.
> 
> Having two is safe, but u-boot will be unable to bring onenand equipped
> igep up.

Well if you have a complete series with proposed dts changes for
n8x0 and n900, I can try to test those. My n800 seems to have stopped
booting today.. Maybe I need to reflash it after playing with onenand.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros Nov. 10, 2017, 8:12 a.m. UTC | #6
On 09/11/17 20:48, Tony Lindgren wrote:
> * Ladislav Michl <ladis@linux-mips.org> [171109 18:35]:
>> On Thu, Nov 09, 2017 at 10:26:45AM -0800, Tony Lindgren wrote:
>>> Well we should have the dependencies merged first to avoid breaking
>>
>> Yes, that's what cover letter says :-)
>> Also, I still count on your suggestion to merge it via mtd tree.
> 
> Well only after it has been tested :) At this point I'd say
> we're best off waiting for v4.16 merge window on the clean
> up too as the merge window is about to open.
> 
>>> git bisect. After applying this and the first patch I see:
>>>
>>> omap-gpmc 6e000000.gpmc: /ocp@68000000/gpmc@6e000000/onenand@0,0 has no 'bank-width' property
>>> omap-gpmc 6e000000.gpmc: failed to probe DT child 'onenand': -22
>>>
>>> So seems like more dts changes are needed to test this.
>>
>> Argh... You are right, I should add this into serie:
>> https://patchwork.kernel.org/patch/10043259/

You don't need to resend that patch as it has been already
reviewed and in my v4.16 queue. Just indicate in the cover letter that this
series depends on that patch.

> 
> Well don't we still need the related dts changes posted
> and merged first> 
> Regards,
> 
> Tony
>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 4acd32a1c4ef..aa5b1a439564 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -838,6 +838,7 @@ 
 	onenand@0,0 {
 		#address-cells = <1>;
 		#size-cells = <1>;
+		compatible = "ti,omap2-onenand";
 		reg = <0 0 0x20000>;	/* CS0, offset 0, IO size 128K */
 
 		gpmc,sync-read;