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

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

Date:   Fri, 2 Mar 2018 14:40:42 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Bryan.Whitehead@...rochip.com
Cc:     davem@...emloft.net, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 1/2] lan743x: Add main source files for new
 lan743x driver

On Fri, Mar 02, 2018 at 05:57:54AM +0000, Bryan.Whitehead@...rochip.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