[U-Boot] [RFC PATCH v4 0/23] Add Driver Model support to network stack

[Simon Glass]

Hi Joe,

On 1 March 2015 at 15:12, Joe Hershberger <joe.hershberger at gmail.com> wrote:
> On Sun, Mar 1, 2015 at 10:13 AM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Joe,
>>
>> On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger at ni.com>
>> wrote:
>> > Add support for the Ethernet MAC controllers.  Phy support will come
>> > later.
>> >
>> > I am still leaving this as an RFC because I plan to add real board
>> > support
>> > before committing to mainline. When it is acceptable / accepted, I will
>> > push it
>> > as a dev branch on the net repo until a real device is supported. If any
>> > required changes are discovered in the process of supporting a real
>> > device I
>> > will send those as a patch against the dev branch, but then squash
>> > before
>> > sending the non-RFC version. I plan to rebase when the merge window
>> > opens
>> > anyway.
>> >
>> > If desired, let me know which of the non-DM related prerequisite patches
>> > are
>> > wanted for this release.
>> >
>> > I've added unit tests to verify functionality.
>> >
>> > There is an additional driver for sandbox that bridges to the RAW
>> > Ethernet API in Linux which lets you test with real traffic. It now
>> > supports localhost as well (the 'lo' interface).
>>
>> I've got some comments queued up the the series so will send those
>> soon (never got to it this week so am catching up!). I wanted to try
>> it out on a board to see how things look. The short answer is that it
>> works really nicely on sunxi. I'll send some patches that I needed,
>> hopefully that doesn't duplicate any work you have done.
>
> It's great that works on your board without much effort. Looking at those
> patches I wouldn't say it duplicates much effort. It's more like your
> changes are a stop-gap to my efforts, which makes me less inclined to wait
> for all of the additional phy and mdio driver-model changes to be complete
> before integrating the dm-eth support to mainline.
>
>> In terms of getting this into mainline, I'd be happy to use
>> u-boot-dm/next if that suits you and Tom. There are series for PCI and
>> USB to sort out, and if the last merge window is any indication it's a
>> real struggle to get multiple large series applied within the merge
>> window when there are dependencies between them.
>
> That makes sense to me. Since it is a next branch, we are still agreeing
> that the branch will be rebased on top of the release, correct?

Yes, once it is in u-boot-dm/next I'll keep it rebased. But since we
are at RC2 I don't expect much effort there.

I want to do this because I originally planned to get PCI into the
current release, but found there were just too many patches to respin
and apply during the merge window, and I didn't get to it. This will
make it easier.

>
>> Some general comments that I will put here for want of a better place:
>>
>> - it would be good to have the DM code in drivers/net/eth-uclass.c at some
>> point
>
> I completely agree.  I moved it there probably 3 different times locally in
> slightly different ways and backed it out each time.  I think it should be a
> follow-on clean-up patch. It makes this initial patch series messier than I
> wanted (hence I backed it out). That is the long-term goal.
>
>> - struct eth_pdata is used by the uclass and is common to all drivers,
>> but I wonder if we will find that drivers want to add their own
>> private platdata? I added phy_interface but that is generic. Let's
>> see.
>
> Yeah, I think that's something that can be revisited pretty easily if the
> need becomes clear.
>
>> - I think the recv() method should change before long. The
>> NetReceive() call should be made from the uclass since it is common to
>> all drivers. Then the recv() method can return a packet if it finds
>> one, but not submit it for processing
>
> I agree. I never liked how it was laid out and now is a great opportunity to
> change it since every driver has to be touched anyway.
>
>> One interesting point for me is that you have taken a slightly more
>> ambitious approach with the conversion by not reusing eth_device. That
>> seems to have have worked out well and makes me think I could revisit
>> SPI flash perhaps and do the same.
>
> I agree that it's nicer to get rid of the extra wrapping structure.
> Naturally the refactor could happen any time, but it seemed simple enough to
> just do now.

All sounds good.

>
>> >
>> > Changes in v4:
>> > -New to v4
>> > -Fix compile regression in !DM_ETH case
>> > -New to v4
>> > -New to v4
>> > -New to v4
>> > -New to v4
>> > -New to v4
>>
>> If you put this in a patch, patman will remove duplicates in the cover
>> letter.
>>
>> Series-process-log: uniq
>>
>> You can also sort with:
>>
>> Series-process-log: sort, uniq
>
> Thanks for the tip. Any reason these are not enabled by default?

>From memory I think some people didn't want their change logs to be
reordered (perhaps because they use more than one line per entry,
which I never do).

I suppose we could change it and see if anyone sqeals!

[snip]

Regards,
Simon