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

[Michal Simek]

Hi,

On 09. 12. 20 17:30, Michael Walle wrote:
> 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")

Also take a look at 83e4c7e9ffa57fe4116967999c223c952a46a78a
which is more or less the same things as is done in linux
by351d224f64afc1b3b359a1738b7d4600c7e64061

And we are using it for i2c subsystem.

If you look at Linux kernel i2c/spi subsystems they are using it for
quite a while. Recently mmc subsystem starts to use it.
On the other hand we had similar discussion around networking and it has
never started to be used.

In general make sense if you have uclass that it is recorded(based on
aliases) the first highest free ID and start to use it for devices which
are not listed or don't have record in aliases.

That's IMHO the best predictable behavior we could reach. If you care
about numbering scheme then your device should have alias.

Also if there are devices which doesn't have alias keyword we should
work with DT guys to get it listed in the spec.

Thanks,
Michal