linux-kernel - Re: [PATCH net-next v2 5/5] net: mediatek: sgmii: refactor power cycling into mtk_pcs

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] [day] [month] [year] [list]

Date:   Mon, 19 Sep 2022 15:56:06 +0200
From:   Alexander 'lynxis' Couzens <lynxis@...0.eu>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
        Sean Wang <sean.wang@...iatek.com>,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [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