Patchwork [GIT,PULL] at91: fixes for 3.9-rc #2

login
register
mail settings
Submitter Nicolas Ferre
Date March 26, 2013, 10:16 a.m.
Message ID <51517592.5010300@atmel.com>
Download mbox
Permalink /patch/231157/
State New
Headers show

Pull-request

git://github.com/at91linux/linux-at91.git tags/at91-fixes

Comments

Nicolas Ferre - March 26, 2013, 10:16 a.m.
Arnd, Olof,

Another "fixes" pull-request for AT91 on top of material that you have already
taken. Fixes are manly typos but the bad node declaration and some misspelling
can cause confusion.

The following changes since commit 0ed66befaae893e82c9f016238282d73ef9fd6c7:

  ARM: at91: fix infinite loop in at91_irq_suspend/resume (2013-03-14 09:37:55 +0100)

are available in the git repository at:

  git://github.com/at91linux/linux-at91.git tags/at91-fixes

for you to fetch changes up to 9476a186078f47bdc5f03560074691de98cd767b:

  ARM: at91: Fix typo in restart code panic message (2013-03-26 11:00:02 +0100)

----------------------------------------------------------------
Little fixes mainly related to at91sam9x5 DT.
One typo is the only modification in C code.

----------------------------------------------------------------
Maxime Ripard (1):
      ARM: at91: Fix typo in restart code panic message

Nicolas Ferre (4):
      ARM: at91/dt: fix macb node declaration
      ARM: at91/trivial: typos in compatible property
      ARM: at91/trivial: fix model name for SAM9G15-EK
      ARM: at91: remove partial parameter in bootargs for at91sam9x5ek.dtsi

 arch/arm/boot/dts/at91sam9g15.dtsi  |  2 +-
 arch/arm/boot/dts/at91sam9g15ek.dts |  2 +-
 arch/arm/boot/dts/at91sam9g25.dtsi  |  2 +-
 arch/arm/boot/dts/at91sam9g25ek.dts |  9 +++++++++
 arch/arm/boot/dts/at91sam9g35.dtsi  |  2 +-
 arch/arm/boot/dts/at91sam9g35ek.dts |  9 +++++++++
 arch/arm/boot/dts/at91sam9x25.dtsi  |  2 +-
 arch/arm/boot/dts/at91sam9x25ek.dts | 14 ++++++++++++++
 arch/arm/boot/dts/at91sam9x35.dtsi  |  2 +-
 arch/arm/boot/dts/at91sam9x35ek.dts |  9 +++++++++
 arch/arm/boot/dts/at91sam9x5ek.dtsi |  7 +------
 arch/arm/mach-at91/setup.c          |  2 +-
 12 files changed, 49 insertions(+), 13 deletions(-)
Arnd Bergmann - March 26, 2013, 10:22 a.m.
On Tuesday 26 March 2013, Nicolas Ferre wrote:
> Another "fixes" pull-request for AT91 on top of material that you have already
> taken. Fixes are manly typos but the bad node declaration and some misspelling
> can cause confusion.

Hi Nicolas,

I'd prefer to take only fixes for serious bugs into 3.9 now, since I have
already sent the fixes for -rc5. As far as I can tell, the macb node patch
is the only one that falls into that category, but for that one, I'd
prefer if it could be redone in a simpler way, by replacing

+       ahb {
+               apb {
+                       macb0: ethernet@f802c000 {
+                               phy-mode = "rmii";
+                               status = "okay";
+                       };
+               };
+       };


with

	@macb0 {
		phy-mode = "rmii";
		status = "okay";
	}

as there is no need to provide the full path when you already have
a label for the device.

	Arnd
