On Sun, Feb 16, 2025 at 03:47:18PM +0000, Russell King (Oracle) wrote:
> On Sun, Feb 16, 2025 at 08:39:51PM +0800, Inochi Amaoto wrote:
> > +static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
> > +{
> > + struct sophgo_dwmac *dwmac = priv;
> > + long rate;
> > + int ret;
> > +
> > + rate = rgmii_clock(speed);
> > + if (rate < 0) {
> > + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> > + return;
> > + }
> > +
> > + ret = clk_set_rate(dwmac->clk_tx, rate);
> > + if (ret)
> > + dev_err(dwmac->dev, "failed to set tx rate %ld: %pe\n",
> > + rate, ERR_PTR(ret));
> > +}
>
> There are a bunch of other platform support in stmmac that are doing
> the same:
>
> dwmac-s32.c is virtually identical
> dwmac-imx.c does the same, although has some pre-conditions
> dwmac-dwc-qos-eth.c is virually identical but the two steps are split
> across a bunch of register writes
> dwmac-starfive.c looks the same
> dwmac-rk.c also
> dwmac-intel-plat.c also
>
> So, I wonder whether either this should be a helper, or whether core
> code should be doing this. Maybe something to look at as a part of
> this patch submission?
Inochi, please could you look at the datasheet for this IP. Is the
transmit clock a part of the IP? Can we expect all devices integrating
this IP to have such a clock? That would be a good indicator the clock
handling should be moved into the core.
Andrew
