| Message ID | Pine.LNX.4.64.1305171458550.31481@axis700.grange (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 17, 2013 at 03:00:23PM +0200, Guennadi Liakhovetski wrote: > Hi Simon, Magnus > > On Wed, 3 Apr 2013, Simon Horman wrote: > > > From: Magnus Damm <damm@opensource.se> > > > > Add LAN9220 support to the APE6EVM board using C and DT. > > At this point the PFC driver lacks DT bindings so to > > configure the PFC we use PINCTRL in C board code. > > > > Signed-off-by: Magnus Damm <damm@opensource.se> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > --- > > arch/arm/boot/dts/r8a73a4-ape6evm.dts | 23 ++++++++++++++++++- > > arch/arm/mach-shmobile/board-ape6evm.c | 38 ++++++++++++++++++++++++++++++++ > > 2 files changed, 60 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > index 833f703..f603c69 100644 > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > @@ -16,7 +16,7 @@ > > compatible = "renesas,ape6evm", "renesas,r8a73a4"; > > > > chosen { > > - bootargs = "console=ttySC0,115200 ignore_loglevel"; > > + bootargs = "console=ttySC0,115200 ignore_loglevel root=/dev/nfs ip=dhcp"; > > }; > > > > memory@40000000 { > > @@ -24,8 +24,29 @@ > > reg = <0 0x40000000 0 0x40000000>; > > }; > > > > + ape6evm_fixed_3v3: fixedregulator@0 { > > + compatible = "regulator-fixed"; > > + regulator-name = "3V3"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-always-on; > > + }; > > + > > lbsc { > > #address-cells = <1>; > > #size-cells = <1>; > > + > > + ethernet@8000000 { > > + compatible = "smsc,lan9118", "smsc,lan9115"; > > + reg = <0x08000000 0x1000>; > > + interrupt-parent = <&irqc1>; > > + interrupts = <8 0x4>; > > + phy-mode = "mii"; > > + reg-io-width = <4>; > > + smsc,irq-active-high; > > + smsc,irq-push-pull; > > + vdd33a-supply = <&ape6evm_fixed_3v3>; > > + vddvario-supply = <&ape6evm_fixed_3v3>; > > + }; > > }; > > }; > > The above didn't work in my tests without this: > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > index f603c69..4fb0102 100644 > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > @@ -33,8 +33,10 @@ > }; > > lbsc { > + compatible = "simple-bus"; > #address-cells = <1>; > #size-cells = <1>; > + ranges = <0 0 0 0x80000000>; > > ethernet@8000000 { > compatible = "smsc,lan9118", "smsc,lan9115"; > Could you please post this as a formal patch and indicate if you would like it included as a fix in v3.10 or not? Also, I am curious to know what your tests are.
Hi Simon On Wed, 22 May 2013, Simon Horman wrote: > On Fri, May 17, 2013 at 03:00:23PM +0200, Guennadi Liakhovetski wrote: > > Hi Simon, Magnus > > > > On Wed, 3 Apr 2013, Simon Horman wrote: > > > > > From: Magnus Damm <damm@opensource.se> > > > > > > Add LAN9220 support to the APE6EVM board using C and DT. > > > At this point the PFC driver lacks DT bindings so to > > > configure the PFC we use PINCTRL in C board code. > > > > > > Signed-off-by: Magnus Damm <damm@opensource.se> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > > --- > > > arch/arm/boot/dts/r8a73a4-ape6evm.dts | 23 ++++++++++++++++++- > > > arch/arm/mach-shmobile/board-ape6evm.c | 38 ++++++++++++++++++++++++++++++++ > > > 2 files changed, 60 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > index 833f703..f603c69 100644 > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > @@ -16,7 +16,7 @@ > > > compatible = "renesas,ape6evm", "renesas,r8a73a4"; > > > > > > chosen { > > > - bootargs = "console=ttySC0,115200 ignore_loglevel"; > > > + bootargs = "console=ttySC0,115200 ignore_loglevel root=/dev/nfs ip=dhcp"; > > > }; > > > > > > memory@40000000 { > > > @@ -24,8 +24,29 @@ > > > reg = <0 0x40000000 0 0x40000000>; > > > }; > > > > > > + ape6evm_fixed_3v3: fixedregulator@0 { > > > + compatible = "regulator-fixed"; > > > + regulator-name = "3V3"; > > > + regulator-min-microvolt = <3300000>; > > > + regulator-max-microvolt = <3300000>; > > > + regulator-always-on; > > > + }; > > > + > > > lbsc { > > > #address-cells = <1>; > > > #size-cells = <1>; > > > + > > > + ethernet@8000000 { > > > + compatible = "smsc,lan9118", "smsc,lan9115"; > > > + reg = <0x08000000 0x1000>; > > > + interrupt-parent = <&irqc1>; > > > + interrupts = <8 0x4>; > > > + phy-mode = "mii"; > > > + reg-io-width = <4>; > > > + smsc,irq-active-high; > > > + smsc,irq-push-pull; > > > + vdd33a-supply = <&ape6evm_fixed_3v3>; > > > + vddvario-supply = <&ape6evm_fixed_3v3>; > > > + }; > > > }; > > > }; > > > > The above didn't work in my tests without this: > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > index f603c69..4fb0102 100644 > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > @@ -33,8 +33,10 @@ > > }; > > > > lbsc { > > + compatible = "simple-bus"; > > #address-cells = <1>; > > #size-cells = <1>; > > + ranges = <0 0 0 0x80000000>; > > > > ethernet@8000000 { > > compatible = "smsc,lan9118", "smsc,lan9115"; > > > > Could you please post this as a formal patch and indicate > if you would like it included as a fix in v3.10 or not? Can do that, sure, just thought maybe it would be better to fix the original patch. Besides, I wasn't sure what the correct values for "ranges" are, I just picked up something, that would be sufficient for ethernet. But if more devices are added to it in the future, maybe different ranges values would be needed. I think actually, lbsc should map the 3 BSC areas, so, the correct property would be + ranges = <0 0 0 0x14000000>; > Also, I am curious to know what your tests are. Just booting with NFS root. I think, anything involving ethernet. Without the "simple-bus" property the lbsc node doesn't get scanned, so, no device is added. Without "ranges" addresses cannot be mapped correctly. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Wed, May 22, 2013 at 07:22:59PM +0200, Guennadi Liakhovetski wrote: > Hi Simon > > On Wed, 22 May 2013, Simon Horman wrote: > > > On Fri, May 17, 2013 at 03:00:23PM +0200, Guennadi Liakhovetski wrote: > > > Hi Simon, Magnus > > > > > > On Wed, 3 Apr 2013, Simon Horman wrote: > > > > > > > From: Magnus Damm <damm@opensource.se> > > > > > > > > Add LAN9220 support to the APE6EVM board using C and DT. > > > > At this point the PFC driver lacks DT bindings so to > > > > configure the PFC we use PINCTRL in C board code. > > > > > > > > Signed-off-by: Magnus Damm <damm@opensource.se> > > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > > > --- > > > > arch/arm/boot/dts/r8a73a4-ape6evm.dts | 23 ++++++++++++++++++- > > > > arch/arm/mach-shmobile/board-ape6evm.c | 38 ++++++++++++++++++++++++++++++++ > > > > 2 files changed, 60 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > index 833f703..f603c69 100644 > > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > @@ -16,7 +16,7 @@ > > > > compatible = "renesas,ape6evm", "renesas,r8a73a4"; > > > > > > > > chosen { > > > > - bootargs = "console=ttySC0,115200 ignore_loglevel"; > > > > + bootargs = "console=ttySC0,115200 ignore_loglevel root=/dev/nfs ip=dhcp"; > > > > }; > > > > > > > > memory@40000000 { > > > > @@ -24,8 +24,29 @@ > > > > reg = <0 0x40000000 0 0x40000000>; > > > > }; > > > > > > > > + ape6evm_fixed_3v3: fixedregulator@0 { > > > > + compatible = "regulator-fixed"; > > > > + regulator-name = "3V3"; > > > > + regulator-min-microvolt = <3300000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + regulator-always-on; > > > > + }; > > > > + > > > > lbsc { > > > > #address-cells = <1>; > > > > #size-cells = <1>; > > > > + > > > > + ethernet@8000000 { > > > > + compatible = "smsc,lan9118", "smsc,lan9115"; > > > > + reg = <0x08000000 0x1000>; > > > > + interrupt-parent = <&irqc1>; > > > > + interrupts = <8 0x4>; > > > > + phy-mode = "mii"; > > > > + reg-io-width = <4>; > > > > + smsc,irq-active-high; > > > > + smsc,irq-push-pull; > > > > + vdd33a-supply = <&ape6evm_fixed_3v3>; > > > > + vddvario-supply = <&ape6evm_fixed_3v3>; > > > > + }; > > > > }; > > > > }; > > > > > > The above didn't work in my tests without this: > > > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > index f603c69..4fb0102 100644 > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > @@ -33,8 +33,10 @@ > > > }; > > > > > > lbsc { > > > + compatible = "simple-bus"; > > > #address-cells = <1>; > > > #size-cells = <1>; > > > + ranges = <0 0 0 0x80000000>; > > > > > > ethernet@8000000 { > > > compatible = "smsc,lan9118", "smsc,lan9115"; > > > > > > > Could you please post this as a formal patch and indicate > > if you would like it included as a fix in v3.10 or not? > > Can do that, sure, just thought maybe it would be better to fix the > original patch. Besides, I wasn't sure what the correct values for > "ranges" are, I just picked up something, that would be sufficient for > ethernet. But if more devices are added to it in the future, maybe > different ranges values would be needed. I think actually, lbsc should map > the 3 BSC areas, so, the correct property would be I believe the original patches are past the point where they can be updated. With regards to the correct values, perhaps Magnus can help here? > > + ranges = <0 0 0 0x14000000>; > > > Also, I am curious to know what your tests are. > > Just booting with NFS root. I think, anything involving ethernet. Without > the "simple-bus" property the lbsc node doesn't get scanned, so, no device > is added. Without "ranges" addresses cannot be mapped correctly. Ok, it sounds like it should be fixed for v3.10.
Hi, Sorry, a bit behind on email and just discovered this. On Wed, May 22, 2013 at 07:22:59PM +0200, Guennadi Liakhovetski wrote: > > > The above didn't work in my tests without this: > > > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > index f603c69..4fb0102 100644 > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > @@ -33,8 +33,10 @@ > > > }; > > > > > > lbsc { > > > + compatible = "simple-bus"; > > > #address-cells = <1>; > > > #size-cells = <1>; > > > + ranges = <0 0 0 0x80000000>; > > > > > > ethernet@8000000 { > > > compatible = "smsc,lan9118", "smsc,lan9115"; > > > > > > > Could you please post this as a formal patch and indicate > > if you would like it included as a fix in v3.10 or not? > > Can do that, sure, just thought maybe it would be better to fix the > original patch. Besides, I wasn't sure what the correct values for > "ranges" are, I just picked up something, that would be sufficient for > ethernet. But if more devices are added to it in the future, maybe > different ranges values would be needed. I think actually, lbsc should map > the 3 BSC areas, so, the correct property would be > > + ranges = <0 0 0 0x14000000>; > > > Also, I am curious to know what your tests are. > > Just booting with NFS root. I think, anything involving ethernet. Without > the "simple-bus" property the lbsc node doesn't get scanned, so, no device > is added. Without "ranges" addresses cannot be mapped correctly. Since it's a flat mapping you can just specify an empty "ranges" property, no need to actually define contents. For more info, see http://devicetree.org/Device_Tree_Usage#Ranges_.28Address_Translation.29 -Olof
Hi Olof On Mon, 27 May 2013, Olof Johansson wrote: > Hi, > > Sorry, a bit behind on email and just discovered this. > > On Wed, May 22, 2013 at 07:22:59PM +0200, Guennadi Liakhovetski wrote: > > > > > The above didn't work in my tests without this: > > > > > > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > index f603c69..4fb0102 100644 > > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > > > > @@ -33,8 +33,10 @@ > > > > }; > > > > > > > > lbsc { > > > > + compatible = "simple-bus"; > > > > #address-cells = <1>; > > > > #size-cells = <1>; > > > > + ranges = <0 0 0 0x80000000>; > > > > > > > > ethernet@8000000 { > > > > compatible = "smsc,lan9118", "smsc,lan9115"; > > > > > > > > > > Could you please post this as a formal patch and indicate > > > if you would like it included as a fix in v3.10 or not? > > > > Can do that, sure, just thought maybe it would be better to fix the > > original patch. Besides, I wasn't sure what the correct values for > > "ranges" are, I just picked up something, that would be sufficient for > > ethernet. But if more devices are added to it in the future, maybe > > different ranges values would be needed. I think actually, lbsc should map > > the 3 BSC areas, so, the correct property would be > > > > + ranges = <0 0 0 0x14000000>; > > > > > Also, I am curious to know what your tests are. > > > > Just booting with NFS root. I think, anything involving ethernet. Without > > the "simple-bus" property the lbsc node doesn't get scanned, so, no device > > is added. Without "ranges" addresses cannot be mapped correctly. > > Since it's a flat mapping you can just specify an empty "ranges" property, no > need to actually define contents. Yes, I knew about that, but does this also hold for busses with different #address-cells properties, i.e. between 64- and 32-bit busses? > For more info, see http://devicetree.org/Device_Tree_Usage#Ranges_.28Address_Translation.29 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Tue, May 28, 2013 at 11:26 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Hi Olof > > On Mon, 27 May 2013, Olof Johansson wrote: > >> Hi, >> >> Sorry, a bit behind on email and just discovered this. >> >> On Wed, May 22, 2013 at 07:22:59PM +0200, Guennadi Liakhovetski wrote: >> >> > > > The above didn't work in my tests without this: >> > > > >> > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts >> > > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts >> > > > index f603c69..4fb0102 100644 >> > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts >> > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts >> > > > @@ -33,8 +33,10 @@ >> > > > }; >> > > > >> > > > lbsc { >> > > > + compatible = "simple-bus"; >> > > > #address-cells = <1>; >> > > > #size-cells = <1>; >> > > > + ranges = <0 0 0 0x80000000>; >> > > > >> > > > ethernet@8000000 { >> > > > compatible = "smsc,lan9118", "smsc,lan9115"; >> > > > >> > > >> > > Could you please post this as a formal patch and indicate >> > > if you would like it included as a fix in v3.10 or not? >> > >> > Can do that, sure, just thought maybe it would be better to fix the >> > original patch. Besides, I wasn't sure what the correct values for >> > "ranges" are, I just picked up something, that would be sufficient for >> > ethernet. But if more devices are added to it in the future, maybe >> > different ranges values would be needed. I think actually, lbsc should map >> > the 3 BSC areas, so, the correct property would be >> > >> > + ranges = <0 0 0 0x14000000>; >> > >> > > Also, I am curious to know what your tests are. >> > >> > Just booting with NFS root. I think, anything involving ethernet. Without >> > the "simple-bus" property the lbsc node doesn't get scanned, so, no device >> > is added. Without "ranges" addresses cannot be mapped correctly. >> >> Since it's a flat mapping you can just specify an empty "ranges" property, no >> need to actually define contents. > > Yes, I knew about that, but does this also hold for busses with different > #address-cells properties, i.e. between 64- and 32-bit busses? Oh, good question -- I don't think it does. But why go down, you might as well use 2 cells in that part of the tree too? -Olof
On Wed, 29 May 2013, Olof Johansson wrote: > On Tue, May 28, 2013 at 11:26 PM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > Hi Olof > > > > On Mon, 27 May 2013, Olof Johansson wrote: > > > >> Hi, > >> > >> Sorry, a bit behind on email and just discovered this. > >> > >> On Wed, May 22, 2013 at 07:22:59PM +0200, Guennadi Liakhovetski wrote: > >> > >> > > > The above didn't work in my tests without this: > >> > > > > >> > > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > >> > > > b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > >> > > > index f603c69..4fb0102 100644 > >> > > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts > >> > > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts > >> > > > @@ -33,8 +33,10 @@ > >> > > > }; > >> > > > > >> > > > lbsc { > >> > > > + compatible = "simple-bus"; > >> > > > #address-cells = <1>; > >> > > > #size-cells = <1>; > >> > > > + ranges = <0 0 0 0x80000000>; > >> > > > > >> > > > ethernet@8000000 { > >> > > > compatible = "smsc,lan9118", "smsc,lan9115"; > >> > > > > >> > > > >> > > Could you please post this as a formal patch and indicate > >> > > if you would like it included as a fix in v3.10 or not? > >> > > >> > Can do that, sure, just thought maybe it would be better to fix the > >> > original patch. Besides, I wasn't sure what the correct values for > >> > "ranges" are, I just picked up something, that would be sufficient for > >> > ethernet. But if more devices are added to it in the future, maybe > >> > different ranges values would be needed. I think actually, lbsc should map > >> > the 3 BSC areas, so, the correct property would be > >> > > >> > + ranges = <0 0 0 0x14000000>; > >> > > >> > > Also, I am curious to know what your tests are. > >> > > >> > Just booting with NFS root. I think, anything involving ethernet. Without > >> > the "simple-bus" property the lbsc node doesn't get scanned, so, no device > >> > is added. Without "ranges" addresses cannot be mapped correctly. > >> > >> Since it's a flat mapping you can just specify an empty "ranges" property, no > >> need to actually define contents. > > > > Yes, I knew about that, but does this also hold for busses with different > > #address-cells properties, i.e. between 64- and 32-bit busses? > > Oh, good question -- I don't think it does. But why go down, you might > as well use 2 cells in that part of the tree too? Ok, I didn't write the original implementation of that bus and, therefore, didn't decide how many address cells it shall use. My task was to fix it. And yes, I agree, that there are 2 ways to fix the memory mapping: either define ranges to map from 64-bits to 32-bits, or to change all devices on the bus (currently there's only 1) to use 64-bit "reg" property and use "ranges" with no arguments. Since that bus is clearly within a 32-bit area, I would prefer the former solution not to have to drag 64 bits around an make device nodes simpler. Can we keep this approach or are you strongly against it? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
On Thursday 30 May 2013, Guennadi Liakhovetski wrote: > Ok, I didn't write the original implementation of that bus and, therefore, > didn't decide how many address cells it shall use. My task was to fix it. > And yes, I agree, that there are 2 ways to fix the memory mapping: either > define ranges to map from 64-bits to 32-bits, or to change all devices on > the bus (currently there's only 1) to use 64-bit "reg" property and use > "ranges" with no arguments. Since that bus is clearly within a 32-bit > area, I would prefer the former solution not to have to drag 64 bits > around an make device nodes simpler. Can we keep this approach or are you > strongly against it? I believe we should attempt to have the DT representation as close as possible to what the hardware actually does. If this is a AHB bus or something else that is known to be always 32 bit, I would use a ranges property and #address-cells=<1>, but if this is a 64-bit wide AXIv4 instance I would use #address-cells=<2> and probably omit the ranges. There is also the case where a device is attached to an external bus that has 0-based addressing. In that case I would put the actual translation into the ranges property, with all addresses that are translated by the bus controller, such as lbsc { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges = <0 0x08000000 0 0x1000000>; /* 16 MB @ 128 MB */ ethernet@0 { compatible = "smsc,lan9118", "smsc,lan9115"; reg = <0 0x1000>; ... Arnd
diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm.dts b/arch/arm/boot/dts/r8a73a4-ape6evm.dts index f603c69..4fb0102 100644 --- a/arch/arm/boot/dts/r8a73a4-ape6evm.dts +++ b/arch/arm/boot/dts/r8a73a4-ape6evm.dts @@ -33,8 +33,10 @@ }; lbsc { + compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; + ranges = <0 0 0 0x80000000>; ethernet@8000000 { compatible = "smsc,lan9118", "smsc,lan9115";