lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next v2 5/5] net: mediatek: sgmii: refactor power cycling into mtk_pcs_config()
> > -	/* PHYA power down */
> > - regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL,
> > SGMII_PHYA_PWD); -
> > /* Set SGMII phy speed */
> > regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val);
> > val &= ~RG_PHY_SPEED_MASK;
> > @@ -72,9 +57,6 @@ static int mtk_pcs_setup_mode_force(struct
> > mtk_pcs *mpcs, {
> > unsigned int val;
> >
> > - /* PHYA power down */
> > - regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL,
> > SGMII_PHYA_PWD); -
> > regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val);
> > val &= ~RG_PHY_SPEED_MASK;
> > if (interface == PHY_INTERFACE_MODE_2500BASEX)
>
> After powering the PHY down, the next thing that is done is to
> configure the speed. Even with my comments on patch 4, this can still
> be consolidated.

I'll move more code out of the functions.

>
> > @@ -115,12 +85,27 @@ static int mtk_pcs_config(struct phylink_pcs
> > *pcs, unsigned int mode, struct mtk_pcs *mpcs =
> > pcs_to_mtk_pcs(pcs);
>
> unsigned int val;
>
> > int err = 0;
> >
> > + /* PHYA power down */
> > + regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL,
> > SGMII_PHYA_PWD);
> > +
>
> regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val);
> val &= ~RG_PHY_SPEED_MASK;
> if (interface == PHY_INTERFACE_MODE_2500BASEX)
> val |= RG_PHY_SPEED_3_125G;
> regmap_write(mpcs->regmap, mpcs->ana_rgc3, val);
>
> which would make logical sense to do here, so we always configure the
> speed for the PCS correctly.
>
> That then leaves the configuration of SGMSYS_PCS_CONTROL_1 and
> SGMSYS_SGMII_MODE, which I think could also be consolidated, but I'll
> leave that to those with the hardware to make that decision.
>
> Reading between the lines of the code in this driver, it looks to me
> like this hardware supports only SGMII, but doesn't actually support
> 1000base-X and 2500base-X with negotiation. Is that correct? If so,
> it would be good to add a mtk_pcs_validate() function that clears
> ETHTOOL_LINK_MODE_Autoneg_BIT for these modes.

I don't know. I don't have hardware to debug
the serdes interface further. I only have a test board with mt7622 soc
connect via SGMII/2500basex to a realtek phy (rtl8221).

Maybe the maintainers from mediatek could share some knowledge if the
SGMII block supports 1000/2500basex autoneg?

At least with the public available datasheets (mt7531, mt7622) doesn't
explain it further.
I could also imagine we need to modify the page register
(PCS_SPEED_ABILITY) and link timer to get autoneg for
1000basex/2500basex working.

Best,
lynxis

\
 
 \ /
  Last update: 2022-09-19 15:59    [W:0.044 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site