On Tue, Feb 9, 2010 at 4:00 PM, John Linn <john.l...@xilinx.com> wrote: >> -----Original Message----- >> From: John Williams [mailto:john.willi...@petalogix.com] >> Sent: Tuesday, February 09, 2010 3:30 PM >> To: John Linn >> Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; jgar...@pobox.com; >> grant.lik...@secretlab.ca; >> jwbo...@linux.vnet.ibm.com; Sadanand Mutyala >> Subject: Re: [PATCH] [V3] net: emaclite: adding MDIO and phy lib support >> >> Hi John, >> >> Sorry If I'm painting bike-sheds here, just one tiny tweak might be in >> order to standardise your mutex_unlock exit path: >> >> > +static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg) >> > +{ >> > + struct net_local *lp = bus->priv; >> > + u32 ctrl_reg; >> > + u32 rc; >> > + >> > + mutex_lock(&lp->mdio_mutex); >> > + >> > + if (xemaclite_mdio_wait(lp)) { >> > + mutex_unlock(&lp->mdio_mutex); >> > + return -ETIMEDOUT; >> > + } >> >> [snip] >> >> >> > + if (xemaclite_mdio_wait(lp)) { >> > + mutex_unlock(&lp->mdio_mutex); >> > + return -ETIMEDOUT; >> > + } >> >> [snip] >> >> >> > + dev_dbg(&lp->ndev->dev, >> > + "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n", >> > + phy_id, reg, rc); >> > + >> > + return rc; >> >> Can this be better expressed like this: >> >> my_func() { >> mutex_lock() >> .. >> >> if(some error) { >> rc=-ETIMEDOUT; >> goto out_unlock; >> } >> ... >> >> /* success path */ >> rc=0; >> .. >> out_unlock: >> mutex_unlock() >> return rc; >> } >> >> >> Is this style still favoured in driver exit paths? > > It looks to me like the mutex is not needed in the driver mdio functions as > there's a mutex in the mdiobus functions already.
Yes, you're correct, but you still need to protect against direct calls to the read/write routines from within the driver. But you can probably use the mdio_lock mutex for this. g. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
