kirkwood, nand, clocks and device-tree

kirkwood, nand, clocks and device-tree

[Arnaud Patard (Rtp)]

Hi,

I wanted to convert iconnect to use the new nand DT support on kirkwood
but my system was freezing hard during boot. So, I've debugged it a
little bit and have several questions.

The first one is about were is defined the nand. It's defined as a leaf
of the "ocp" bus. I would have expected to see it at the same level but
I've seen nothing proving if it's wrong or right when comparing with
hardware. Can anyone give me some information about that ?

Now, the real questions about my problem. It's hanging because the nand
platform device created throught DT is called f4000000.nand and not
orion_nand. This means that the clock gets disabled by
clk_disable_unused() so first access to the nand fails badly.
To workaround, I've added into mach-kirkwood/common.c:

+       orion_clkdev_add(NULL, "f4000000.nand", runit);


So the clock stays enabled and my system is booting. Would a patch
doing that would be accepted or would it be better to add a "orion_nand"
id so that one can get the clock either with the device or with the id,
avoiding to add extra clocks for each SoC ?

The bonus question is that while debugging this issue, I've disabled
all nand declarations in kirkwood.dtsi and my localy modified
kirkwood-iconnect.dts but I was still getting a hard freeze. I think
that means that the "runit" clock is doing more than NAND and SPI but
I've found no details in the 6281 manual about what's using this
clock. Again, does someone have more details about this (which would
explain why disabling this runit clock will freeze my system) ?

Thanks,
Arnaud

kirkwood, nand, clocks and device-tree

[Andrew Lunn]

On Fri, May 18, 2012 at 11:50:42AM +0200, Arnaud Patard wrote:
> Hi,
> 
> I wanted to convert iconnect to use the new nand DT support on kirkwood
> but my system was freezing hard during boot. So, I've debugged it a
> little bit and have several questions.
> 
> The first one is about were is defined the nand. It's defined as a leaf
> of the "ocp" bus. I would have expected to see it at the same level but
> I've seen nothing proving if it's wrong or right when comparing with
> hardware. Can anyone give me some information about that ?
> 
> Now, the real questions about my problem. It's hanging because the nand
> platform device created throught DT is called f4000000.nand and not
> orion_nand. This means that the clock gets disabled by
> clk_disable_unused() so first access to the nand fails badly.
> To workaround, I've added into mach-kirkwood/common.c:
> 
> +       orion_clkdev_add(NULL, "f4000000.nand", runit);

Although this works, its the wrong way to do it. Jamie Lentin figured
out the right way to do this. Jamie, care to explain please...

> The bonus question is that while debugging this issue, I've disabled
> all nand declarations in kirkwood.dtsi and my localy modified
> kirkwood-iconnect.dts but I was still getting a hard freeze. I think
> that means that the "runit" clock is doing more than NAND and SPI but
> I've found no details in the 6281 manual about what's using this
> clock.

Yes, i found the same. I tried extending the SPI driver so that it
turned the clock off between transfers. Hard crash like you. I suspect
i might have to mark this clock as not to be turned off when unused.

  Andrew

kirkwood, nand, clocks and device-tree

[Jamie Lentin]

On Sun, 20 May 2012, Andrew Lunn wrote:

> On Fri, May 18, 2012 at 11:50:42AM +0200, Arnaud Patard wrote:
>> Hi,
>>
>> I wanted to convert iconnect to use the new nand DT support on kirkwood
>> but my system was freezing hard during boot. So, I've debugged it a
>> little bit and have several questions.
>>
>> The first one is about were is defined the nand. It's defined as a leaf
>> of the "ocp" bus. I would have expected to see it at the same level but
>> I've seen nothing proving if it's wrong or right when comparing with
>> hardware. Can anyone give me some information about that ?
>>
>> Now, the real questions about my problem. It's hanging because the nand
>> platform device created throught DT is called f4000000.nand and not
>> orion_nand. This means that the clock gets disabled by
>> clk_disable_unused() so first access to the nand fails badly.
>> To workaround, I've added into mach-kirkwood/common.c:
>>
>> +       orion_clkdev_add(NULL, "f4000000.nand", runit);
>
> Although this works, its the wrong way to do it. Jamie Lentin figured
> out the right way to do this. Jamie, care to explain please...

board-dt.c should have an auxdata struct, to fix the name of 
"f4000000.nand" to orion_nand:-

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 3cef3c7..6d45a07 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -24,6 +24,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
  	{ }
  };

