Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

5 messages, 3 authors, 2016-08-29 · open the first message on its own page

Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

From: Arnd Bergmann <hidden>
Date: 2016-08-22 11:55:22

On Saturday, August 20, 2016 11:35:35 AM CEST Martin Blumenstingl wrote:
+- reg: The first register range should be the one of the DWMAC
+       controller. The second range is is for the Amlogic specific
+       configuration (for example the PRG_ETHERNET register range
+       on Meson8b and newer)
 
...
+Example for GXBB:
+       ethmac: ethernet@c9410000 {
+               compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
+               reg = <0x0 0xc9410000 0x0 0x10000>,
+                       <0x0 0xc8834540 0x0 0x8>;
The address "0xc8834540" suggests that this is part of a larger register
range that is used for various things, i.e. a "syscon" type of device.

How about making this a syscon reference rather than a "reg" address?

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

Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

From: Martin Blumenstingl <hidden>
Date: 2016-08-22 12:04:49

On Mon, Aug 22, 2016 at 1:55 PM, Arnd Bergmann [off-list ref] wrote:
On Saturday, August 20, 2016 11:35:35 AM CEST Martin Blumenstingl wrote:
quoted
+- reg: The first register range should be the one of the DWMAC
+       controller. The second range is is for the Amlogic specific
+       configuration (for example the PRG_ETHERNET register range
+       on Meson8b and newer)
...
quoted
+Example for GXBB:
+       ethmac: ethernet@c9410000 {
+               compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
+               reg = <0x0 0xc9410000 0x0 0x10000>,
+                       <0x0 0xc8834540 0x0 0x8>;
The address "0xc8834540" suggests that this is part of a larger register
range that is used for various things, i.e. a "syscon" type of device.
You are right, these are part of the cbus range (which is already
defined in meson-gxbb.dtsi)
How about making this a syscon reference rather than a "reg" address?
The first version of my patch ([0]) used
syscon_regmap_lookup_by_phandle. Maybe I did it wrong (and I should
have passed the cbus syscon-node instead of defining a new one just
for the 2x32bit PRG_ETHERNET registers).
I am perfectly fine with either way - however it seems that some other
dwmac glue implementations are also using a second set of resources
(that doesn't automatically make it "correct" though).


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

Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-08-22 15:25:30

On Monday, August 22, 2016 2:04:49 PM CEST Martin Blumenstingl wrote:
On Mon, Aug 22, 2016 at 1:55 PM, Arnd Bergmann [off-list ref] wrote:
quoted
On Saturday, August 20, 2016 11:35:35 AM CEST Martin Blumenstingl wrote:
quoted
+- reg: The first register range should be the one of the DWMAC
+       controller. The second range is is for the Amlogic specific
+       configuration (for example the PRG_ETHERNET register range
+       on Meson8b and newer)
...
quoted
+Example for GXBB:
+       ethmac: ethernet@c9410000 {
+               compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
+               reg = <0x0 0xc9410000 0x0 0x10000>,
+                       <0x0 0xc8834540 0x0 0x8>;
The address "0xc8834540" suggests that this is part of a larger register
range that is used for various things, i.e. a "syscon" type of device.
You are right, these are part of the cbus range (which is already
defined in meson-gxbb.dtsi)
quoted
How about making this a syscon reference rather than a "reg" address?
The first version of my patch ([0]) used
syscon_regmap_lookup_by_phandle. Maybe I did it wrong (and I should
have passed the cbus syscon-node instead of defining a new one just
for the 2x32bit PRG_ETHERNET registers).
I am perfectly fine with either way - however it seems that some other
dwmac glue implementations are also using a second set of resources
(that doesn't automatically make it "correct" though).
It really depends on the kind of SoC. Some may have a suboptimal
binding, on some others there may be a distinct register area that
just contains a few additional registers for the dwmac.

	Arnd

Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

From: Martin Blumenstingl <hidden>
Date: 2016-08-28 16:15:31

On Mon, Aug 22, 2016 at 5:25 PM, Arnd Bergmann [off-list ref] wrote:
It really depends on the kind of SoC. Some may have a suboptimal
binding, on some others there may be a distinct register area that
just contains a few additional registers for the dwmac.
the dwmac PHY configuration registers (2x32bit) on the GXBB SoC are
part of the "periphs" region/module. This is already defined as
"simple-bus" in meson-gxbb.dtsi, see [0]
On Meson8b this is slightly different: there is no specific "periphs"
region - there the dwmac PHY configuration registers are directly
located in the cbus region at a slightly different offset than on the
GXBB SoCs.

In the future we might need a third memory region because the latest
reference kernel contains some more PHY configuration registers on
newer SoCs (GXL = S905X).

Please let me know if you're OK with the dts definition in it's
current state - or let me know how you would like to change it.

PS: I will re-send the patches in a v3 in a few minutes because that
fixes a bug during module unload.


Regards,
Martin


[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi#n217
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] net: dt-bindings: Document the new Meson8b and GXBB DWMAC bindings

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-08-29 13:31:12

On Sunday 28 August 2016, Martin Blumenstingl wrote:
On Mon, Aug 22, 2016 at 5:25 PM, Arnd Bergmann [off-list ref] wrote:
quoted
It really depends on the kind of SoC. Some may have a suboptimal
binding, on some others there may be a distinct register area that
just contains a few additional registers for the dwmac.
the dwmac PHY configuration registers (2x32bit) on the GXBB SoC are
part of the "periphs" region/module. This is already defined as
"simple-bus" in meson-gxbb.dtsi, see [0]
On Meson8b this is slightly different: there is no specific "periphs"
region - there the dwmac PHY configuration registers are directly
located in the cbus region at a slightly different offset than on the
GXBB SoCs.

In the future we might need a third memory region because the latest
reference kernel contains some more PHY configuration registers on
newer SoCs (GXL = S905X).

Please let me know if you're OK with the dts definition in it's
current state - or let me know how you would like to change it.

PS: I will re-send the patches in a v3 in a few minutes because that
fixes a bug during module unload.
I don't really see a good way to describe this hardware then. If it
was only the first case, I'd suggest marking the periphs bus node
as "compatible="simple-bus","syscon";" so you could have a
reference to it, but that doesn't seem to work well in the second
case, unless you can a separate DT node just for the PHY config
registers there.

With the third case, is there any logic at all behind the
register map?

Maybe someone else has a better idea for how to describe this.
In general, we try to avoid overlapping "reg" properties, but I
even see that the "periphs" node  on gxbb has a "reg" property
(is this intentional) that overlaps with the registers in its
ranges, so adding another one won't make this worse than it
already is.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help