On Tue, Jan 09, 2024 at 08:22:40PM +0530, Sneh Shah wrote: > > > On 1/9/2024 1:00 AM, Andrew Lunn wrote: > > On Mon, Jan 08, 2024 at 05:41:28PM +0530, Sneh Shah wrote: > >> Serdes phy needs to operate at 2500 mode for 2.5G speed and 1000 > >> mode for 1G/100M/10M speed. > >> Added changes to configure serdes phy and mac based on link speed. > > > > Please take a look at: > > > > https://www.kernel.org/doc/html/next/process/maintainer-netdev.html > > > > The Subject is missing which tree this is for. Also, net-next is > > closed at the moment. > > It was supposed to be net-next. Missed updating in subject. > Sorry for that! > If net-next is closed at the moment, how to proceed further? > Should I wait until it gets reopened? Yes, please repost in two weeks time. > >> switch (ethqos->speed) { > >> + case SPEED_2500: > >> + val &= ~ETHQOS_MAC_CTRL_PORT_SEL; > >> + rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG, > >> + RGMII_CONFIG2_RGMII_CLK_SEL_CFG, > >> + RGMII_IO_MACRO_CONFIG2); > >> + if (ethqos->serdes_speed != SPEED_2500) > >> + phy_set_speed(ethqos->serdes_phy, SPEED_2500); > > > > Is calling phy_set_speed() expensive? Why not just unconditionally > > call it? > > > It reconfigures whole serdes phy block, with lots of register read/writes. > So I feel it is better to avoid doing this unconditionally O.K, please add this to the commit message. Part of the purpose of the commit message is to try to answer questions the reviewers are going to ask when they look at the code. Its better to put more in the commit message than less, it helps get your code merged faster, and reduces the load on reviewers. Andrew