[U-Boot] [PATCH 05/10] net: designware: Add a post-started hook

[Joe Hershberger]

Hi Bin,

On Wed, Oct 7, 2015 at 4:45 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi,
>
> On Tue, Oct 6, 2015 at 12:39 AM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> Hi Sjoerd,
>>
>> On Mon, Oct 5, 2015 at 3:55 AM, Sjoerd Simons
>> <sjoerd.simons at collabora.co.uk> wrote:
>>>
>>> On Sat, 2015-10-03 at 15:30 +0100, Simon Glass wrote:
>>> > Hi Sjoerd,
>>>
>>> > >  static int designware_eth_send(struct udevice *dev, void *packet,
>>> > > int length)
>>> > > diff --git a/drivers/net/designware.h b/drivers/net/designware.h
>>> > > index 47e727b..b45599b 100644
>>> > > --- a/drivers/net/designware.h
>>> > > +++ b/drivers/net/designware.h
>>> > > @@ -236,6 +236,10 @@ struct dw_eth_dev {
>>> > >  #endif
>>> > >         struct phy_device *phydev;
>>> > >         struct mii_dev *bus;
>>> > > +
>>> > > +#ifdef CONFIG_DM_ETH
>>> > > +       int (*started)(struct udevice *dev);
>>> > > +#endif
>>> >
>>> > With driver model we should not need to add such hooks. is this
>>> > needed
>>> > because we don't have a PHY uclass yet?
>>>
>>> Essentially I need to be able to configure one of the GMAC clocks
>>> depending on the result of the phy negotiation. Looking at the linux
>>> kernel this seems a rather common item for the dw gmac interface.
>>>
>>> I guess, I could do without that hook by exporting all functions
>>> required to fill the eth_ops struct and override the start function.
>>> Would you prefer that?
>>
>> I prefer the hook.
>>
>>> I guess a PHY uclass would also help here, assuming such a uclass would
>>> provide a callback for link state changes (e.g. like of_phy_connect in
>>> Linux).
>>
>
> From what I read to this codes, the work done in the hook is to
> program the MAC's clock, not the PHY. So not sure if the PHY uclass
> can help here.
>
>> I agree that once we have the phy uclass then we should handle it much
>> more like Linux, but for now the hook seems cleaner.
>>
>
> Has anyone looked at my suggestion about where we should insert the hook?

I looked at this today and agree that for the DW driver, that is a
more appropriate place to put this hook (especially if renamed as you
proposed).

Ultimately I still think it will be better to have a hook added to
UCLASS_ETH that is invoked by UCLASS_ETH_PHY on link change. This way
it is available to other eth drivers as well. I know that we will also
need this for the macb driver.

-Joe