All of lore.kernel.org
 help / color / mirror / Atom feed
From: gerlando.falauto@keymile.com (Gerlando Falauto)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
Date: Wed, 17 Jul 2013 08:35:38 +0200	[thread overview]
Message-ID: <51E63B3A.2010807@keymile.com> (raw)
In-Reply-To: <20130716125531.GD2317@localhost>

Hi Ezequiel,

On 07/16/2013 02:56 PM, Ezequiel Garcia wrote:
[...]
> Also, speaking of "device bus" this nand node should be behind a devicebus node.
>
> 		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000   /* internal-regs */
> 			  MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>;
>
> 		devbus {
> 			status = "okay";
> 			ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>;
>
> 			/* nand */
> 			nand {
> 				compatible = "marvell,orion-nand";
> 				reg = <0 0x400>;
> 			};
> 		};
>
> (notice this will allow you to relocate the base address of the NAND windows
> easily if it conflicts with your PCIe needs).

I am MAYBE slowly starting to understand this whole mbus rework.
Just one remark though: don't you think it would make sense to add 
something like:

#define MBUS_ID_INTERNAL_REGS	MBUS_ID(0xf0, 0x01)
#define MBUS_ID_NAND		MBUS_ID(0x01, 0x2f)

I personally have a hard time reading numeric values for 
GPIO_ACTIVE_LOW/GPIO_ACTIVE_HIGH.

Thanks,
Gerlando

>
>> avoid a later incosistency between the "unit-address" and the first
>> "reg" address:
>>
>>>    		#address-cells = <1>;
>>>    		#size-cells = <1>;
>>> @@ -171,7 +172,7 @@
>>   >   		nand at 3000000 {
>> 		     ^^^^^^^
>
> Oh, this should be fixed. I just missed it, and nobody noticed either.
>

WARNING: multiple messages have this Message-ID (diff)
From: Gerlando Falauto <gerlando.falauto@keymile.com>
To: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Cc: Lior Amsalem <alior@marvell.com>, Andrew Lunn <andrew@lunn.ch>,
	Jason Cooper <jason@lakedaemon.net>,
	"Longchamp, Valentin" <Valentin.Longchamp@keymile.com>,
	"devicetree-discuss@lists.ozlabs.org"
	<devicetree-discuss@lists.ozlabs.org>,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH] ARM: Kirkwood: Fix the internal register ranges translation
Date: Wed, 17 Jul 2013 08:35:38 +0200	[thread overview]
Message-ID: <51E63B3A.2010807@keymile.com> (raw)
In-Reply-To: <20130716125531.GD2317@localhost>

Hi Ezequiel,

On 07/16/2013 02:56 PM, Ezequiel Garcia wrote:
[...]
> Also, speaking of "device bus" this nand node should be behind a devicebus node.
>
> 		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000   /* internal-regs */
> 			  MBUS_ID(0x01, 0x2f) 0 0 0xf4000000 0x400>;
>
> 		devbus {
> 			status = "okay";
> 			ranges = <0 MBUS_ID(0x01, 0x2f) 0 0x400>;
>
> 			/* nand */
> 			nand {
> 				compatible = "marvell,orion-nand";
> 				reg = <0 0x400>;
> 			};
> 		};
>
> (notice this will allow you to relocate the base address of the NAND windows
> easily if it conflicts with your PCIe needs).

I am MAYBE slowly starting to understand this whole mbus rework.
Just one remark though: don't you think it would make sense to add 
something like:

#define MBUS_ID_INTERNAL_REGS	MBUS_ID(0xf0, 0x01)
#define MBUS_ID_NAND		MBUS_ID(0x01, 0x2f)

I personally have a hard time reading numeric values for 
GPIO_ACTIVE_LOW/GPIO_ACTIVE_HIGH.

Thanks,
Gerlando

>
>> avoid a later incosistency between the "unit-address" and the first
>> "reg" address:
>>
>>>    		#address-cells = <1>;
>>>    		#size-cells = <1>;
>>> @@ -171,7 +172,7 @@
>>   >   		nand@3000000 {
>> 		     ^^^^^^^
>
> Oh, this should be fixed. I just missed it, and nobody noticed either.
>

  parent reply	other threads:[~2013-07-17  6:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-18 15:31 [PATCH] ARM: Kirkwood: Fix the internal register ranges translation Ezequiel Garcia
2013-06-18 15:31 ` Ezequiel Garcia
2013-06-18 19:42 ` Sebastian Hesselbarth
2013-06-18 19:42   ` Sebastian Hesselbarth
2013-06-18 19:47   ` Ezequiel Garcia
2013-06-18 19:47     ` Ezequiel Garcia
2013-06-19 18:36     ` Jason Cooper
2013-06-19 18:36       ` Jason Cooper
2013-06-19 18:42       ` Ezequiel Garcia
2013-06-19 18:42         ` Ezequiel Garcia
2013-06-19 18:43         ` Jason Cooper
2013-06-19 18:43           ` Jason Cooper
2013-06-21 15:41 ` Jason Cooper
2013-06-21 15:41   ` Jason Cooper
2013-07-16  9:37 ` Gerlando Falauto
2013-07-16  9:37   ` Gerlando Falauto
2013-07-16 12:56   ` Ezequiel Garcia
2013-07-16 12:56     ` Ezequiel Garcia
2013-07-16 18:51     ` Gerlando Falauto
2013-07-16 18:51       ` Gerlando Falauto
2013-07-18 14:05       ` Ezequiel Garcia
2013-07-18 14:05         ` Ezequiel Garcia
2013-07-17  6:35     ` Gerlando Falauto [this message]
2013-07-17  6:35       ` Gerlando Falauto
2013-07-18 13:55       ` Ezequiel Garcia
2013-07-18 13:55         ` Ezequiel Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51E63B3A.2010807@keymile.com \
    --to=gerlando.falauto@keymile.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.