On 9/18/20 5:26 PM, Biju Das wrote: [...]
+++ b/drivers/net/ravb.c @@ -438,7 +438,8 @@ static int ravb_config(struct udevice *dev)
writel(mask, eth->iobase + RAVB_REG_ECMR);
-phy->drv->writeext(phy, -1, 0x02, 0x08, (0x0f << 5) | 0x19); +if (phy->drv->writeext) +phy->drv->writeext(phy, -1, 0x02, 0x08, (0x0f << 5) | 0x19);
Shouldn't we rather move this into the PHY driver altogether ?
If I fix, the phydriver with empty function, compiler will complain about
unused parameter right?
What about other phy's which doesn't have this callback.
I mean, should we not move this entire code which configures something in a Micrel PHY, and is specific to Micrel PHY, into the Micrel PHY driver and remove the whole call of writeext from this ethernet driver ?
Ok , this function is invoked during data transfer.
The ravb_config() is invoked when starting the interface, see ravb_start(), it is not repeatedly called during data transfer.
We could remove this function and test on all RCar boards to see any issues present or not?
No, because calling the writeext configures something in the Micrel KSZ90x1 PHY, and by removing it, I suspect the Micrel PHY would stop working.
By looking at [1], only this driver is using writeext. [1]https://elixir.bootlin.com/u-boot/v2020.10-rc4/A/ident/writeext
git grep indicates a couple more sites where the writeext is called. But look into the KSZ9031 datasheet, that particular writeext call seems to be setting up RGMII Clock Pad Skew (MMD Address 2, Register 8), and I think there is a matching DT binding to set those up too, rxc-skew-ps and txc-skew-ps I think.
So I think if you set those two DT properties in rcar2/3 DTs accordingly (and that might already be the case, but please double-check), you can even drop this entire writeext altogether.
[...]