netdev - Re: [PATCH v1] net: phy: adin: Add flags to disable enhanced link detection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f5920799e6b0b6b5321ca38eb3b28024dc1be81f.camel@gmail.com>
Date:   Tue, 28 Feb 2023 16:09:44 +0100
From:   Nuno Sá <noname.nuno@...il.com>
To:     Ken Sloat <ken.s@...iscite.com>
Cc:     Michael Hennerich <michael.hennerich@...log.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] net: phy: adin: Add flags to disable enhanced link
 detection

Hi,

Thanks for the patch! Some comments from my side...

On Tue, 2023-02-28 at 09:40 -0500, Ken Sloat wrote:
> Enhanced link detection is an ADI PHY feature that allows for earlier
> detection of link down if certain signal conditions are met. This
> feature is for the most part enabled by default on the PHY. This is
> not suitable for all applications and breaks the IEEE standard as
> explained in the ADI datasheet.
> 
> To fix this, add override flags to disable enhanced link detection
> for 1000BASE-T and 100BASE-TX respectively by clearing any related
> feature enable bits.
> 
> This new feature was tested on an ADIN1300 but according to the
> datasheet applies equally for 100BASE-TX on the ADIN1200.
> 
> Signed-off-by: Ken Sloat <ken.s@...iscite.com>
> ---
>  drivers/net/phy/adin.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> index da65215d19bb..8809f3e036a4 100644
> --- a/drivers/net/phy/adin.c
> +++ b/drivers/net/phy/adin.c
> @@ -69,6 +69,15 @@
>  #define ADIN1300_EEE_CAP_REG                   0x8000
>  #define ADIN1300_EEE_ADV_REG                   0x8001
>  #define ADIN1300_EEE_LPABLE_REG                        0x8002
> +#define ADIN1300_FLD_EN_REG                            0x8E27
> +#define   ADIN1300_FLD_PCS_ERR_100_EN                  BIT(7)
> +#define   ADIN1300_FLD_PCS_ERR_1000_EN                 BIT(6)
> +#define   ADIN1300_FLD_SLCR_OUT_STUCK_100_EN   BIT(5)
> +#define   ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN  BIT(4)
> +#define   ADIN1300_FLD_SLCR_IN_ZDET_100_EN             BIT(3)
> +#define   ADIN1300_FLD_SLCR_IN_ZDET_1000_EN            BIT(2)
> +#define   ADIN1300_FLD_SLCR_IN_INVLD_100_EN            BIT(1)
> +#define   ADIN1300_FLD_SLCR_IN_INVLD_1000_EN   BIT(0)
>  #define ADIN1300_CLOCK_STOP_REG                        0x9400
>  #define ADIN1300_LPI_WAKE_ERR_CNT_REG          0xa000
>  
> @@ -508,6 +517,31 @@ static int adin_config_clk_out(struct phy_device
> *phydev)
>                               ADIN1300_GE_CLK_CFG_MASK, sel);
>  }
>  
> +static int adin_config_fld_en(struct phy_device *phydev)
> +{
> +       struct device *dev = &phydev->mdio.dev;
> +       int reg;
> +
> +       reg = phy_read_mmd(phydev, MDIO_MMD_VEND1,
> ADIN1300_FLD_EN_REG);
> +       if (reg < 0)
> +               return reg;
> +
> +       if (device_property_read_bool(dev, "adi,disable-fld-1000base-
> t"))

"adi,disable-fld-1000base-tx"?

> +               reg &= ~(ADIN1300_FLD_PCS_ERR_1000_EN |
> +                                ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN
> |
> +                                ADIN1300_FLD_SLCR_IN_ZDET_1000_EN |
> +                                ADIN1300_FLD_SLCR_IN_INVLD_1000_EN);
> +
> +       if (device_property_read_bool(dev, "adi,disable-fld-100base-
> tx"))
> +               reg &= ~(ADIN1300_FLD_PCS_ERR_100_EN |
> +                                ADIN1300_FLD_SLCR_OUT_STUCK_100_EN |
> +                                ADIN1300_FLD_SLCR_IN_ZDET_100_EN |
> +                                ADIN1300_FLD_SLCR_IN_INVLD_100_EN);
> +
> +       return phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +                            ADIN1300_FLD_EN_REG, reg);

nit: You could use phy_clear_bits_mmd() to simplify the function a bit.


You also need to add these new properties to:

Documentation/devicetree/bindings/net/adi,adin.yaml


- Nuno Sá