Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

8 messages, 5 authors, 2012-08-24 · open the first message on its own page

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Stephen Warren <hidden>
Date: 2012-08-24 02:46:30

On 08/23/2012 06:28 PM, Tabi Timur-B04825 wrote:
Stephen Warren wrote:
quoted
On 08/23/2012 01:44 PM, Timur Tabi wrote:
quoted
Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
device, like an FPGA.  The device must be memory-mapped and contain only
8-bit registers (which keeps things simple).
quoted
+++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
quoted
+	/* The FPGA node */
+	fpga: board-control@3,0 {
+		compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+		reg = <3 0 0x30>;
Why not add the following here:

	#address-cells = <1>:
	#size-cells = <1>;
	ranges = <...>;
I forgot to add them in the txt file.  They are in the real device tree.
quoted
quoted
+
+		mdio-mux-emi2 {
+			compatible = "mdio-mux-mmioreg", "mdio-mux";
+			mdio-parent-bus = <&xmdio0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <9>; // BRDCFG1
Then, that'd have to be <9 1>;
Actually, I had #size-cells = <0>.
I think that if you have #size-cells=<0>, then you'll see the following
error message when attempting to translate the address into the parent's
address space:

prom_parse: Bad cell count for /board-control@3,0/mdio-mux-emi2

I fixed that error for cases where actual address translation isn't
required (i.e. coming up with the platform device name), but IIRC it'll
still trigger if you actually want to translate the address.

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Timur Tabi <hidden>
Date: 2012-08-24 16:27:19

Stephen Warren wrote:
quoted
quoted
quoted
quoted
Then, that'd have to be <9 1>;
Actually, I had #size-cells = <0>.
I think that if you have #size-cells=<0>, then you'll see the following
error message when attempting to translate the address into the parent's
address space:

prom_parse: Bad cell count for /board-control@3,0/mdio-mux-emi2
It doesn't appear to be working.  Here's my tree:

/ {
	model = "fsl,P5020DS";
	compatible = "fsl,P5020DS";
	#address-cells = <2>;
	#size-cells = <2>;
	interrupt-parent = <&mpic>;

	lbc: localbus@ffe124000 {
		reg = <0xf 0xfe124000 0 0x1000>;
		ranges = <0 0 0xf 0xe8000000 0x08000000
			  2 0 0xf 0xffa00000 0x00040000
			  3 0 0xf 0xffdf0000 0x00008000>;

		fpga: board-control@3,0 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
			reg = <3 0 0x30>;

			mdio-mux-emi1 {
				compatible = "mdio-mux-mmioreg";
				mdio-parent-bus = <&mdio0>;
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <9 1>; // BRDCFG1
				mux-mask = <0x78>; // EMI1

That means that the physical address that I need is fffdf0009.  However,
when I call of_address_to_resource(), the returned address I get is fe8000009.

So it's not picking up the "3" in the 'reg' property of the
board-control@3,0 node.  What am I missing?  Do I need a 'ranges' property
in the board-control@3,0 node?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Stephen Warren <hidden>
Date: 2012-08-24 18:29:41

On 08/24/2012 10:27 AM, Timur Tabi wrote:
Stephen Warren wrote:
quoted
quoted
quoted
quoted
quoted
Then, that'd have to be <9 1>;
Actually, I had #size-cells = <0>.
quoted
I think that if you have #size-cells=<0>, then you'll see the following
error message when attempting to translate the address into the parent's
address space:

prom_parse: Bad cell count for /board-control@3,0/mdio-mux-emi2
It doesn't appear to be working.  Here's my tree:

/ {
	model = "fsl,P5020DS";
	compatible = "fsl,P5020DS";
	#address-cells = <2>;
	#size-cells = <2>;
	interrupt-parent = <&mpic>;

	lbc: localbus@ffe124000 {
		reg = <0xf 0xfe124000 0 0x1000>;
		ranges = <0 0 0xf 0xe8000000 0x08000000
			  2 0 0xf 0xffa00000 0x00040000
			  3 0 0xf 0xffdf0000 0x00008000>;

		fpga: board-control@3,0 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
			reg = <3 0 0x30>;

			mdio-mux-emi1 {
				compatible = "mdio-mux-mmioreg";
				mdio-parent-bus = <&mdio0>;
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <9 1>; // BRDCFG1
				mux-mask = <0x78>; // EMI1

That means that the physical address that I need is fffdf0009.  However,
when I call of_address_to_resource(), the returned address I get is fe8000009.

So it's not picking up the "3" in the 'reg' property of the
board-control@3,0 node.  What am I missing?  Do I need a 'ranges' property
in the board-control@3,0 node?
Yes.

When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.

I thought the code error-checked for a missing ranges property, but I
guess not...

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Timur Tabi <hidden>
Date: 2012-08-24 18:36:31

Stephen Warren wrote:
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.

I thought the code error-checked for a missing ranges property, but I
guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent.  For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.

Anyway, thanks to Scott for helping me figure this out.  I was missing a
ranges property:

	fpga: board-control@3,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
		reg = <3 0 0x30>;
		ranges = <0 3 0 0x30>;

This maps the child address of 0 to the parent address of 3 0.  It seems
obvious now, but it was driving me crazy.  We've never put child devices
under our FPGA nodes, so there was no prior use case of a 'ranges'
property in any of the localbus devices that I could learn from.  Plus,
this is the first time we're probing directly on a child of a localbus device.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Scott Wood <hidden>
Date: 2012-08-24 18:43:04

On 08/24/2012 01:36 PM, Timur Tabi wrote:
Stephen Warren wrote:
quoted
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.

I thought the code error-checked for a missing ranges property, but I
guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent.  For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.

Anyway, thanks to Scott for helping me figure this out.  I was missing a
ranges property:

	fpga: board-control@3,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
		reg = <3 0 0x30>;
		ranges = <0 3 0 0x30>;

This maps the child address of 0 to the parent address of 3 0.  It seems
obvious now, but it was driving me crazy.  We've never put child devices
under our FPGA nodes, so there was no prior use case of a 'ranges'
property in any of the localbus devices that I could learn from.  Plus,
this is the first time we're probing directly on a child of a localbus device.
There's ep8248e.dts, not that I'd have expected you to look there. :-)

-Scott

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Stephen Warren <hidden>
Date: 2012-08-24 18:56:05

On 08/24/2012 12:36 PM, Timur Tabi wrote:
Stephen Warren wrote:
quoted
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.

I thought the code error-checked for a missing ranges property, but I
guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent.  For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.
In the I2C case, the address spaces are disjoint, so there's never any
mapping between them, so there's no need for ranges.

Any time the child address space is intended to be part of the parent's
address space, I believe ranges is supposed to be specified, perhaps
even mandatory, even if the translation is 1:1.
Anyway, thanks to Scott for helping me figure this out.  I was missing a
ranges property:

	fpga: board-control@3,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
		reg = <3 0 0x30>;
		ranges = <0 3 0 0x30>;

This maps the child address of 0 to the parent address of 3 0.
Yes, that looks reasonable.

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: David Miller <davem@davemloft.net>
Date: 2012-08-24 19:07:40

From: Stephen Warren <redacted>
Date: Fri, 24 Aug 2012 12:56:05 -0600
In the I2C case, the address spaces are disjoint, so there's never any
mapping between them, so there's no need for ranges.

Any time the child address space is intended to be part of the parent's
address space, I believe ranges is supposed to be specified, perhaps
even mandatory, even if the translation is 1:1.
Regardless, you really can't just generically translate ranges
in some universal way and expect it to work in all cases.

You need bus specific drivers to deal with various special
cases.

See the *_map() methods implemented in:

	arch/sparc/kernel/of_device_64.c

for example.

Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device

From: Scott Wood <hidden>
Date: 2012-08-24 19:18:02

On 08/24/2012 02:07 PM, David Miller wrote:
From: Stephen Warren <redacted>
Date: Fri, 24 Aug 2012 12:56:05 -0600
quoted
In the I2C case, the address spaces are disjoint, so there's never any
mapping between them, so there's no need for ranges.

Any time the child address space is intended to be part of the parent's
address space, I believe ranges is supposed to be specified, perhaps
even mandatory, even if the translation is 1:1.
Yes, it's mandatory (even if the kernel lets you get away without it for
the sake of some broken Apple firmware, IIRC).  If the translation is an
identity map you can use an empty "ranges;".
Regardless, you really can't just generically translate ranges
in some universal way and expect it to work in all cases.

You need bus specific drivers to deal with various special
cases.

See the *_map() methods implemented in:

	arch/sparc/kernel/of_device_64.c

for example.
We don't expect it to work in all cases.  We expect it to work if the
bus node is on the whitelist for which we create devices on
platform_bus, there's a platform driver that binds against it, and that
driver calls of_iomap() or equivalent because the binding says that reg
refers to something that is memory mapped.

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