Re: [PATCH v3 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Mar 02, 2018 at 05:57:54AM +0000, Bryan.Whitehead@microchip.com wrote:
> > > +static int lan743x_phy_reset(struct lan743x_adapter *adapter) {
> > > +	u32 data;
> > > +
> > > +	data = lan743x_csr_read(adapter, PMT_CTL);
> > > +	data |= PMT_CTL_ETH_PHY_RST_;
> > > +	lan743x_csr_write(adapter, PMT_CTL, data);
> > > +
> > > +	return readx_poll_timeout(LAN743X_CSR_READ_OP, PMT_CTL,
> > data,
> > > +				  (!(data & PMT_CTL_ETH_PHY_RST_) &&
> > > +				  (data & PMT_CTL_READY_)),
> > > +				  50000, 1000000);
> > > +}
> > 
> > Hi Bryan
> > 
> > Could you explain this a bit more. What exactly is it resetting? Do we need to
> > tell the phylib that the PHY has been reset and that it needs to re-program it?
> > Or by phy do you mean a SERDES interface?
> 
> Hi Andrew,
> 
> This function resets the Ethernet phy. But it is called only in
> probe and before mdiobus_register. So I don't believe it is
> necessary to tell phylib.

Hi Bryan

So the PHY is built in? It should be safe. Normally we have the PHY
driver, or the generic layers handle any such reset via a GPIO. But
built in is different.

> [snip]
> > > +
> > > +	/* PHY interrupt enabled here */
> > > +	phy_start(phydev);
> > > +	phy_start_aneg(phydev);
> > > +	return 0;
> > 
> > Are phy interrupts really enabled here? I could of missed it, but i don't see
> > anywhere PHY interrupts are configured. This is either done via device tree,
> > you set phydev->irq, or mdiobus->irq[X].
> 

> Sorry that is an obsolete comment, I will remove it. It is not using
> phy interrupts. It's using polling.

Assuming it is built in, does the MAC get the interrupt?
phy_mac_interrupt() can be called if so.

    Andrew