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 18:26:14 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <andrew@...n.ch>
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

> > > > +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.

Hi Andrew,

The phy is built in for LAN7430, and external for LAN7431. But the same reset should work in both cases because it asserts the normal phy reset line. 

> 
> > [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.

I believe the MAC does support a phy interrupt. But right now it's
working well using phy polling. Is it ok if I post pone supporting phy
interrupts until a future patch? I am planning a series of patches, and I
can add support for phy interrupts to the list.

Bryan