Re: [PATCH v2 net-next 02/10] net: phy: bcm7xx: Add jumbo frame configuration to PHY

[Heiner Kallweit <hkallweit1@gmail.com>]

On 25.03.2020 16:22, Vladimir Oltean wrote:
> From: Murali Krishna Policharla <murali.policharla@broadcom.com>
> 
> Add API to configure jumbo frame settings in PHY during initial PHY
> configuration.
> 
> Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/phy/bcm-phy-lib.c | 28 ++++++++++++++++++++++++++++
>  drivers/net/phy/bcm-phy-lib.h |  1 +
>  drivers/net/phy/bcm7xxx.c     |  4 ++++
>  include/linux/brcmphy.h       |  1 +
>  4 files changed, 34 insertions(+)
> 
> diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
> index e0d3310957ff..a26c80e13b43 100644
> --- a/drivers/net/phy/bcm-phy-lib.c
> +++ b/drivers/net/phy/bcm-phy-lib.c
> @@ -423,6 +423,34 @@ int bcm_phy_28nm_a0b0_afe_config_init(struct phy_device *phydev)
>  }
>  EXPORT_SYMBOL_GPL(bcm_phy_28nm_a0b0_afe_config_init);
>  
> +int bcm_phy_enable_jumbo(struct phy_device *phydev)
> +{
> +	int val = 0, ret = 0;
> +
> +	ret = phy_write(phydev, MII_BCM54XX_AUX_CTL,
> +			MII_BCM54XX_AUXCTL_SHDWSEL_MISC);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = phy_read(phydev, MII_BCM54XX_AUX_CTL);
> +
> +	/* Enable extended length packet reception */
> +	val |= MII_BCM54XX_AUXCTL_ACTL_EXT_PKT_LEN;
> +	ret = phy_write(phydev, MII_BCM54XX_AUX_CTL, val);
> +

There are different helpers already in bcm-phy-lib,
e.g. bcm54xx_auxctl_read. Also bcm_phy_write_misc()
has has quite something in common with your new function.
It would be good if a helper could be used here.

> +	if (ret < 0)
> +		return ret;
> +
> +	val = phy_read(phydev, MII_BCM54XX_ECR);
> +
> +	/* Enable 10K byte packet length reception */
> +	val |= BIT(0);
> +	ret =  phy_write(phydev, MII_BCM54XX_ECR, val);
> +

Why not use phy_set_bits() ?

> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(bcm_phy_enable_jumbo);
> +
>  MODULE_DESCRIPTION("Broadcom PHY Library");
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Broadcom Corporation");
> diff --git a/drivers/net/phy/bcm-phy-lib.h b/drivers/net/phy/bcm-phy-lib.h
> index c86fb9d1240c..129df819be8c 100644
> --- a/drivers/net/phy/bcm-phy-lib.h
> +++ b/drivers/net/phy/bcm-phy-lib.h
> @@ -65,5 +65,6 @@ void bcm_phy_get_stats(struct phy_device *phydev, u64 *shadow,
>  		       struct ethtool_stats *stats, u64 *data);
>  void bcm_phy_r_rc_cal_reset(struct phy_device *phydev);
>  int bcm_phy_28nm_a0b0_afe_config_init(struct phy_device *phydev);
> +int bcm_phy_enable_jumbo(struct phy_device *phydev);
>  
>  #endif /* _LINUX_BCM_PHY_LIB_H */
> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
> index af8eabe7a6d4..692048d86ab1 100644
> --- a/drivers/net/phy/bcm7xxx.c
> +++ b/drivers/net/phy/bcm7xxx.c
> @@ -178,6 +178,10 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
>  		break;
>  	}
>  
> +	if (ret)
> +		return ret;
> +
> +	ret =  bcm_phy_enable_jumbo(phydev);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index b475e7f20d28..19bd86019e93 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -119,6 +119,7 @@
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL	0x00
>  #define MII_BCM54XX_AUXCTL_ACTL_TX_6DB		0x0400
>  #define MII_BCM54XX_AUXCTL_ACTL_SMDSP_ENA	0x0800
> +#define MII_BCM54XX_AUXCTL_ACTL_EXT_PKT_LEN	0x4000
>  
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC			0x07
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN	0x0010
>