> While the patch in net-next fix a broken condition (PHY driver exist but > doesn't have LEDs OPs), this account a much possible scenario. > > It's totally ok if the PHY driver is not loaded and we fallback to the > Generic PHY and there are LEDs node. > > This is the case with something like > ip link set eth0 down > rmmod air_en8811h > ip link set eth0 up > > On this up, the Generic PHY is loaded and LEDs will wrongly be > registered. We should not add the LED to the phydev LEDs list. > > Do you think this logic is wrong and we should print a warning also in > this case? Or should we bite it and just return 0 with no warning at > all? (again my concern is the additional LEDs entry in sysfs that won't > be actually usable as everything will be rejected) We should not add LEDs which we cannot drive. That much is clear to me. I would also agree that LEDs in DT which we cannot drive is not fatal. So the return value should be 0. The only really open point is phydev_err(), phydev_warn() or phydev_dbg(). Since it is not fatal, phydev_err() is wrong. I would probably go with phydev_dbg(), to aid somebody debugging why the LEDs don't appear in some conditions. Andrew
