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
