[U-Boot] [RFC PATCH v3 01/14] dm: core: Allow seq numbers to be resolved before probe

[Simon Glass]

Hi Joe,

On 13 February 2015 at 19:33, Joe Hershberger <joe.hershberger at gmail.com> wrote:
> On Thu, Feb 12, 2015 at 11:14 PM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Joe,
>>
>> On 10 February 2015 at 23:08, Joe Hershberger <joe.hershberger at gmail.com>
>> wrote:
>> > Hi Simon,
>> >
>> > On Tue, Feb 10, 2015 at 10:39 PM, Simon Glass <sjg at chromium.org> wrote:
>> >>
>> >> Hi Joe,
>> >>
>> >> On 10 February 2015 at 18:30, Joe Hershberger <joe.hershberger at ni.com>
>> >> wrote:
>> >> > Before this patch, if the sequence numbers were resolved before
>> >> > probe,
>> >> > this code would insist on defining new non-conflicting-with-itself
>> >> > seq
>> >> > numbers. Now any "non -1" seq number is accepted as already resolved.
>> >>
>> >> Can you explain what problem this solves? At present, when probing a
>> >> device, ->seq must be -1 (sort-of by definition since it doesn't exist
>> >> as an active device in the uclass).
>> >
>> > Please look at eth_post_bind() in patch 07/14.  The Ethernet devices
>> > need to
>> > write their hardware addresses to the registers in bind (since it needs
>> > to
>> > happen regardless of the device being used so that Linux will see the
>> > MAC
>> > address).  As such, the sequence number is needed to look up the
>> > ethaddr. In
>> > order to avoid probing all the devices to get the seq number resolved, I
>> > resolve it in post_bind to avoid the rest of the overhead (thus no
>> > longer
>> > probing in post_bind, which was one of the issues previously).  Then
>> > when
>> > probe comes along, the seq is already resolved.  That's why this patch
>> > is
>> > needed.
>>
>> OK I see.
>>
>> This is a bit messy. If the MAC address assignment is part of the bind
>> step then it shouldn't need the seq number.
>
> Not sure why you say that.  The reason I need the seq number is because I
> need to look up the proper env variable for the MAC address.  E.g. ethaddr,
> eth2addr, etc.  The seq number select which one to read from the env.

We should be able to do this after a probe. A device which is bound
but not probed does not have a sequence number, as things currently
stand.

>
>> I can think of some poor ways to do this but a nice way is not obvious!
>
> Not sure what you're referring to here.  What is "this" in this context?

Figuring out the sequence number.

>
>> One option would be probe all the Ethernet devices on startup. If
>> probe() only set up the hardware (including MAC address) then that
>> might work. It would be fairly fast since it wouldn't involve starting
>> up the link, etc. I suspect you are worried about a lot of Ethernet
>> devices sitting around probed by unused. I'm not sure if that matters
>> though.
>
> I had it probing the devices originally (by calling first and next) and you
> commented that it shouldn't happen until the devices are used.  However, I

That was because your code was probing things in the bind mehod.

> don't think we can guarantee that all drivers that come later will have
> simple probe (since that's not part of the contract).  I think I agree with
> your original statement that we should not probe.  It seems more suitable to
> write the hwaddr in bind as a known and limited side effect.

I don't like the idea of an ethernet device supporting writing its
hardware address before it is probed. Until it is probed we don't
really know it is there, nor where it is exactly (bus, memory
address). So I think writing the hardware address makes more sense
after probe. But probe should not happen as part of bind. It seems to
me it could happen in your eth_init().

>
> The seq number resolution seems fairly well contained as I implemented it in
> bind.  I simply call the core function and write the result to the device
> member.  Then of course this patch to remove the assert.

Yes it is well contained, but I still don't think it is right. If you
want to put '#ifndef CONFIG_DM_NET' around the assert in
uclass_resolve_seq() while we work it out, that is OK with me.

Regards,
Simon