Re: [PATCH net-next 1/1] Added BASE-T1 PHY support to PHY Subsystem

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote:
> BASE-T1 is a category of Ethernet PHYs.
> They use a single copper pair for transmission.
> This patch add basic support for this category of PHYs.
> It coveres the discovery of abilities and basic configuration.
> It includes setting fixed speed and enabling auto-negotiation.
> BASE-T1 devices should always Clause-45 managed.
> Therefore, this patch extends phy-c45.c.
> While for some functions like auto-neogtiation different registers are
> used, the layout of these registers is the same for the used fields.
> Thus, much of the logic of basic Clause-45 devices can be reused.
> 
> Signed-off-by: Christian Herber <christian.herber@nxp.com>
> ---
>  drivers/net/phy/phy-c45.c    | 113 +++++++++++++++++++++++++++++++----
>  drivers/net/phy/phy-core.c   |   4 +-
>  include/uapi/linux/ethtool.h |   2 +
>  include/uapi/linux/mdio.h    |  21 +++++++
>  4 files changed, 129 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index b9d4145781ca..9ff0b8c785de 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -8,13 +8,23 @@
>  #include <linux/mii.h>
>  #include <linux/phy.h>
>  
> +#define IS_100BASET1(phy) (linkmode_test_bit( \
> +			   ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \
> +			   (phy)->supported))
> +#define IS_1000BASET1(phy) (linkmode_test_bit( \
> +			    ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \
> +			    (phy)->supported))

Hi Christian

We already have the flag phydev->is_gigabit_capable. Maybe add a flag
phydev->is_t1_capable

> +
> +static u32 get_aneg_ctrl(struct phy_device *phydev);
> +static u32 get_aneg_stat(struct phy_device *phydev);

No forward declarations please. Put the code in the right order so
they are not needed.

Thanks

     Andrew