Hi Michal, Heinrich, On Fri, 11 Dec 2020 at 00:54, Michal Simek <michal.si...@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.si...@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.si...@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 <mich...@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@40c00000"; > >>>> i2c0 = "/amba_pl/i2c@40800000"; > >>>> serial0 = "/amba_pl/serial@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