[V2,1/3] net: phy: DP83822: Clean up config_init code

Message ID 20220522134146.120312-1-michael@amarulasolutions.com
State New
Headers show
Series
  • [V2,1/3] net: phy: DP83822: Clean up config_init code
Related show

Commit Message

Michael Trimarchi May 22, 2022, 1:41 p.m. UTC
Rename some variables and reduce a bit code complexity. No
functional changes

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/net/phy/dp83822.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

Comments

Tommaso Merciai May 23, 2022, 6:52 a.m. UTC | #1
On Sun, May 22, 2022 at 03:41:44PM +0200, Michael Trimarchi wrote:
> Rename some variables and reduce a bit code complexity. No
> functional changes
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/net/phy/dp83822.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index ce17b2af3218..c344b8ffaf3c 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -380,31 +380,26 @@ static int dp83822_config_init(struct phy_device *phydev)
>  {
>  	struct dp83822_private *dp83822 = phydev->priv;
>  	struct device *dev = &phydev->mdio.dev;
> -	int rgmii_delay;
> -	s32 rx_int_delay;
> -	s32 tx_int_delay;
> +	int value = 0;

Use "val" instead of "value" name variable, for naming convention.

> +	s32 trx_int_delay;
>  	int err = 0;
>  	int bmcr;
>  
>  	if (phy_interface_is_rgmii(phydev)) {
> -		rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> +		trx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
>  						      true);
>  
> -		if (rx_int_delay <= 0)
> -			rgmii_delay = 0;
> -		else
> -			rgmii_delay = DP83822_RX_CLK_SHIFT;
> +		if (trx_int_delay > 0)
> +			value = DP83822_RX_CLK_SHIFT;
>  
> -		tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> +		trx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
>  						      false);
> -		if (tx_int_delay <= 0)
> -			rgmii_delay &= ~DP83822_TX_CLK_SHIFT;
> -		else
> -			rgmii_delay |= DP83822_TX_CLK_SHIFT;
> +		if (trx_int_delay > 0)
> +			value |= DP83822_TX_CLK_SHIFT;
>  
> -		if (rgmii_delay) {
> +		if (value) {
>  			err = phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> -					       MII_DP83822_RCSR, rgmii_delay);
> +					       MII_DP83822_RCSR, value);
>  			if (err)
>  				return err;
>  		}
> -- 
> 2.25.1
>
Michael Trimarchi May 23, 2022, 7:05 a.m. UTC | #2
Hi

Il lun 23 mag 2022, 08:52 Tommaso Merciai <
tommaso.merciai@amarulasolutions.com> ha scritto:

