Messages in this thread | | | From | Vladimir Oltean <> | Date | Fri, 9 Aug 2019 14:09:02 +0300 | Subject | Re: [PATCH] net: phy: at803x: stop switching phy delay config needlessly |
| |
Hi Andre,
On Fri, 9 Aug 2019 at 13:00, André Draszik <git@andred.net> wrote: > > Hi Vladimir, > > On Fri, 2019-08-09 at 12:43 +0300, Vladimir Oltean wrote: > > Hi Andre, > > > > On Fri, 9 Aug 2019 at 03:58, André Draszik <git@andred.net> wrote: > > > This driver does a funny dance disabling and re-enabling > > > RX and/or TX delays. In any of the RGMII-ID modes, it first > > > disables the delays, just to re-enable them again right > > > away. This looks like a needless exercise. > > > > > > Just enable the respective delays when in any of the > > > relevant 'id' modes, and disable them otherwise. > > > > > > Also, remove comments which don't add anything that can't be > > > seen by looking at the code. > > > > > > Signed-off-by: André Draszik <git@andred.net> > > > CC: Andrew Lunn <andrew@lunn.ch> > > > CC: Florian Fainelli <f.fainelli@gmail.com> > > > CC: Heiner Kallweit <hkallweit1@gmail.com> > > > CC: "David S. Miller" <davem@davemloft.net> > > > CC: netdev@vger.kernel.org > > > --- > > > > Is there any particular problem you're facing? Does this make any difference? > > This is a clean-up, reducing the number of lines and if statements > by removing unnecessary code paths and comments. >
Ok. Did checkpatch not complain about the braces which you left open around a single line?
> > Cheers, > Andre' > > > > > > > drivers/net/phy/at803x.c | 26 ++++++-------------------- > > > 1 file changed, 6 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c > > > index 222ccd9ecfce..2ab51f552e92 100644 > > > --- a/drivers/net/phy/at803x.c > > > +++ b/drivers/net/phy/at803x.c > > > @@ -257,35 +257,21 @@ static int at803x_config_init(struct phy_device *phydev) > > > * after HW reset: RX delay enabled and TX delay disabled > > > * after SW reset: RX delay enabled, while TX delay retains the > > > * value before reset. > > > - * > > > - * So let's first disable the RX and TX delays in PHY and enable > > > - * them based on the mode selected (this also takes care of RGMII > > > - * mode where we expect delays to be disabled) > > > */ > > > - > > > - ret = at803x_disable_rx_delay(phydev); > > > - if (ret < 0) > > > - return ret; > > > - ret = at803x_disable_tx_delay(phydev); > > > - if (ret < 0) > > > - return ret; > > > - > > > if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > > > phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) { > > > - /* If RGMII_ID or RGMII_RXID are specified enable RX delay, > > > - * otherwise keep it disabled > > > - */ > > > ret = at803x_enable_rx_delay(phydev); > > > - if (ret < 0) > > > - return ret; > > > + } else { > > > + ret = at803x_disable_rx_delay(phydev); > > > } > > > + if (ret < 0) > > > + return ret; > > > > > > if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > > > phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) { > > > - /* If RGMII_ID or RGMII_TXID are specified enable TX delay, > > > - * otherwise keep it disabled > > > - */ > > > ret = at803x_enable_tx_delay(phydev); > > > + } else { > > > + ret = at803x_disable_tx_delay(phydev); > > > } > > > > > > return ret; > > > -- > > > 2.20.1 > > > > > > > Regards, > > -Vladimir >
Thanks, -Vladimir
| |