On Mon, 26 Aug 2019 20:36:14 +0200, Marek Behun <marek.be...@nic.cz> wrote: > > Ask yourself what is the single task achieved by this function, and name > > this > > operation accordingly. It seems to change the CMODE to be writable, only > > supported by certain switch models right? So in addition to port_get_cmode > > and port_set_cmode, you can add port_set_cmode_writable, and call it right > > before or after port_set_cmode in mv88e6xxx_port_setup_mac. > > Andrew's complaint was also about this function being called every time > cmode is to be changed. The cmode does need to be made writable only > once. In this sense it does make sense to put into into > mv88e6xxx_setup_port.
mv88e6xxx_port_setup_mac is called by mv88e6xxx_setup_port as expected and also .phylink_mac_config. I don't think they are called that often and both deal with configuration, so I'd prefer to keep this consistent and group the two operations together in mv88e6xxx_port_setup_mac, if that's good for Andrew too. Thanks, Vivien