[RFC PATCH 1/1] net: xilinx: handle internal PHY/PCS

[Nikita Shubin]

Hello Harini!

Sorry for long response time.

On Wed, 24 Aug 2022 12:51:08 +0000
"Katakam, Harini" <harini.katakam at amd.com> wrote:


> Hi Nikita,
> 
> > -----Original Message-----
> > From: Simek, Michal <michal.simek at amd.com>
> > Sent: Thursday, August 11, 2022 1:16 PM
> > To: Nikita Shubin <nikita.shubin at maquefel.me>; Agarwal, Swati
> > <swati.agarwal at amd.com>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey at amd.com>
> > Cc: linux at yadro.com; Nikita Shubin <n.shubin at yadro.com>; Joe
> > Hershberger <joe.hershberger at ni.com>; Ramon Fried
> > <rfried.dev at gmail.com>; u-boot at lists.denx.de
> > Subject: Re: [RFC PATCH 1/1] net: xilinx: handle internal PHY/PCS
> > 
> > Hi Nikita,
> > 
> > On 8/2/22 12:53, Nikita Shubin wrote:  
> > > From: Nikita Shubin <n.shubin at yadro.com>
> > >
> > > In SGMII/1000BaseX Xilinx AXI Ethernet may also have an Internal
> > > PHY (PCS) in addition to external PHY, in that case we should
> > > also set at least BMCR_ANENABLE.
> > >
> > > PCS are not visible until axinet bringup, so init should be done
> > > after, controller is brought up, then we should poll
> > > BMSR_ANEGCOMPLETE prior to polling the external PHY.
> > >
> > > The PCS handling relies on "pcs-handle" device tree handle which
> > > serves the similar purpose in Linux device tree.
> > >
> > > Signed-off-by: Nikita Shubin <n.shubin at yadro.com>
> > > ---  
> <snip>
> > > @@ -540,6 +565,20 @@ static int axiemac_start(struct udevice *dev)
> > >   			return -1;
> > >   	}
> > >
> > > +	if (pcsdev) {
> > > +		/* It looks like we need a bit of delay for core
> > > to come up
> > > +		 * may be we could poll MgtRdy or PhyRstCmplt bit
> > > +		 * in 0x00000010, but 1 msec is no a big deal.
> > > +		 */
> > > +		udelay(1000);
> > > +		ret = phywrite(priv, pcsdev->addr, MII_BMCR,
> > > +				BMCR_ANENABLE | BMCR_FULLDPLX |  
> > BMCR_SPEED1000);  
> 
> Thanks for the patch.
> Could you please confirm that BMCR_ISOLATE is also being handled in
> the autonegotiation path? For ex., the equivalent in linux can be
> found here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/phylink.c#n3050

For linux BMCR_ISOLATE is cleared and BMCR_ANENABLE is set (if mode is
MLO_AN_INBAND which is definitely our case) in
phylink_mii_c22_pcs_config. 

In current patch BMCR_ISOLATE is cleared by write, may be indeed
something like:
phyread(priv, pcsdev->addr, MII_BMCR, &val);
val &= ~(BMCR_ISOLATE);
val |= BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000;
phywrite(priv, pcsdev->addr, MII_BMCR, val);

Whould make more sense.

> 
> Could you also please handle the SGMII/1000BaseX standard selection
> in the PCS PMA IP? For reference, please see:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#n1619

You mean to handle "xlnx,switch-x-sgmii" device tree property in same
manner as linux does? No problem then.

Yours,
Nikita Shubin.


> 
> Regards,
> Harini