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

From: Timur Tabi <[email-masked]>
Date: Wed, 22 Aug 2012 17:38:37 -0500

David Daney wrote:

> I wonder if *fpga is really a good name for this.  It is a general 
> purpose multiplexer with a memory mapped control register.  I would call 
> it something like mdio-mux-mmioreg.

At one point, I thought of using mdio-mux-bitbang, but -mmioreg is better.
 Thanks.

>> +- mdio-mux-device : phandle, points to the FPGA (or similar) node.  This
>> +	must be a memory-mapped device with 8-bit registers.
> 
> You shouldn't need this.  Just make the multiplexer a child of FPGA node 
> to indicate where it lives.

The problem is that we don't normally consider the FPGA node to be a bus,
so its child nodes won't get probed.  That's why I have this:

compatible = "mdio-mux-fpga", "mdio-mux";
                               ^^^^^^^^

This allows me to have multiple mdio-mux parent nodes (which I do, since I
have multiple mdio bus muxes), and they all get registered and probed
properly because I also do this:

static const struct of_device_id of_device_ids[] __devinitconst = {
	{
		.compatible	= "simple-bus"
	},
	{
		.compatible	= "fsl,srio",
	},
...
	{
		.compatible	= "mdio-mux",
	},
	{}
};

The .compatible = "mdio-mux" is what causes all of the mdio-mux nodes to
be registered.  Therefore, it's simpler if all the mdio-mux nodes are root
nodes.

>> +
>> +- mdio-mux-register : integer, contains the offset of the register that
>> +	controls the bus multiplexer.
> 
> This should just be the normal "reg" properly

Ok.

>> +- mdio-mux-mask : integer, contains an 8-bit mask that specifies which
>> +	bits in the register control the actual bus multiplexer.  The
>> +	'reg' property of each child mdio-mux node must be constrained by
>> +	this mask.
>> +
> 
> "reg-mask" ??

Ok.

> Do you need a shift too?

The 'reg' property of the mdio bus child nodes should take the shift into
account.  That's why, in the example, I have mask=0x6 and reg=0 or reg=2.
 There's even code in the driver to make sure that the 'reg' values are
constrained to the mask.

-- 
Timur Tabi
Linux kernel developer at Freescale