> On Sun, May 22, 2022 at 03:41:44PM +0200, Michael Trimarchi wrote:
> > Rename some variables and reduce a bit code complexity. No
> > functional changes
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> >  drivers/net/phy/dp83822.c | 25 ++++++++++---------------
> >  1 file changed, 10 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index ce17b2af3218..c344b8ffaf3c 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -380,31 +380,26 @@ static int dp83822_config_init(struct phy_device
> *phydev)
> >  {
> >       struct dp83822_private *dp83822 = phydev->priv;
> >       struct device *dev = &phydev->mdio.dev;
> > -     int rgmii_delay;
> > -     s32 rx_int_delay;
> > -     s32 tx_int_delay;
> > +     int value = 0;
>
> Use "val" instead of "value" name variable, for naming convention.
>

Is it val already uses in this file?

Anyway I can change

Michael

>
> > +     s32 trx_int_delay;
> >       int err = 0;
> >       int bmcr;
> >
> >       if (phy_interface_is_rgmii(phydev)) {
> > -             rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> > +             trx_int_delay = phy_get_internal_delay(phydev, dev, NULL,
> 0,
> >                                                     true);
> >
> > -             if (rx_int_delay <= 0)
> > -                     rgmii_delay = 0;
> > -             else
> > -                     rgmii_delay = DP83822_RX_CLK_SHIFT;
> > +             if (trx_int_delay > 0)
> > +                     value = DP83822_RX_CLK_SHIFT;
> >
> > -             tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> > +             trx_int_delay = phy_get_internal_delay(phydev, dev, NULL,
> 0,
> >                                                     false);
> > -             if (tx_int_delay <= 0)
> > -                     rgmii_delay &= ~DP83822_TX_CLK_SHIFT;
> > -             else
> > -                     rgmii_delay |= DP83822_TX_CLK_SHIFT;
> > +             if (trx_int_delay > 0)
> > +                     value |= DP83822_TX_CLK_SHIFT;
> >
> > -             if (rgmii_delay) {
> > +             if (value) {
> >                       err = phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > -                                            MII_DP83822_RCSR,
> rgmii_delay);
> > +                                            MII_DP83822_RCSR, value);
> >                       if (err)
> >                               return err;
> >               }
> > --
> > 2.25.1
> >
>
> --
> Tommaso Merciai
> Embedded Linux Engineer
> tommaso.merciai@amarulasolutions.com
> __________________________________
>
> Amarula Solutions SRL
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
> T. +39 042 243 5310
> info@amarulasolutions.com
> www.amarulasolutions.com
>
Tommaso Merciai May 23, 2022, 7:14 a.m. UTC | #3
On Mon, May 23, 2022 at 09:05:38AM +0200, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Il lun 23 mag 2022, 08:52 Tommaso Merciai <
> tommaso.merciai@amarulasolutions.com> ha scritto:
> 
> > On Sun, May 22, 2022 at 03:41:44PM +0200, Michael Trimarchi wrote:
> > > Rename some variables and reduce a bit code complexity. No
> > > functional changes
> > >
> > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > ---
> > >  drivers/net/phy/dp83822.c | 25 ++++++++++---------------
> > >  1 file changed, 10 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > > index ce17b2af3218..c344b8ffaf3c 100644
> > > --- a/drivers/net/phy/dp83822.c
> > > +++ b/drivers/net/phy/dp83822.c
> > > @@ -380,31 +380,26 @@ static int dp83822_config_init(struct phy_device
> > *phydev)
> > >  {
> > >       struct dp83822_private *dp83822 = phydev->priv;
> > >       struct device *dev = &phydev->mdio.dev;
> > > -     int rgmii_delay;
> > > -     s32 rx_int_delay;
> > > -     s32 tx_int_delay;
> > > +     int value = 0;
> >
> > Use "val" instead of "value" name variable, for naming convention.
> >
> 
> Is it val already uses in this file?
> 
> Anyway I can change

Hi,
You are right it's ok "value". Is the name use in almost all the driver.

Tommaso

> 
> Michael
> 
> >
> > > +     s32 trx_int_delay;
> > >       int err = 0;
> > >       int bmcr;
> > >
> > >       if (phy_interface_is_rgmii(phydev)) {
> > > -             rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> > > +             trx_int_delay = phy_get_internal_delay(phydev, dev, NULL,
> > 0,
> > >                                                     true);
> > >
> > > -             if (rx_int_delay <= 0)
> > > -                     rgmii_delay = 0;
> > > -             else
> > > -                     rgmii_delay = DP83822_RX_CLK_SHIFT;
> > > +             if (trx_int_delay > 0)
> > > +                     value = DP83822_RX_CLK_SHIFT;
> > >
> > > -             tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
> > > +             trx_int_delay = phy_get_internal_delay(phydev, dev, NULL,
> > 0,
> > >                                                     false);
> > > -             if (tx_int_delay <= 0)
> > > -                     rgmii_delay &= ~DP83822_TX_CLK_SHIFT;
> > > -             else
> > > -                     rgmii_delay |= DP83822_TX_CLK_SHIFT;
> > > +             if (trx_int_delay > 0)
> > > +                     value |= DP83822_TX_CLK_SHIFT;
> > >
> > > -             if (rgmii_delay) {
> > > +             if (value) {
> > >                       err = phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > > -                                            MII_DP83822_RCSR,
> > rgmii_delay);
> > > +                                            MII_DP83822_RCSR, value);
> > >                       if (err)
> > >                               return err;
> > >               }
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Tommaso Merciai
> > Embedded Linux Engineer
> > tommaso.merciai@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions SRL
> > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> > T. +39 042 243 5310
> > info@amarulasolutions.com
> > www.amarulasolutions.com
> >

Patch

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index ce17b2af3218..c344b8ffaf3c 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -380,31 +380,26 @@  static int dp83822_config_init(struct phy_device *phydev)
 {
 	struct dp83822_private *dp83822 = phydev->priv;
 	struct device *dev = &phydev->mdio.dev;
-	int rgmii_delay;
-	s32 rx_int_delay;
-	s32 tx_int_delay;
+	int value = 0;
+	s32 trx_int_delay;
 	int err = 0;
 	int bmcr;
 
 	if (phy_interface_is_rgmii(phydev)) {
-		rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
+		trx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
 						      true);
 
-		if (rx_int_delay <= 0)
-			rgmii_delay = 0;
-		else
-			rgmii_delay = DP83822_RX_CLK_SHIFT;
+		if (trx_int_delay > 0)
+			value = DP83822_RX_CLK_SHIFT;
 
-		tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
+		trx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
 						      false);
-		if (tx_int_delay <= 0)
-			rgmii_delay &= ~DP83822_TX_CLK_SHIFT;
-		else
-			rgmii_delay |= DP83822_TX_CLK_SHIFT;
+		if (trx_int_delay > 0)
+			value |= DP83822_TX_CLK_SHIFT;
 
-		if (rgmii_delay) {
+		if (value) {
 			err = phy_set_bits_mmd(phydev, DP83822_DEVADDR,
-					       MII_DP83822_RCSR, rgmii_delay);
+					       MII_DP83822_RCSR, value);
 			if (err)
 				return err;
 		}