Nicolas Ferre - March 26, 2013, 10:56 a.m.
On 03/26/2013 11:22 AM, Arnd Bergmann :
> On Tuesday 26 March 2013, Nicolas Ferre wrote:
>> Another "fixes" pull-request for AT91 on top of material that you have already
>> taken. Fixes are manly typos but the bad node declaration and some misspelling
>> can cause confusion.
> 
> Hi Nicolas,
> 
> I'd prefer to take only fixes for serious bugs into 3.9 now, since I have
> already sent the fixes for -rc5. As far as I can tell, the macb node patch
> is the only one that falls into that category

Yes, fine, I stack them for 3.10 then.


> , but for that one, I'd
> prefer if it could be redone in a simpler way, by replacing
> 
> +       ahb {
> +               apb {
> +                       macb0: ethernet@f802c000 {
> +                               phy-mode = "rmii";
> +                               status = "okay";
> +                       };
> +               };
> +       };
> 
> 
> with
> 
> 	@macb0 {
> 		phy-mode = "rmii";
> 		status = "okay";
> 	}
> 
> as there is no need to provide the full path when you already have
> a label for the device.

Indeed, at first sight its looks like a nice solution. But I wonder if
we add several nodes in this kind of .dtsi files, we may end up with
mess between APB peripherals and board specific ones like leds, NAND, etc.
Moreover, the plain and full naming of the node makes it identifiable
without doubts, even someone not completely familiar with DT.

I slightly prefer to keep it like this, but I can still change my mind ;-)

Bye,
Arnd Bergmann - March 26, 2013, 11:05 a.m.
On Tuesday 26 March 2013, Nicolas Ferre wrote:
> Indeed, at first sight its looks like a nice solution. But I wonder if
> we add several nodes in this kind of .dtsi files, we may end up with
> mess between APB peripherals and board specific ones like leds, NAND, etc.
> Moreover, the plain and full naming of the node makes it identifiable
> without doubts, even someone not completely familiar with DT.
> 
> I slightly prefer to keep it like this, but I can still change my mind ;-)

If you don't want to use the labels, it's probably better to drop from
the dtsi files. I think you can also write 

ahb/apb/ethernet@802c000 {
	phy-mode = "rmii";
        status = "okay";
};

although I think most people prefer the labels, and I have not tried
the above.

Back to the bug fix, I think we can have a simpler version for 3.9
if you just mark the ethernet node as "disabled" in the 
at91sam9g15.dtsi file, and then do the patch to change the default
in an add-on patch for 3.10.

	Arnd
Nicolas Ferre - March 27, 2013, 11:28 a.m.
On 03/26/2013 12:05 PM, Arnd Bergmann :
> On Tuesday 26 March 2013, Nicolas Ferre wrote:
>> Indeed, at first sight its looks like a nice solution. But I wonder if
>> we add several nodes in this kind of .dtsi files, we may end up with
>> mess between APB peripherals and board specific ones like leds, NAND, etc.
>> Moreover, the plain and full naming of the node makes it identifiable
>> without doubts, even someone not completely familiar with DT.
>>
>> I slightly prefer to keep it like this, but I can still change my mind ;-)
> 
> If you don't want to use the labels, it's probably better to drop from
> the dtsi files. I think you can also write 
> 
> ahb/apb/ethernet@802c000 {
> 	phy-mode = "rmii";
>         status = "okay";
> };
> 
> although I think most people prefer the labels, and I have not tried
> the above.
> 
> Back to the bug fix, I think we can have a simpler version for 3.9
> if you just mark the ethernet node as "disabled" in the 
> at91sam9g15.dtsi file,

It will not work, the at91sam9g15.dtsi is included before the buggy
at91sam9x5ek.dtsi in the board .dts file.

Modifying only the at91sam9g15ek.dts and disabling it there will create
an awful mess:
macb0:
at91sam9x5.dtsi: status = "disabled";
at91sam9x5ek.dtsi: status = "okay";   <<-- bug!
at91sam9g15ek.dts: status = "disabled"; <<-- fix?

> and then do the patch to change the default
> in an add-on patch for 3.10.

If it is too late for this kernel revision, I would prefer to postpone
this modification and tag it for "stable" than have to deal with this
issue twice...

Bye,