Hi Vivien, On Fri, 16 Aug 2019 19:05:37 -0400 Vivien Didelot <vivien.dide...@gmail.com> wrote:
> I think the DSA switch port_setup/port_teardown operations are fine, but the > idea would be that the drivers must no longer setup their ports directly > in their .setup function. So for mv88e6xxx precisely, we should rename > mv88e6xxx_setup_port to mv88e6xxx_port_setup, and move all the port-related > code from mv88e6xxx_setup into mv88e6xxx_port_setup. I looked into the driver, and found out that mv88e6xxx_setup calls many other setup functions after the calls to mv88e6xxx_setup_port for each port: 1. setup errata 2. cache cmode 3. for each port setup_port 4. irl setup 5. mac setup 6. phy setup 7. vtu setup 8. pvt setup 9. atu setup 10. broadcast setuo 11. pot setup 12. rmu setup 13. rsvd2cpu setup 14. trunk setup 15. devmap setup 16. pri setup 17. ptp setup 18. hwtstamp setup 19. stats setup The problem is that some of these steps (after step 3) may depend on some of the work done by step 3. Some of these functions iterate again over the port array (mv88e6xxx_hwtstamp_setup, for example). We cannot simply move step 3 to be called from DSA after mv88e6xxx_setup. I now do not know exactly what to do about the error prone naming of setup_port vs port_setup. One way would be to rename the mv88e6xxx_setup_port function to mv88e6xxx_setup_port_regs, or mv88e6xxx_port_pre_setup, or something like that. Would the names mv88e6xxx_port_setup and mv88e6xxx_setup_port_regs still be very confusing and error prone? I think maybe yes... Other solution would be to, instead of the .port_setup() and .port_teardown() DSA ops, create the .after_setup() and .before_teardown() ops I mentioned in the previous mail. And yet another (in my opinion very improper) solution could be that the .setup() method could call dsa_port_setup() from within itself, to ensure that the needed structres exist. Please let me know what you think about this. The first solution to me currently seems as the easiest. Marek
