[net-next,1/7] net: phy: marvell10g: Use get_features to get the PHY abilities

The Alaska family of 10G PHYs has more abilities than the ones listed in
PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.

Make use of the newly introduced .get_features call to build this list,
using genphy_c45_pma_read_abilities to build the list of supported
linkmodes, and adding autoneg ability based on what's reported by the AN
MMD.

.config_init is still used to validate the interface_mode.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/marvell10g.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

On Thu, Feb 21, 2019 at 10:51:22AM +0100, Maxime Chevallier wrote:
> The Alaska family of 10G PHYs has more abilities than the ones listed in
> PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.
> 
> Make use of the newly introduced .get_features call to build this list,
> using genphy_c45_pma_read_abilities to build the list of supported
> linkmodes, and adding autoneg ability based on what's reported by the AN
> MMD.
> 
> .config_init is still used to validate the interface_mode.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  drivers/net/phy/marvell10g.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 9ea27acf05ad..65ef469adf58 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev)
>  
>  static int mv3310_config_init(struct phy_device *phydev)
>  {
> -	int ret, val;
> -
>  	/* Check that the PHY interface type is compatible */
>  	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
>  	    phydev->interface != PHY_INTERFACE_MODE_XAUI &&
> @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev)
>  	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
>  		return -ENODEV;
>  
> +	return 0;
> +}
> +
> +static int mv3310_get_features(struct phy_device *phydev)
> +{
> +	int ret, val;

Please try to keep the formatting/style consistent in the file you are
editing.  A blank line here would do that.  Thanks.

>  	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
>  		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
>  		if (val < 0)
> @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = {
>  		.phy_id		= 0x002b09aa,
>  		.phy_id_mask	= MARVELL_PHY_ID_MASK,
>  		.name		= "mv88x3310",
> -		.features	= PHY_10GBIT_FEATURES,
> +		.get_features	= mv3310_get_features,
>  		.soft_reset	= gen10g_no_soft_reset,
>  		.config_init	= mv3310_config_init,
>  		.probe		= mv3310_probe,
> -- 
> 2.19.2
> 
>

Hello Russell,

On Thu, 21 Feb 2019 10:22:15 +0000
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:

>> +	return 0;
>> +}
>> +
>> +static int mv3310_get_features(struct phy_device *phydev)
>> +{
>> +	int ret, val;  
>
>Please try to keep the formatting/style consistent in the file you are
>editing.  A blank line here would do that.  Thanks.

Sorry, I missed that (and so did checkpatch apparently). I'll send a V2.

Thanks,

Maxime

On 21.02.2019 10:51, Maxime Chevallier wrote:
> The Alaska family of 10G PHYs has more abilities than the ones listed in
> PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.
> 
> Make use of the newly introduced .get_features call to build this list,
> using genphy_c45_pma_read_abilities to build the list of supported
> linkmodes, and adding autoneg ability based on what's reported by the AN
> MMD.
> 
> .config_init is still used to validate the interface_mode.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  drivers/net/phy/marvell10g.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 9ea27acf05ad..65ef469adf58 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev)
>  
>  static int mv3310_config_init(struct phy_device *phydev)
>  {
> -	int ret, val;
> -
>  	/* Check that the PHY interface type is compatible */
>  	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
>  	    phydev->interface != PHY_INTERFACE_MODE_XAUI &&
> @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev)
>  	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
>  		return -ENODEV;
>  
> +	return 0;
> +}
> +
> +static int mv3310_get_features(struct phy_device *phydev)
> +{

After my just submitted patch to include the aneg capability checking in
genphy_c45_pma_read_abilities() function mv3310_get_features() isn't
needed any longer and can be replaced with the generic one.
But we can make this change afterwards, then you don't have to
rework your series.

Also I'm not sure whether there will be a 5.0-rc8 or whether beginning
of next week we'll see 5.0. In the latter case we're a little bit in a
hurry because the merge window will start very soon.

> +	int ret, val;
>  	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
>  		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
>  		if (val < 0)
> @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = {
>  		.phy_id		= 0x002b09aa,
>  		.phy_id_mask	= MARVELL_PHY_ID_MASK,
>  		.name		= "mv88x3310",
> -		.features	= PHY_10GBIT_FEATURES,
> +		.get_features	= mv3310_get_features,
>  		.soft_reset	= gen10g_no_soft_reset,
>  		.config_init	= mv3310_config_init,
>  		.probe		= mv3310_probe,
>

On Fri, 22 Feb 2019 19:42:29 +0100
Heiner Kallweit <hkallweit1@gmail.com> wrote:

>After my just submitted patch to include the aneg capability checking in
>genphy_c45_pma_read_abilities() function mv3310_get_features() isn't
>needed any longer and can be replaced with the generic one.

I'll still need it to handle the 2.5G/5G abilities that aren't
correctly reported on 3310. I'll be able to use the generic
function for the 2110 though, which is nice.

>But we can make this change afterwards, then you don't have to
>rework your series.
>
>Also I'm not sure whether there will be a 5.0-rc8 or whether beginning
>of next week we'll see 5.0. In the latter case we're a little bit in a
>hurry because the merge window will start very soon.

OK, I'll re-spin the series quickly with the small cleanup needed if
everything's OK to you regarding the rest of it.

Thanks,

Maxime