+struct of_dev_auxdata kirkwood_dt_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("mrvl,orion-nand", KIRKWOOD_NAND_MEM_PHYS_BASE, "orion_nand", NULL),
+	{ }
+};
+
  static void __init kirkwood_dt_init(void)
  {
  	pr_info("Kirkwood: %s, TCLK=%d.\n", kirkwood_id(), kirkwood_tclk);
@@ -61,7 +66,8 @@ static void __init kirkwood_dt_init(void)
  	if (of_machine_is_compatible("dlink,dns-kirkwood"))
  		dnskw_init();

-	of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
+	of_platform_populate(NULL, kirkwood_dt_match_table,
+			kirkwood_dt_auxdata_lookup, NULL);
  }

  static const char *kirkwood_dt_board_compat[] = {

I should really submit this :) What should it be based on? A new revision 
of my previous patches, or should I assume that my work and Andrew's clk 
work have already been merged? (apologies if this is a dumb question)

>> The bonus question is that while debugging this issue, I've disabled
>> all nand declarations in kirkwood.dtsi and my localy modified
>> kirkwood-iconnect.dts but I was still getting a hard freeze. I think
>> that means that the "runit" clock is doing more than NAND and SPI but
>> I've found no details in the 6281 manual about what's using this
>> clock.
>
> Yes, i found the same. I tried extending the SPI driver so that it
> turned the clock off between transfers. Hard crash like you. I suspect
> i might have to mark this clock as not to be turned off when unused.

+1 --- I found this also when the last patchset temporarily removed all 
NAND support (and thus RUNIT was being disabled during startup).

>
>  Andrew
>

-- 
Jamie Lentin

kirkwood, nand, clocks and device-tree

[Andrew Lunn]

> I should really submit this :) What should it be based on? A new
> revision of my previous patches, or should I assume that my work and
> Andrew's clk work have already been merged? (apologies if this is a
> dumb question)

Yes, you can assume both are in. arm-soc has them, so the 3.5-rc1
should have them.

For the next cycle we need to improve working together on DT porting.
There is a lot of work going on here, in kirkwood, orion5x, dove, and
the two new Armada SoCs, all needing/porting drivers to DT.  I think
we need a tree where patches are quickly added once they are stable,
so that others can profit from the work.

       Andrew

kirkwood, nand, clocks and device-tree

[Jason Cooper]

On Sun, May 20, 2012 at 03:22:14PM +0200, Andrew Lunn wrote:
> > I should really submit this :) What should it be based on? A new
> > revision of my previous patches, or should I assume that my work and
> > Andrew's clk work have already been merged? (apologies if this is a
> > dumb question)
> 
> Yes, you can assume both are in. arm-soc has them, so the 3.5-rc1
> should have them.
> 
> For the next cycle we need to improve working together on DT porting.
> There is a lot of work going on here, in kirkwood, orion5x, dove, and
> the two new Armada SoCs, all needing/porting drivers to DT.  I think
> we need a tree where patches are quickly added once they are stable,
> so that others can profit from the work.

I'm setting up a branch scheme that should handle this:

# main branch for pull-requests to Arnd/Olof

for-arm-soc

# branches that are ready to merge into for-arm-soc

board/<boardname>

	# branches which can be merged as depends

	driver/<driver>
	dt/<binding>

# branches which can be changed/rebased/etc

