[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240906064358.xmqbs7fzf6kd7ydp@DEN-DL-M31836.microchip.com>
Date: Fri, 6 Sep 2024 08:43:58 +0200
From: Horatiu Vultur - M31836 <Horatiu.Vultur@...rochip.com>
To: Parthiban Veerasooran - I17164 <Parthiban.Veerasooran@...rochip.com>
CC: "andrew@...n.ch" <andrew@...n.ch>, "hkallweit1@...il.com"
<hkallweit1@...il.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>, "ramon.nordin.rodriguez@...roamp.se"
<ramon.nordin.rodriguez@...roamp.se>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, UNGLinuxDriver
<UNGLinuxDriver@...rochip.com>, Thorsten Kummermehr - M21127
<Thorsten.Kummermehr@...rochip.com>
Subject: Re: [PATCH net-next v2 7/7] net: phy: microchip_t1s: configure
collision detection based on PLCA mode
The 09/04/2024 11:46, Parthiban Veerasooran - I17164 wrote:
> Hi Horatiu,
>
> On 03/09/24 12:13 pm, Horatiu Vultur wrote:
> > The 09/02/2024 20:04, Parthiban Veerasooran wrote:
> >
> > Hi Parthiban,
> >
> >> As per LAN8650/1 Rev.B0/B1 AN1760 (Revision F (DS60001760G - June 2024))
> >> and LAN8670/1/2 Rev.C1/C2 AN1699 (Revision E (DS60001699F - June 2024)),
> >> under normal operation, the device should be operated in PLCA mode.
> >> Disabling collision detection is recommended to allow the device to
> >> operate in noisy environments or when reflections and other inherent
> >> transmission line distortion cause poor signal quality. Collision
> >> detection must be re-enabled if the device is configured to operate in
> >> CSMA/CD mode.
> >
> > Is this something that applies only to Microchip PHYs or is something
> > generic that applies to all the PHYs.
> Yes, the behavior is common for all the PHYs but it is purely up to the
> PHY chip design specific.
>
> 1. Some vendors will enable this feature in the chip level by latching
> the register bits. There we don't need software interface.
> 2. Some vendors need software interface to enable this feature like our
> Microchip PHY does.
Don't you think then is better to create something more generic so other
PHYs to able to do this?
>
> Best regards,
> Parthiban V
> >
> >>
> >> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@...rochip.com>
> >> ---
> >> drivers/net/phy/microchip_t1s.c | 42 ++++++++++++++++++++++++++++++---
> >> 1 file changed, 39 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
> >> index bd0c768df0af..a0565508d7d2 100644
> >> --- a/drivers/net/phy/microchip_t1s.c
> >> +++ b/drivers/net/phy/microchip_t1s.c
> >> @@ -26,6 +26,12 @@
> >> #define LAN865X_REG_CFGPARAM_CTRL 0x00DA
> >> #define LAN865X_REG_STS2 0x0019
> >>
> >> +/* Collision Detector Control 0 Register */
> >> +#define LAN86XX_REG_COL_DET_CTRL0 0x0087
> >> +#define COL_DET_CTRL0_ENABLE_BIT_MASK BIT(15)
> >> +#define COL_DET_ENABLE BIT(15)
> >> +#define COL_DET_DISABLE 0x0000
> >> +
> >> #define LAN865X_CFGPARAM_READ_ENABLE BIT(1)
> >>
> >> /* The arrays below are pulled from the following table from AN1699
> >> @@ -370,6 +376,36 @@ static int lan867x_revb1_config_init(struct phy_device *phydev)
> >> return 0;
> >> }
> >>
> >> +/* As per LAN8650/1 Rev.B0/B1 AN1760 (Revision F (DS60001760G - June 2024)) and
> >> + * LAN8670/1/2 Rev.C1/C2 AN1699 (Revision E (DS60001699F - June 2024)), under
> >> + * normal operation, the device should be operated in PLCA mode. Disabling
> >> + * collision detection is recommended to allow the device to operate in noisy
> >> + * environments or when reflections and other inherent transmission line
> >> + * distortion cause poor signal quality. Collision detection must be re-enabled
> >> + * if the device is configured to operate in CSMA/CD mode.
> >> + *
> >> + * AN1760: https://www.microchip.com/en-us/application-notes/an1760
> >> + * AN1699: https://www.microchip.com/en-us/application-notes/an1699
> >> + */
> >> +static int lan86xx_plca_set_cfg(struct phy_device *phydev,
> >> + const struct phy_plca_cfg *plca_cfg)
> >> +{
> >> + int ret;
> >> +
> >> + ret = genphy_c45_plca_set_cfg(phydev, plca_cfg);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (plca_cfg->enabled)
> >> + return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
> >> + LAN86XX_REG_COL_DET_CTRL0,
> >> + COL_DET_CTRL0_ENABLE_BIT_MASK,
> >> + COL_DET_DISABLE);
> >> +
> >> + return phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
> >> + COL_DET_CTRL0_ENABLE_BIT_MASK, COL_DET_ENABLE);
> >> +}
> >> +
> >> static int lan86xx_read_status(struct phy_device *phydev)
> >> {
> >> /* The phy has some limitations, namely:
> >> @@ -403,7 +439,7 @@ static struct phy_driver microchip_t1s_driver[] = {
> >> .config_init = lan867x_revc_config_init,
> >> .read_status = lan86xx_read_status,
> >> .get_plca_cfg = genphy_c45_plca_get_cfg,
> >> - .set_plca_cfg = genphy_c45_plca_set_cfg,
> >> + .set_plca_cfg = lan86xx_plca_set_cfg,
> >> .get_plca_status = genphy_c45_plca_get_status,
> >> },
> >> {
> >> @@ -413,7 +449,7 @@ static struct phy_driver microchip_t1s_driver[] = {
> >> .config_init = lan867x_revc_config_init,
> >> .read_status = lan86xx_read_status,
> >> .get_plca_cfg = genphy_c45_plca_get_cfg,
> >> - .set_plca_cfg = genphy_c45_plca_set_cfg,
> >> + .set_plca_cfg = lan86xx_plca_set_cfg,
> >> .get_plca_status = genphy_c45_plca_get_status,
> >> },
> >> {
> >> @@ -423,7 +459,7 @@ static struct phy_driver microchip_t1s_driver[] = {
> >> .config_init = lan865x_revb_config_init,
> >> .read_status = lan86xx_read_status,
> >> .get_plca_cfg = genphy_c45_plca_get_cfg,
> >> - .set_plca_cfg = genphy_c45_plca_set_cfg,
> >> + .set_plca_cfg = lan86xx_plca_set_cfg,
> >> .get_plca_status = genphy_c45_plca_get_status,
> >> },
> >> };
> >> --
> >> 2.34.1
> >>
> >
>
--
/Horatiu
Powered by blists - more mailing lists