From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Fri, 11 Dec 2020 08:53:53 +0100 Subject: [PATCH 00/27] dm: Change the way sequence numbers are implemented In-Reply-To: <4b61676a-7d69-f697-9c02-7ada7aa33634@gmx.de> References: <20201130015402.2328621-1-sjg@chromium.org> <641a4b13e178b03ff5a92dd13b566c18@walle.cc> <2159499f-2215-0c48-91cb-c86964080ee8@xilinx.com> <9e767b8c-a7f6-fca1-2942-3a757d154158@xilinx.com> <4b61676a-7d69-f697-9c02-7ada7aa33634@gmx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> wrote: >>>> >>>> Hi Simon, >>>> >>>> On 10. 12. 20 18:27, Simon Glass wrote: >>>>> Hi Michal, >>>>> >>>>> On Thu, 10 Dec 2020 at 00:34, Michal Simek >>>>> 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 >>>>>>>> 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