On Wed, 8 Oct 2008, Lennert Buytenhek wrote:
quoted text
> +
> +static int reg_read(struct dsa_switch *ds, int addr, int reg)
> +{
> +	return mdiobus_read(ds->master_mii_bus, addr, reg);
> +}

Instead of all the code to add the mdio buses to the device tree and exporting
mdiobus_read(), could you have just added the switch chip as a "phy" on the
mdio bus?  Then used phy_read/write() on it?  I know it's not a phy, but you
don't need to use the phy state machine and so on.

Freescale gianfar MACs configure the serdes for SGMII using a virtual "TBI"
device on the MDIO bus.  Supposedly Andy has patch that adds them as devices
on the MDIO bus, isn't of the gianfar specific MDIO back-door they currently
use.

quoted text
> +#define REG_READ(addr, reg)					\
> +	({							\
> +		int __ret;					\
> +								\
> +		__ret = reg_read(ds, addr, reg);		\
> +		if (__ret < 0)					\
> +			return __ret;				\
> +		__ret;						\
> +	})

Macro with a hidden use of a local ('ds') and a hidden return?  The former is
discouraged (but can sure save a lot of typing) but the latter seems like it's
just begging to cause a missing unlock or free on an error path.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html