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

[Katakam, Harini]

Hi Nikita,

<snip>
> > > >
> > > > +	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/tre
> > e/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.

Thanks, yes it is cleared by the explicit value written. That's fine.
The read-modify-write snippet you mentioned above is just good to have.

> 
> >
> > 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/tre
> > e/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.

Yes, thanks.

Regards,
Harini