netdev - Re: [PATCHv3 5/5] [NET] dsa: add support for the Marvell 88E6060 switch chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 7 Oct 2008 17:45:41 -0700 (PDT)
From:	Trent Piepho <tpiepho@...escale.com>
To:	Lennert Buytenhek <buytenh@...tstofly.org>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCHv3 5/5] [NET] dsa: add support for the Marvell 88E6060
 switch chip

On Wed, 8 Oct 2008, Lennert Buytenhek wrote:
> +
> +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.

> +#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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html