Re: [PATCH] net: dsa: microchip: look for phy-mode in port nodes

[Helmut Grohne <helmut.grohne@intenta.de>]

On Thu, Jul 16, 2020 at 09:00:00AM +0200, Helmut Grohne wrote:
> I've prepared a patch based one the one-CPU-port assumption. It really
> becomes way simpler that way. I'd like to give it a little more testing
> before sending it.

I'm sorry, but it is not that simple. Testing revealed a fatal flaw.

At the time ksz_switch_register is called, dev->cpu_port is not
necessarily initialized. For ksz8795, it is initialized during detect
and that is fine. For ksz9477, it is initialized in
ksz9477_config_cpu_port, which comes much too late. The ksz9477 driver
actually handles the case where we choose a CPU port and selects it
using dsa_is_cpu_port (which derives this information from the device
tree during dsa_register_switch).

So in ksz_switch_register, I have no good way of knowing which port will
end up being the CPU port. Options include:
 * Replicating the logic from ksz9477_config_cpu_port. I expect that
   doing this would make the driver harder to maintain.
 * Move the cpu_port computation from ksz9477_config_cpu_port to
   ksz9477_switch_detect. I don't think this would work, because we
   presently call detect before dsa_register_switch, which parses the
   device tree. During detect, dsa_is_cpu_port is not usable.
 * Add a function to ksz_common.c that looks up the phy mode for a port
   from the device tree on demand. That way, ksz9477_config_cpu_port
   could call into the common code instead of reading a ksz_device
   property. It would defer parsing the device tree though and it could
   issue the warning multiple times.
 * Stick with my previous patch that stores the phy mode per-port.

Given the above, the last option seems least bad to me. Do you agree?

Helmut