[U-Boot] [PATCH 3/4] dm: net: macb: Implement link speed change callback
Bin Meng
bmeng.cn at gmail.com
Wed May 22 06:55:20 UTC 2019
Hi Lukas,
On Mon, May 20, 2019 at 7:29 PM Auer, Lukas
<lukas.auer at aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Thu, 2019-05-16 at 02:12 -0700, Bin Meng wrote:
> > At present the link speed change callback is a nop. According to
> > macb device tree bindings, an optional "tx_clk" is used to clock
> > the ethernet controller's TX_CLK under different link speed.
> >
> > In 10/100 MII mode, transmit logic must be clocked from a free
> > running clock generated by the external PHY. In gigabit GMII mode,
> > the controller, not the external PHY, must generate the 125 MHz
> > transmit clock towards the PHY.
> >
> > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > ---
> >
> > drivers/net/macb.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > index b7f404e..7d86fa1 100644
> > --- a/drivers/net/macb.c
> > +++ b/drivers/net/macb.c
> > @@ -497,6 +497,32 @@ static int macb_phy_find(struct macb_device *macb, const char *name)
> > #ifdef CONFIG_DM_ETH
> > int __weak macb_linkspd_cb(struct udevice *dev, unsigned int speed)
> > {
> > +#ifdef CONFIG_CLK
> > + struct clk tx_clk;
> > + ulong rate;
> > + int ret;
> > +
> > + ret = clk_get_by_name(dev, "tx_clk", &tx_clk);
> > + if (ret)
> > + return 0;
>
> Can you return errors in ret here?
No, we should ignore DT that does not contain the "tx_clk" source
hence cannot return error here. I will add a comment in v2.
>
> > +
> > + switch (speed) {
> > + case _10BASET:
> > + rate = 2500000;
> > + break;
> > + case _100BASET:
> > + rate = 25000000;
> > + break;
> > + case _1000BASET:
> > + default:
> > + rate = 125000000;
> > + break;
>
> The Linux driver simply returns in the default case, without changing
> tx_clk. I am wondering if we should do the same in U-Boot?
Sure.
>
> > + }
> > +
> > + if (tx_clk.dev)
> > + clk_set_rate(&tx_clk, rate);
>
> Can you also check the return value of clk_set_rate() here?
Will add the return value check in v2.
Regards,
Bin
More information about the U-Boot
mailing list