Subject: Re: [PATCH net-next] net: phy: icplus: Call phy_restore_page() when phy_select_page() fails

From: Russell King - ARM Linux admin <[email-masked]>
Date: Wed, 17 Feb 2021 10:21:58 +0000

On Wed, Feb 17, 2021 at 11:12:11AM +0100, Michael Walle wrote:
> Am 2021-02-17 11:04, schrieb Russell King - ARM Linux admin:
> > On Wed, Feb 17, 2021 at 09:17:59AM +0300, Dan Carpenter wrote:
> > > Smatch warns that there is a locking issue in this function:
> > > 
> > > drivers/net/phy/icplus.c:273 ip101a_g_config_intr_pin()
> > > warn: inconsistent returns '&phydev->mdio.bus->mdio_lock'.
> > >   Locked on  : 242
> > >   Unlocked on: 273
> > > 
> > > It turns out that the comments in phy_select_page() say we have to
> > > call
> > > phy_restore_page() even if the call to phy_select_page() fails.
> > 
> > It seems it's a total waste of time documenting functions...
> 
> You once said
> 
> """
> Kernel development is fundamentally a difficult, frustrating and
> depressing activity.
> """
> 
> But really this comment doesn't make it much better. Yes I've made
> a mistake although I _read_ the function documentation. So shame on
> me.

It wasn't aimed at you - it was more pointing out that in the normal
process of kernel development, reading documentation is fairly low,
yet we spend time creating it. So, does writing documentation actually
help, or does it just slow down the development cycle? Does it have a
net positive value? Personally, I don't think it does.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!