staging/*


Does that cover everthing?  Or is there something I missed?

thx,

Jason.

kirkwood, nand, clocks and device-tree

[Jason Cooper]

On Sun, May 20, 2012 at 02:05:54PM +0100, Jamie Lentin wrote:
> board-dt.c should have an auxdata struct, to fix the name of
> "f4000000.nand" to orion_nand:-
> 
> diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
> index 3cef3c7..6d45a07 100644
> --- a/arch/arm/mach-kirkwood/board-dt.c
> +++ b/arch/arm/mach-kirkwood/board-dt.c
> @@ -24,6 +24,11 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
>  	{ }
>  };
> 
> +struct of_dev_auxdata kirkwood_dt_auxdata_lookup[] __initdata = {
> +	OF_DEV_AUXDATA("mrvl,orion-nand", KIRKWOOD_NAND_MEM_PHYS_BASE, "orion_nand", NULL),
> +	{ }
> +};
> +
>  static void __init kirkwood_dt_init(void)
>  {
>  	pr_info("Kirkwood: %s, TCLK=%d.\n", kirkwood_id(), kirkwood_tclk);
> @@ -61,7 +66,8 @@ static void __init kirkwood_dt_init(void)
>  	if (of_machine_is_compatible("dlink,dns-kirkwood"))
>  		dnskw_init();
> 
> -	of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
> +	of_platform_populate(NULL, kirkwood_dt_match_table,
> +			kirkwood_dt_auxdata_lookup, NULL);
>  }
> 
>  static const char *kirkwood_dt_board_compat[] = {
> 
> I should really submit this :) What should it be based on? A new

Yes, please do.  It's a good thing I haven't run across a nand-based
dreamplug yet. ;-)

As Andrew mentioned, until v3.5-rc1 comes out, please base on and
up-to-date arm-soc.

thx,

Jason.

kirkwood, nand, clocks and device-tree

[Andrew Lunn]

> > For the next cycle we need to improve working together on DT porting.
> > There is a lot of work going on here, in kirkwood, orion5x, dove, and
> > the two new Armada SoCs, all needing/porting drivers to DT.  I think
> > we need a tree where patches are quickly added once they are stable,
> > so that others can profit from the work.
> 
> I'm setting up a branch scheme that should handle this:
> 
> # main branch for pull-requests to Arnd/Olof
> 
> for-arm-soc

I would say that this branch needs to be stable, i.e. no rebasing.  We
need to encourage everybody to use it as the basis for their work,
since it will have everything applied that can be shared. No rebasing
means that if something is broken, a patch is applied on top. Then
just before the merge window you can squash the patch into the
original work so we have a clean patchset going upwards.

> # branches that are ready to merge into for-arm-soc
> 
> board/<boardname>
> 
> 	# branches which can be merged as depends

Do you mean old style, none DT, boards?

> 	driver/<driver>
> 	dt/<binding>

You need to explain that a bit more. Since dt/bindings means changes
to drivers, don't you have the danger of merge conflicts?

Overall, maybe you are over engineering? We need comments from Arnd &
Olof, since they run a system like this, and know if the complexity is
needed for just one platform.

Also, my guess is, the next cycle is going to be extraordinary. We
have major cleanup going on after merging into one/two directories. We
have probably most of the DT support going in, two new SoC types
getting supported, and hopefully Dove and Orion5x catching up with
their DT support. So, many people will be working on core
infrastructure, not individual board files. It is only after that is
done, will the work return to everybody in there own corner working on
just single boards.

       Andrew

kirkwood, nand, clocks and device-tree

[Jason Cooper]

On Mon, May 21, 2012 at 07:23:22AM +0200, Andrew Lunn wrote:
> > > For the next cycle we need to improve working together on DT porting.
> > > There is a lot of work going on here, in kirkwood, orion5x, dove, and
> > > the two new Armada SoCs, all needing/porting drivers to DT.  I think
> > > we need a tree where patches are quickly added once they are stable,
> > > so that others can profit from the work.
> > 
> > I'm setting up a branch scheme that should handle this:
> > 
> > # main branch for pull-requests to Arnd/Olof
> > 
> > for-arm-soc
> 
> I would say that this branch needs to be stable, i.e. no rebasing.  We
> need to encourage everybody to use it as the basis for their work,
> since it will have everything applied that can be shared. No rebasing
> means that if something is broken, a patch is applied on top. Then
> just before the merge window you can squash the patch into the
> original work so we have a clean patchset going upwards.

Yes.

> > # branches that are ready to merge into for-arm-soc
> > 
> > board/<boardname>
> > 
> 
> Do you mean old style, none DT, boards?

No, new DT boards.

I moved my comment to be close to what it was referring to:
> > 	# branches which can be merged as depends
> > 	driver/<driver>
> > 	dt/<binding>
> 
> You need to explain that a bit more. Since dt/bindings means changes
> to drivers, don't you have the danger of merge conflicts?

Right, perhaps just a branch per patch series.  Since each submitted
patch series tends to be self-grouped.  My main goal was to come up with
a naming scheme to to indicate branches folks can base off of, merge as
depends, or just look at (staging/unstable).  Perhaps:

	for-arm-soc
	depend/orion_nand
	depend/kw_board_raidsonic
	staging/orion_spi

That should be a little cleaner

> Overall, maybe you are over engineering?

Probably.  ;-P

> We need comments from Arnd & Olof, since they run a system like this,
> and know if the complexity is needed for just one platform.
> 
> Also, my guess is, the next cycle is going to be extraordinary. We
> have major cleanup going on after merging into one/two directories. We
> have probably most of the DT support going in, two new SoC types
> getting supported, and hopefully Dove and Orion5x catching up with
> their DT support. So, many people will be working on core
> infrastructure, not individual board files. It is only after that is
> done, will the work return to everybody in there own corner working on
> just single boards.

If we get all that done in this cycle, they should only have to write
dts files...

thx,

Jason.

kirkwood, nand, clocks and device-tree

[Andrew Lunn]

> > > board/<boardname>
> > > 
> > 
> > Do you mean old style, none DT, boards?
> 
> No, new DT boards.

O.K. Then a better name would be dt-board/<boardname>

     Andrew

kirkwood, nand, clocks and device-tree

[Jason Cooper]

On Mon, May 21, 2012 at 07:35:06PM +0200, Andrew Lunn wrote:
> > > > board/<boardname>
> > > > 
> > > 
> > > Do you mean old style, none DT, boards?
> > 
> > No, new DT boards.
> 
> O.K. Then a better name would be dt-board/<boardname>

Well, no new boards are being accepted without DT support.  However, I
have no strong opinion on the matter, so dt-board/ it is, then.

thx,

Jason.

[PATCH] ARM: kirkwood: Force nand platform device name

[Jamie Lentin]

The clk framework relies on device names to be constant. Pre-dt, the
NAND platform device was called "orion_nand" which is what the framework
expects. The following auxdata block forces the name to "orion_nand",
so the clk framework finds it and keeps required clocks enabled.

Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
 arch/arm/mach-kirkwood/board-dt.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index edc3f8a..dbf7fe3 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -25,6 +25,12 @@ static struct of_device_id kirkwood_dt_match_table[] __initdata = {
 	{ }
 };
 
+struct of_dev_auxdata kirkwood_dt_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("mrvl,orion-nand", KIRKWOOD_NAND_MEM_PHYS_BASE,
+		       "orion_nand", NULL),
+	{ }
+};
+
 static void __init kirkwood_dt_init(void)
 {
 	pr_info("Kirkwood: %s, TCLK=%d.\n", kirkwood_id(), kirkwood_tclk);
@@ -68,7 +74,8 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("raidsonic,ib-nas62x0"))
 		ib62x0_init();
 
-	of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
+	of_platform_populate(NULL, kirkwood_dt_match_table,
+			kirkwood_dt_auxdata_lookup, NULL);
 }
 
 static const char *kirkwood_dt_board_compat[] = {
-- 
1.7.10

kirkwood, nand, clocks and device-tree

[Arnd Bergmann]

On Monday 21 May 2012, Jason Cooper wrote:
> On Mon, May 21, 2012 at 07:35:06PM +0200, Andrew Lunn wrote:
> > > > > board/<boardname>
> > > > > 
> > > > 
> > > > Do you mean old style, none DT, boards?
> > > 
> > > No, new DT boards.
> > 
> > O.K. Then a better name would be dt-board/<boardname>
> 
> Well, no new boards are being accepted without DT support.  However, I
> have no strong opinion on the matter, so dt-board/ it is, then.

Just one branch should be enough for all the boards, I think. As soon
as you have the gpio bindings taken care of, the board specific parts
will shrink significantly anyway.

	Arnd

kirkwood, nand, clocks and device-tree

[Jason Cooper]

On Tue, May 22, 2012 at 07:33:37AM +0000, Arnd Bergmann wrote:
> On Monday 21 May 2012, Jason Cooper wrote:
> > On Mon, May 21, 2012 at 07:35:06PM +0200, Andrew Lunn wrote:
> > > > > > board/<boardname>
> > > > > > 
> > > > > 
> > > > > Do you mean old style, none DT, boards?
> > > > 
> > > > No, new DT boards.
> > > 
> > > O.K. Then a better name would be dt-board/<boardname>
> > 
> > Well, no new boards are being accepted without DT support.  However, I
> > have no strong opinion on the matter, so dt-board/ it is, then.
> 
> Just one branch should be enough for all the boards, I think. As soon
> as you have the gpio bindings taken care of, the board specific parts
> will shrink significantly anyway.

Ok, sounds good.

thx,

Jason.