[PATCH 00/27] dm: Change the way sequence numbers are implemented

[Michael Walle]

Hi Simon,

Am 2020-12-09 17:23, schrieb Simon Glass:
> On Tue, 8 Dec 2020 at 15:52, Michael Walle <michael at walle.cc> wrote:
>> Am 2020-11-30 02:53, schrieb Simon Glass:
>> > At present each device has two sequence numbers, with 'req_seq' being
>> > set up at bind time and 'seq' at probe time. The idea is that devices
>> > can 'request' a sequence number and then the conflicts are resolved
>> > when
>> > the device is probed.
>> >
>> > This makes things complicated in a few cases, since we don't really
>> > know
>> > (at bind time) what the sequence number will end up being. We want to
>> > honour the bind-time requests if at all possible, but in fact the only
>> > source of these at present is the devicetree aliases.
>> >
>> > Apart from the obvious need for sequence numbers to supports U-Boot's
>> > numbering on devices on the command line, the current scheme was
>> > designed to:
>> >
>> > - avoid calculating the sequence number until it is needed, to save
>> >   execution time
>> > - allow multiple devices to obtain a particular sequence number as they
>> >   are probed and removed
>> > - retain a record of the 'requested' sequence number even if it turns
>> > out
>> >   that a device could not get it (to allow debugging and retrying)
>> >
>> > After some years using the current scheme it seems on balance that
>> > these
>> > goals don't have as much merit as first thought. The first point would
>> > be persuasive except that we end up reading the devicetree aliases at
>> > bind-time anyway. So the work of resolving the sequence numbers during
>> > probing is not that great. The second point hasn't really been an
>> > issue,
>> > as there is typically no contention for sequence numbers (boards tend
>> > to
>> > allocate them statically in the devicetree). Re the third point, we can
>> > often figure out what was requested by looking at aliases, and in the
>> > cases where we can't, it doesn't seem to matter much.
>> >
>> > Since we have the devicetree available at bind time, we may as well
>> > just
>> > use it, in the hope that the required processing will turn out to be
>> > useful later (i.e. the device actually gets used). In addition, it is
>> > simpler to use a single sequence number, since it avoids confusion and
>> > some extra code.
>> >
>> > This series moves U-Boot to use a single, bind-time sequence number.
>> > All
>> > uclasses with the DM_UC_FLAG_SEQ_ALIAS flag enabled will assign
>> > sequence
>> > numbers to their devices, so that as soon as a device is bound, it has
>> > a
>> > sequence number. If a devicetree alias provides the number, it will be
>> > used. Otherwise, during initial binding, the first free number is used.
>> 
>> What does "first free number mean"?
>> 
>> I have a device tree with the following aliases for network:
>> 
>> aliases {
>>          ethernet0 = &enetc0;
>>          ethernet1 = &enetc1;
>>          ethernet2 = &enetc2;
>>          ethernet3 = &enetc6;
>> };
>> 
>> The individual devices might be disabled, depending on the board 
>> variant
>> (which might also be dynamically determined during startup).
> 
> By disabled, do you mean that they are marked 'status = "disabled"'?

yes

> If so, then they are ignored by DM and will not claim their number.
> 
>> 
>> My first smoke test with this series show the following:
>> 
>>    uclass 32: eth
>>    0   * enetc-0 @ ffd40e60, seq 0
>>    1   * ax88179_eth @ ffd51f50, seq 1
>> 
>> Looks like the usb ethernet device will get seq 1 assigned (after "usb
>> start"). Is this intended?
>> 
>> If so, this is a problem, because for ethernet devices, the MAC 
>> address
>> is assigned according to the ethNaddr variable. And at least for this
>> board (kontron_sl28) the first four are reserved for the ones with the
>> alias entries. Thus I'd have expected that the usb device will get seq 
>> 4
>> assigned.
> 
> OK, so you mean after all existing aliases, even if they did not bind.
> I think we can do that.

Great, that will also match the current behavior. See
be1a6e94254af205bd67d69e3bdb26b161ccd72f ("dm: uclass: don't assign 
aliased seq numbers")

-michael