| Message ID | 20201130015402.2328621-1-sjg@chromium.org |
|---|---|
| Headers | show |
| Series | dm: Change the way sequence numbers are implemented | expand |
On 11/30/20 2:53 AM, Simon Glass wrote: > 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. > For ad-hoc calls to device_bind() afterwards (e.g. from driver code), the > sequence is set to the maximum sequence number for the uclass + 1. > > Apart from the simplicity gains, overall these changes seem to reduce the > number of tweaks and workarounds needed to get the desired behaviour. > > However there will certainly be some problems created, so board > maintainers should test this out. What is the effect of the series on removable devices like USB storage, USB Ethernet, SATA disks, ...? Is there a test for removable devices? Best regards Heinrich > > > Simon Glass (27): > linker_lists: Fix alignment issue > efi: Drop unwanted message in efi_uc_destroy() > dm: Avoid accessing seq directly > dm: core: Update uclass_find_next_free_req_seq() args > dm: core: Add a new sequence number for devices > dm: test: Add support for new sequence numbers > dm: core: Switch binding to use new sequence numbers > dm: Fix return value in dev_read_alias_seq() > dm: test: Drop assumptions of no sequence numbers > octeon: Don't attempt to set the sequence number > i2c: Update for new sequence numbers > net: Update to use new sequence numbers > pci: Update to use new sequence numbers > spi: Update for new sequence numbers > usb: ehci-mx6: Drop assignment of sequence number > usb: Update for new sequence numbers > x86: Drop unnecessary mp_init logic > x86: Simplify acpi_device_infer_name() > gpio: Update for new sequence numbers > pinctrl: Update for new sequence numbers > dm: Switch over to use new sequence number for dev_seq() > dm: Drop uclass_resolve_seq() > dm: Drop the unused arg in uclass_find_device_by_seq() > dm: core: Simplify uclass_find_next_free_req_seq() > cmd: Drop use of old sequence numbers in commands > dm: core: Drop seq and req_seq > dm: Update documentation for new sequence numbers > > arch/Kconfig | 11 +++ > arch/arm/include/asm/mach-imx/mxc_i2c.h | 2 +- > arch/arm/mach-k3/am6_init.c | 2 +- > arch/arm/mach-k3/j721e_init.c | 2 +- > arch/arm/mach-k3/sysfw-loader.c | 2 +- > arch/sandbox/dts/test.dts | 2 +- > arch/x86/cpu/apollolake/cpu.c | 2 +- > arch/x86/cpu/broadwell/cpu_full.c | 2 +- > arch/x86/cpu/ivybridge/model_206ax.c | 2 +- > arch/x86/cpu/mp_init.c | 23 ++--- > arch/x86/include/asm/mp.h | 2 +- > board/xilinx/versal/board.c | 12 +-- > board/xilinx/zynqmp/zynqmp.c | 12 +-- > cmd/axi.c | 6 +- > cmd/cpu.c | 2 +- > cmd/i2c.c | 6 +- > cmd/misc.c | 2 +- > cmd/osd.c | 6 +- > cmd/pci.c | 7 +- > cmd/pmic.c | 4 +- > cmd/remoteproc.c | 2 +- > cmd/w1.c | 4 +- > doc/api/linker_lists.rst | 62 ++++++++++++ > doc/driver-model/design.rst | 58 +++++++----- > drivers/core/device-remove.c | 1 - > drivers/core/device.c | 52 ++++------ > drivers/core/dump.c | 4 +- > drivers/core/read.c | 4 +- > drivers/core/root.c | 21 ++++- > drivers/core/uclass.c | 115 ++++++++++------------- > drivers/gpio/imx_rgpio2p.c | 2 +- > drivers/gpio/iproc_gpio.c | 2 +- > drivers/gpio/mvebu_gpio.c | 2 +- > drivers/gpio/mxc_gpio.c | 2 +- > drivers/gpio/octeon_gpio.c | 2 +- > drivers/gpio/vybrid_gpio.c | 2 +- > drivers/i2c/ast_i2c.c | 4 +- > drivers/i2c/davinci_i2c.c | 2 +- > drivers/i2c/designware_i2c_pci.c | 16 +--- > drivers/i2c/exynos_hs_i2c.c | 2 +- > drivers/i2c/i2c-gpio.c | 2 +- > drivers/i2c/i2c-uclass.c | 8 +- > drivers/i2c/i2c-versatile.c | 5 - > drivers/i2c/imx_lpi2c.c | 12 +-- > drivers/i2c/intel_i2c.c | 12 +-- > drivers/i2c/lpc32xx_i2c.c | 6 +- > drivers/i2c/muxes/i2c-mux-uclass.c | 4 +- > drivers/i2c/mvtwsi.c | 4 +- > drivers/i2c/mxc_i2c.c | 10 +- > drivers/i2c/nx_i2c.c | 2 +- > drivers/i2c/octeon_i2c.c | 3 +- > drivers/i2c/s3c24x0_i2c.c | 2 +- > drivers/i2c/tegra_i2c.c | 5 +- > drivers/mmc/fsl_esdhc_imx.c | 4 +- > drivers/mmc/octeontx_hsmmc.c | 2 - > drivers/mtd/spi/sandbox.c | 4 +- > drivers/net/dwc_eth_qos.c | 2 +- > drivers/net/fec_mxc.c | 7 +- > drivers/net/fsl-mc/mc.c | 2 +- > drivers/net/fsl_mcdmafec.c | 2 +- > drivers/net/ftgmac100.c | 2 +- > drivers/net/higmacv300.c | 2 +- > drivers/net/mcffec.c | 2 +- > drivers/net/octeontx/nicvf_main.c | 9 +- > drivers/net/octeontx/smi.c | 3 +- > drivers/net/octeontx2/nix.c | 2 +- > drivers/net/octeontx2/rvu_pf.c | 6 +- > drivers/net/xilinx_axi_emac.c | 2 +- > drivers/net/xilinx_emaclite.c | 2 +- > drivers/net/zynq_gem.c | 2 +- > drivers/pci/pci-aardvark.c | 2 +- > drivers/pci/pci-uclass.c | 42 ++++----- > drivers/pci/pci_auto.c | 6 +- > drivers/pci/pcie_dw_mvebu.c | 6 +- > drivers/pci/pcie_dw_ti.c | 6 +- > drivers/pci/pcie_ecam_generic.c | 2 +- > drivers/pci/pcie_fsl.c | 16 ++-- > drivers/pci/pcie_intel_fpga.c | 2 +- > drivers/pci/pcie_layerscape_fixup.c | 4 +- > drivers/pci/pcie_layerscape_gen4.c | 10 +- > drivers/pci/pcie_layerscape_gen4_fixup.c | 2 +- > drivers/pci/pcie_layerscape_rc.c | 12 +-- > drivers/pci/pcie_mediatek.c | 2 +- > drivers/pci/pcie_rockchip.c | 6 +- > drivers/pinctrl/exynos/pinctrl-exynos.c | 2 +- > drivers/serial/serial_mcf.c | 2 +- > drivers/serial/serial_s5p.c | 2 +- > drivers/spi/altera_spi.c | 2 +- > drivers/spi/cf_spi.c | 12 +-- > drivers/spi/fsl_dspi.c | 8 +- > drivers/spi/fsl_espi.c | 2 +- > drivers/spi/octeon_spi.c | 2 +- > drivers/spi/pic32_spi.c | 4 +- > drivers/spi/rk_spi.c | 1 - > drivers/spi/sandbox_spi.c | 2 +- > drivers/spi/spi-uclass.c | 4 +- > drivers/spi/tegra114_spi.c | 2 +- > drivers/spi/tegra20_sflash.c | 2 +- > drivers/spi/tegra20_slink.c | 2 +- > drivers/spi/tegra210_qspi.c | 2 +- > drivers/spi/xilinx_spi.c | 2 +- > drivers/spi/zynq_qspi.c | 2 +- > drivers/spi/zynq_spi.c | 2 +- > drivers/usb/gadget/max3420_udc.c | 2 +- > drivers/usb/host/ehci-mx5.c | 2 +- > drivers/usb/host/ehci-mx6.c | 14 ++- > drivers/usb/host/ehci-omap.c | 2 +- > drivers/usb/host/ehci-vf.c | 8 +- > drivers/usb/host/usb-sandbox.c | 2 +- > drivers/usb/host/usb-uclass.c | 6 +- > drivers/video/vidconsole-uclass.c | 4 +- > drivers/virtio/virtio-uclass.c | 2 +- > drivers/watchdog/ast_wdt.c | 2 +- > drivers/watchdog/at91sam9_wdt.c | 2 +- > drivers/watchdog/cdns_wdt.c | 2 +- > drivers/watchdog/omap_wdt.c | 2 +- > drivers/watchdog/orion_wdt.c | 2 +- > drivers/watchdog/sbsa_gwdt.c | 2 +- > drivers/watchdog/sp805_wdt.c | 2 +- > drivers/watchdog/tangier_wdt.c | 2 +- > drivers/watchdog/xilinx_tb_wdt.c | 2 +- > drivers/watchdog/xilinx_wwdt.c | 2 +- > include/asm-generic/global_data.h | 2 + > include/configs/sandbox.h | 2 +- > include/dm/device.h | 32 +++---- > include/dm/uclass-internal.h | 34 +++---- > include/dm/uclass.h | 15 +-- > include/linker_lists.h | 3 +- > include/pci.h | 2 +- > include/spi.h | 2 +- > lib/acpi/acpi_device.c | 27 +----- > lib/efi_driver/efi_uclass.c | 1 - > lib/efi_loader/efi_device_path.c | 4 +- > net/eth-uclass.c | 24 +++-- > test/dm/acpi.c | 6 +- > test/dm/blk.c | 3 - > test/dm/bus.c | 15 +-- > test/dm/core.c | 19 ++++ > test/dm/eth.c | 14 +-- > test/dm/i2c.c | 3 - > test/dm/spi.c | 3 - > test/dm/test-fdt.c | 100 ++++++++++++-------- > test/dm/test-main.c | 6 ++ > 143 files changed, 583 insertions(+), 569 deletions(-) >
Hi Heinrich, On Tue, 1 Dec 2020 at 01:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 11/30/20 2:53 AM, Simon Glass wrote: > > 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. > > For ad-hoc calls to device_bind() afterwards (e.g. from driver code), the > > sequence is set to the maximum sequence number for the uclass + 1. > > > > Apart from the simplicity gains, overall these changes seem to reduce the > > number of tweaks and workarounds needed to get the desired behaviour. > > > > However there will certainly be some problems created, so board > > maintainers should test this out. > > What is the effect of the series on removable devices like USB storage, > USB Ethernet, SATA disks, ...? They should work as now, in that they get the maximum seq + 1. > > Is there a test for removable devices? The test I added in this series covers the case of devices being bound ad-hoc, so yes I think so. Regards, Simon
Hi Simon, 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). 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. > For ad-hoc calls to device_bind() afterwards (e.g. from driver code), > the > sequence is set to the maximum sequence number for the uclass + 1. > > Apart from the simplicity gains, overall these changes seem to reduce > the > number of tweaks and workarounds needed to get the desired behaviour. > > However there will certainly be some problems created, so board > maintainers should test this out. -michael
On Tue, Dec 08, 2020 at 11:52:07PM +0100, Michael Walle wrote: > Hi Simon, > > 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). > > 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. I want to echo this concern. One of the things that can be challenging when working with devices in Linux is that it's now long accepted that you can't rely on anything like probe order as that can and will change randomly (seemingly). You have to instead rely on some stable attribute of the device. Consistency of device numbering is a feature for us.
Hi Michael, On Tue, 8 Dec 2020 at 15:52, Michael Walle <michael@walle.cc> wrote: > > Hi Simon, > > 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"'? 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. Regards, Simon
Hi Tom, On Wed, 9 Dec 2020 at 09:19, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Dec 08, 2020 at 11:52:07PM +0100, Michael Walle wrote: > > Hi Simon, > > > > 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). > > > > 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. > > I want to echo this concern. One of the things that can be challenging > when working with devices in Linux is that it's now long accepted that > you can't rely on anything like probe order as that can and will change > randomly (seemingly). You have to instead rely on some stable attribute > of the device. Consistency of device numbering is a feature for us. This is still using aliases for devices that have them. But for dynamically allocated devices (without aliases) we always rely on what is next available. I think I can change the algorithm to look for the number after all existing aliases. Regards, Simon
Hi Simon, Am 2020-12-09 17:23, schrieb Simon Glass: > On Tue, 8 Dec 2020 at 15:52, Michael Walle <michael@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") -michael
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@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
Hi Michal, On Thu, 10 Dec 2020 at 00:34, Michal Simek <michal.simek@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@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)? Also has there been any discussion of using phandles instead of strings? Perhaps we could create a new node with that approach. Regards, Simon
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@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@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. > > 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"; }; Thanks, Michal
Hi Michal, On Thu, 10 Dec 2020 at 10:33, Michal Simek <michal.simek@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@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@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. > > > > > 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. Regards, Simon
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@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@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@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). Thanks, Michal
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@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@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@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. Best regards Heinrich
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@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@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@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, Michal
Hi Michal, Heinrich, On Fri, 11 Dec 2020 at 00:54, Michal Simek <michal.simek@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@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@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@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