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

Simon Glass sjg at chromium.org
Fri Dec 11 17:24:11 CET 2020 

Hi Michal, Heinrich,

On Fri, 11 Dec 2020 at 00:54, Michal Simek <michal.simek at xilinx.com> wrote:
>
>
>
> 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 for the background. I think I will have a look at it and ask on
the DT side.

Regards,
Simon

More information about the U-Boot mailing list