Re: [PATCH v2] net: stmmac: dwmac-qcom-ethqos: Add support for 2.5G SGMII

From: Andrew Lunn
Date: Tue Jan 09 2024 - 10:14:26 EST


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