[PATCH 00/27] dm: Change the way sequence numbers are implemented
Michal Simek
michal.simek at xilinx.com
Fri Dec 11 08:53:53 CET 2020
On 11. 12. 20 8:42, Heinrich Schuchardt wrote:
> On 12/11/20 8:28 AM, Michal Simek wrote:
>> Hi Simon,
>>
>> On 10. 12. 20 18:46, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On Thu, 10 Dec 2020 at 10:33, Michal Simek <michal.simek at xilinx.com>
>>> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On 10. 12. 20 18:27, Simon Glass wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On Thu, 10 Dec 2020 at 00:34, Michal Simek
>>>>> <michal.simek at xilinx.com> wrote:
>>>>>>
>>>>>> 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.
>>>>>
>>>>> Do you mean the root of the name (e.g. i2c for i2c1)?
>>>>
>>>> yes assigned the root of the name to specific uclass.
>>>
>>> Oh gosh, I didn't even know they were in the DT spec.
>>
>> In DT spec you have recommended node names which is more or less fit
>> with uclasses.
>>
>>
>>>>
>>>>>
>>>>> Also has there been any discussion of using phandles instead of
>>>>> strings? Perhaps we could create a new node with that approach.
>>>>
>>>> What do you mean by string?
>>>> Aliases are not using phandles. It is just label converted to path.
>>>> for example from dtc -I dtb -O dts.
>>>>
>>>> aliases {
>>>> ethernet0 = "/amba_pl/ethernet at 40c00000";
>>>> i2c0 = "/amba_pl/i2c at 40800000";
>>>> serial0 = "/amba_pl/serial at 40600000";
>>>> };
>>>
>>> Yes that's my point. It uses strings, which is inefficient. I feel we
>>> could use phandles instead and solve a number of problems.
>>
>> If you want to change it we can't change it in one project only. And
>> this discussion should be made against DT specification.
>> Another thing is that that Linux is not capable to update aliases when
>> for example DT overlay is applied. This part is completely ignored even
>> if there is no collision. Nothing important for U-Boot but something to
>> keep in mind.
>> IIRC We don't support run time overlay applying to be able to add more
>> nodes (but would be useful to have it too).
>
> https://lkml.org/lkml/2016/12/20/510
> [PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays
> suggested a mechanism to do so but was not merged.
The last sentence was more for u-boot run time overlay support
especially useful for SOMs.
Thanks,
Michal
More information about the U-Boot
mailing list