Thread (5 messages) 5 messages, 4 authors, 2012-08-30

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

From: Timur Tabi <hidden>
Date: 2012-08-24 20:00:32
Also in: netdev

Stephen Warren wrote:
quoted
+This is a special case of a MDIO bus multiplexer.  A memory-mapped device,
+like an FPGA, is used to control which child bus is connected.  The mdio-mux
+node must be a child of the memory-mapped device.  The driver currently only
+supports devices with eight-bit registers.
That last sentence seems like a property of the driver, not the binding;
I could easily anticipate allowing the size to be 1 or 2 or 4, and a
driver adapter to that in the future.
True, but I couldn't think of a better place to mention this.  Adding
support for multi-byte registers also requires handling the endianness of
those registers.  I have that problem with the mdio-mux-gpio driver.  That
driver assumes that the GPIO bits are numbered in little-endian order, so
my device tree on my big-endian CPU (PowerPC) lists the GPIO pins in
reverse order.
Otherwise, this binding looks great now.
Do you still want me to scrub any references to the register size
requirement from the document?
quoted
+++ b/drivers/net/phy/mdio-mux-mmioreg.c
quoted
+static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
+				      void *data)
+{
+	struct mdio_mux_mmioreg_state *s = data;
+
+	if (current_child ^ desired_child) {
+		void *p = ioremap(s->phys, 1);
+		uint8_t x, y;
+
+		if (!p)
+			return -ENOMEM;
Why not map it during probe?
I thought about that, but I generally don't like mappings that exist for
all eternity even though they're rarely used.  Once the interface is up,
we don't expect any bus muxing to occur.
quoted
+		x = ioread8(p);
+		y = (x & ~s->mask) | desired_child;
+		if (x != y) {
Isn't that always true, given if (current_child ^ desired_child) above?
If current_child == -1, but the bus is already muxed properly, then
there's no point in setting it.  Do you want me to remove the test, or add
a comment?

-- 
Timur Tabi
Linux kernel developer at Freescale
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help