Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[Tabi Timur-B04825]

On Wed, May 2, 2012 at 8:16 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
>
> This code has been working well for about six months on a couple of
> different configurations (boards), so I thought it would be a good
> time to send it out again, and I hope get it on the path towards
> merging.

David,

I'm trying to implement this feature on our boards, which don't use
GPIOs but rather a memory-mapped FPGA.  I control the mux by setting
some bits in one of the FPGA registers.

Do you have a real device tree I can use as an example?

I'm not sure what the "parent" MDIO bus node is supposed to represent.
 Is that that device that actually controls the muxing hardware, which
in our case would be the FPGA?

--=20
Timur Tabi
Linux kernel developer at Freescale=

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[David Daney]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On 05/18/2012 02:42 PM, Tabi Timur-B04825 wrote:
> On Wed, May 2, 2012 at 8:16 PM, David Daney<ddaney.cavm@gmail.com>  wrote:
>> From: David Daney<david.daney@cavium.com>
>>
>> This code has been working well for about six months on a couple of
>> different configurations (boards), so I thought it would be a good
>> time to send it out again, and I hope get it on the path towards
>> merging.
>
> David,
>
> I'm trying to implement this feature on our boards, which don't use
> GPIOs but rather a memory-mapped FPGA.  I control the mux by setting
> some bits in one of the FPGA registers.

You can either:

1) write a standard GPIO driver for the thing that controls the mux, and 
then use mdio-mux-gpio.c, or...

2) Write a new driver modeled on mdio-mux-gpio.c for your switch control.

>
> Do you have a real device tree I can use as an example?
>

Attached.

> I'm not sure what the "parent" MDIO bus node is supposed to represent.
>   Is that that device that actually controls the muxing hardware

No.  It is the device that implements the master 802.3 clause {22,45} 
MDIO Station Management (STA) protocol.

>, which in our case would be the FPGA?

I have no idea what your FPGA implements, so it is hard to say.

A sane person would implement a separate MDIO STA controller for each 
bus, in which case you wouldn't use the multiplexer driver.

Only people dealing with insane hardware need the multiplexer.  The 
patch in net-next has a nice ASCII art picture of such an insane design.

David Daney

[-- Attachment #2: ebb6600.dts --]
[-- Type: audio/vnd.dts, Size: 16812 bytes --]

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[Timur Tabi]

David Daney wrote:
>> > I'm not sure what the "parent" MDIO bus node is supposed to represent.
>> >   Is that that device that actually controls the muxing hardware

> No.  It is the device that implements the master 802.3 clause {22,45} 
> MDIO Station Management (STA) protocol.

Ah, I think I get it.  It is *the* MDIO node that would normally exist if
muxing we're necessary on the board.  From the looks of it, that node
would look exactly the same if you didn't need muxing?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[David Daney]

On 05/18/2012 03:09 PM, Timur Tabi wrote:
> David Daney wrote:
>>>> I'm not sure what the "parent" MDIO bus node is supposed to represent.
>>>>    Is that that device that actually controls the muxing hardware
>
>> No.  It is the device that implements the master 802.3 clause {22,45}
>> MDIO Station Management (STA) protocol.
>
> Ah, I think I get it.  It is *the* MDIO node that would normally exist if
> muxing we're necessary on the board.  From the looks of it, that node
> would look exactly the same if you didn't need muxing?
>

Yes.  You may note in the DTS file I attached in the parent (sorry for 
the fubar mime types), that there are two, almost identical, MDIO 
masters.  smi0 has two directly attached PHYs.  smi1 goes to the mux, 
and each child of the mux has four attached PHYs.

This is a fairly complex configuration as the GPIOs controlling the MDIO 
mux are on I2C GPIO expanders which are themselves behind an I2C mux...

The nice thing about this is that the Linux I2C and MDIO infrastructure 
is all configured dynamically from the device tree and everything works 
well together with no locking issues.  The addition of the deferred 
driver probe mechanism was the last part of the puzzle (I think... 
actually I don't know if all my I2C things are merged yet...).

David Daney

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[Timur Tabi]

David Daney wrote:
> Yes.  You may note in the DTS file I attached in the parent (sorry for 
> the fubar mime types), that there are two, almost identical, MDIO 
> masters.  smi0 has two directly attached PHYs.  smi1 goes to the mux, 
> and each child of the mux has four attached PHYs.

I'm till have trouble understanding all this.  I'm just hacking things up
in order to help me understand it, but it's a slow and painful process.

This call in mdio_mux_init() is failing:

	parent_bus = of_mdio_find_bus(parent_bus_node);

It returns NULL.  Here is my MDIO node:

		fman0: fman@400000 {
			enet0: ethernet@e0000 {
				tbi-handle = <&tbi0>;
				phy-handle = <&phy0>;
				phy-connection-type = "sgmii";
			};

			mdio0: mdio@e1120 {
				gpios = <&gpio0 0 0
					 &gpio0 1 0>;

				tbi0: tbi-phy@8 {
					reg = <0x8>;
					device_type = "tbi-phy";
				};

				phy0: ethernet-phy@1c {
					reg = <0x1c>;
				};
			};
		};

What am I missing?  What do I need to do in order to get
of_mdio_find_bus() to return a real value?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[David Daney]

On 05/24/2012 11:28 AM, Timur Tabi wrote:
> David Daney wrote:
>> Yes.  You may note in the DTS file I attached in the parent (sorry for
>> the fubar mime types), that there are two, almost identical, MDIO
>> masters.  smi0 has two directly attached PHYs.  smi1 goes to the mux,
>> and each child of the mux has four attached PHYs.
>
> I'm till have trouble understanding all this.  I'm just hacking things up
> in order to help me understand it, but it's a slow and painful process.
>
> This call in mdio_mux_init() is failing:
>
> 	parent_bus = of_mdio_find_bus(parent_bus_node);
>

Well, the MDIO bus must have an associated device tree node.

For my OCTEON code, the MDIO bus device is created as a result of the 
call to of_platform_bus_probe(), which takes care of filling in all the 
device tree nodes of the devices it finds and creates.

> It returns NULL.  Here is my MDIO node:
>
> 		fman0: fman@400000 {
> 			enet0: ethernet@e0000 {
> 				tbi-handle =<&tbi0>;
> 				phy-handle =<&phy0>;
> 				phy-connection-type = "sgmii";
> 			};
>
> 			mdio0: mdio@e1120 {
> 				gpios =<&gpio0 0 0
> 					&gpio0 1 0>;
>
> 				tbi0: tbi-phy@8 {
> 					reg =<0x8>;
> 					device_type = "tbi-phy";
> 				};
>
> 				phy0: ethernet-phy@1c {
> 					reg =<0x1c>;
> 				};
> 			};
> 		};
>
> What am I missing?

For starters, I do not see any compatible properties that would allow 
the proper drivers to be bound to anything.

Also I see no MDIO mux node there, so it is unclear why you are even 
asking these questions.

David Daney

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[Timur Tabi]

David Daney wrote:

> Well, the MDIO bus must have an associated device tree node.
> 
> For my OCTEON code, the MDIO bus device is created as a result of the 
> call to of_platform_bus_probe(), which takes care of filling in all the 
> device tree nodes of the devices it finds and creates.

Ok, let me give you some background.  We actually already have MDIO muxing
code in-house, but it's different from yours.  So now I'm rewriting it to
use your design instead.

So our current code looks for "virtual MDIO nodes", and we call
mdiobus_alloc() and then of_mdiobus_register().  I think this is what I'm
missing now.

I just don't know what to do next.  Part of the problem is that I don't
have much experience with MDIO drivers.

>> It returns NULL.  Here is my MDIO node:
>>
>> 		fman0: fman@400000 {
>> 			enet0: ethernet@e0000 {
>> 				tbi-handle =<&tbi0>;
>> 				phy-handle =<&phy0>;
>> 				phy-connection-type = "sgmii";
>> 			};
>>
>> 			mdio0: mdio@e1120 {
>> 				gpios =<&gpio0 0 0
>> 					&gpio0 1 0>;
>>
>> 				tbi0: tbi-phy@8 {
>> 					reg =<0x8>;
>> 					device_type = "tbi-phy";
>> 				};
>>
>> 				phy0: ethernet-phy@1c {
>> 					reg =<0x1c>;
>> 				};
>> 			};
>> 		};
>>
>> What am I missing?
> 
> For starters, I do not see any compatible properties that would allow 
> the proper drivers to be bound to anything.

Ok, that makes sense.

> Also I see no MDIO mux node there, so it is unclear why you are even 
> asking these questions.

I only gave you part of the device tree.  Here's my mdio mux node:

	mdio-mux {
		compatible = "mdio-mux-gpio";
		gpios = <&gpio0 0 0>, <&gpio0 1 0>;
		mdio-parent-bus = <&mdio0>;
		#address-cells = <1>;
		#size-cells = <0>;

		mdio@2 {
			reg = <2>;
			#address-cells = <1>;
			#size-cells = <0>;

			phy21: ethernet-phy@1 {
				reg = <1>;
//				compatible = "marvell,88e1149r", "ethernet-phy-ieee802.3-c22";
				marvell,reg-init = <3 0x10 0 0x5777>,
					<3 0x11 0 0x00aa>,
					<3 0x12 0 0x4105>,
					<3 0x13 0 0x0a60>;
				interrupt-parent = <&gpio0>;
//				interrupts = <10 8>; /* Pin 10, active low */
			};
		};
	};
};




> 
> David Daney
> 


-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v6 0/3] netdev/of/phy: MDIO bus multiplexer support.

[David Daney]

On 05/24/2012 12:03 PM, Timur Tabi wrote:
> David Daney wrote:
>
>> Well, the MDIO bus must have an associated device tree node.
>>
>> For my OCTEON code, the MDIO bus device is created as a result of the
>> call to of_platform_bus_probe(), which takes care of filling in all the
>> device tree nodes of the devices it finds and creates.
>
> Ok, let me give you some background.  We actually already have MDIO muxing
> code in-house, but it's different from yours.  So now I'm rewriting it to
> use your design instead.
>
> So our current code looks for "virtual MDIO nodes", and we call
> mdiobus_alloc() and then of_mdiobus_register().  I think this is what I'm
> missing now.
>
> I just don't know what to do next.

You will have to debug it and find out why the device match is failing, 
then fix it.

David Daney