Re: [BUG] bind command leads to invalid state where plaform data is NULL
Hi Heinrich, On Mon, 19 Jun 2023 at 18:51, Heinrich Schuchardt wrote: > > On 6/19/23 14:57, Simon Glass wrote: > > I suggest a simple device that needs no setup. The original commit[1] > > suggests using USB - does that worK? > > > > Regards, > > Simon > > > > [1] 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a > > driver from the command line") > > I see devices listed after bind but I do not see any device that could > be used after binding it with the bind command. It might be worth asking the author. If there is any parent/uclass/device priv/plat then it won't be set up by this command. > > Here is another instance where using the bind command leads to a crash > (on pine64-lts_defconfig). > > Another question: > Is it correct that two devices have the same name? It shouldn't cause any problems since we don't normally reference devices by name. But it could be confusing for the user if there are two children of the same device with the same name. > > => dm tree > Class Index Probed Driver Name > -- > watchdog 0 [ + ] sunxi_wdt | |-- watchdog@1c20ca0 > sysreset 0 [ ] wdt_reboot | | `-- watchdog@1c20ca0 > => unbind sysreset 0 > => bind watchdog 0 wdt_reboot > => reset > resetting ... > "Synchronous Abort" handler, esr 0x9604, far 0xea06ea70 > elr: 4a044054 lr : 4a034c94 (reloc) > elr: bdf8d054 lr : bdf7dc94 > x0 : x1 : > x2 : bdf7dc6c x3 : ea06ea08 > x4 : b9f1ec78 x5 : b9f2cbd0 > x6 : 0072 x7 : b9f1ed30 > x8 : 0040 x9 : fff0 > x10: 0006 x11: 0001869f > x12: b9f1eec8 x13: b9f1efd0 > x14: x15: b9f1e79f > x16: bdf7dc6c x17: > x18: b9f28d90 x19: 0001 > x20: 0001 x21: > x22: b9f38680 x23: 0001 > x24: bdff4f54 x25: > x26: b9f2d2d0 x27: > x28: x29: b9f1ecd0 > > Code: a8c27bfd d65f03c0 d65f03c0 f943 (f9403463) > > The crash is in do_reset(). Looking at the device, it needs plat data and uses it in wdt_reboot_of_to_plat(). Regards, Simon
Re: [BUG] bind command leads to invalid state where plaform data is NULL
On 6/19/23 14:57, Simon Glass wrote: I suggest a simple device that needs no setup. The original commit[1] suggests using USB - does that worK? Regards, Simon [1] 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a driver from the command line") I see devices listed after bind but I do not see any device that could be used after binding it with the bind command. Here is another instance where using the bind command leads to a crash (on pine64-lts_defconfig). Another question: Is it correct that two devices have the same name? => dm tree Class Index Probed Driver Name -- watchdog 0 [ + ] sunxi_wdt | |-- watchdog@1c20ca0 sysreset 0 [ ] wdt_reboot | | `-- watchdog@1c20ca0 => unbind sysreset 0 => bind watchdog 0 wdt_reboot => reset resetting ... "Synchronous Abort" handler, esr 0x9604, far 0xea06ea70 elr: 4a044054 lr : 4a034c94 (reloc) elr: bdf8d054 lr : bdf7dc94 x0 : x1 : x2 : bdf7dc6c x3 : ea06ea08 x4 : b9f1ec78 x5 : b9f2cbd0 x6 : 0072 x7 : b9f1ed30 x8 : 0040 x9 : fff0 x10: 0006 x11: 0001869f x12: b9f1eec8 x13: b9f1efd0 x14: x15: b9f1e79f x16: bdf7dc6c x17: x18: b9f28d90 x19: 0001 x20: 0001 x21: x22: b9f38680 x23: 0001 x24: bdff4f54 x25: x26: b9f2d2d0 x27: x28: x29: b9f1ecd0 Code: a8c27bfd d65f03c0 d65f03c0 f943 (f9403463) The crash is in do_reset(). Best regards Heinrich
Re: [BUG] bind command leads to invalid state where plaform data is NULL
Hi Heinrich, On Sun, 18 Jun 2023 at 11:12, Heinrich Schuchardt wrote: > > Hello Simon, > > from origin/next I build qemu_arm64_defconfig with CONFIG_CMD_BIND=yes. > > I ran the image with: > > qemu-system-aarch64 -semihosting \ > -machine virt,gic-version=max -accel $(ACCEL) -m 1G -smp cores=2 \ > -bios u-boot.bin -cpu $(CPU) -nographic -gdb tcp::1234 \ > -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \ > -drive if=none,file=arm64.img,format=raw,id=mydisk \ > -drive if=pflash,format=raw,index=1,file=envstore.img \ > -device virtio-rng-pci \ > -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0 > > => scsi bind > => dm tree > Class Index Probed DriverName > --- > root 0 [ + ] root_driver root_driver > pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > ahci 0 [ + ] ahci_pci | `-- ahci_pci > scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > blk 0 [ + ] scsi_blk | |-- > ahci_scsi.id0lun0 > partition 0 [ + ] blk_partition | | |-- > ahci_scsi.id0lun0:1 > partition 1 [ + ] blk_partition | | `-- > ahci_scsi.id0lun0:15 > bootdev 2 [ ] scsi_bootdev | `-- > ahci_scsi.id0lun0.bootdev > => unbind blk 0 > => dm tree > Class Index Probed DriverName > --- > root 0 [ + ] root_driver root_driver > pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > ahci 0 [ + ] ahci_pci | `-- ahci_pci > scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > bootdev 2 [ ] scsi_bootdev | `-- > ahci_scsi.id0lun0.bootdev > => bind scsi 0 scsi_blk > => dm tree > Class Index Probed DriverName > --- > root 0 [ + ] root_driver root_driver > pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > ahci 0 [ + ] ahci_pci | `-- ahci_pci > scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > bootdev 2 [ ] scsi_bootdev | |-- > ahci_scsi.id0lun0.bootdev > blk 0 [ ] scsi_blk | `-- scsi_blk > => part list scsi 0 > dev_get_uclass_plat: null device > "Synchronous Abort" handler, esr 0x9604, far 0xd503201f1472 > elr: 0003392c lr : 00033924 (reloc) > elr: 7ff1492c lr : 7ff14924 > x0 : d503201f140a x1 : > x2 : 0090 x3 : 0013 > x4 : 7eda0419 x5 : 0020 > x6 : ffd0 x7 : 7eda06b0 > x8 : 0010 x9 : > x10: 0006 x11: 0001869f > x12: 0016 x13: 0004 > x14: x15: 7eda073c > x16: 7ff2547c x17: > x18: 7eea0db0 x19: > x20: x21: 0001 > x22: x23: 7eda0740 > x24: x25: > x26: x27: > x28: x29: 7eda06d0 > > Code: aa0303f7 94000287 aa0003f4 f9400260 (f9403418) > Resetting CPU ... > > The crash occurs in blk_read(). blk_read() does not check if desc == NULL. > > I would have expected "bind scsi 0 scsi_blk" to create a valid state > where the platform data is set up. > > Best regards > > Heinrich I am not sure what 'scsi bind' does. But binding a blk device needs to be done by its parent device. I suspect the 'bind' command does not know how to set up things correctly. See for example blk_create_device() which does quite a lot of setup. What are you trying to fix / do? Regards, Simon
Re: [BUG] bind command leads to invalid state where plaform data is NULL
Hi Heinrich, On Mon, 19 Jun 2023 at 13:48, Heinrich Schuchardt wrote: > > On 6/19/23 14:37, Simon Glass wrote: > > Hi Heinrich, > > > > On Sun, 18 Jun 2023 at 11:12, Heinrich Schuchardt > > wrote: > >> > >> Hello Simon, > >> > >> from origin/next I build qemu_arm64_defconfig with CONFIG_CMD_BIND=yes. > >> > >> I ran the image with: > >> > >> qemu-system-aarch64 -semihosting \ > >> -machine virt,gic-version=max -accel $(ACCEL) -m 1G -smp cores=2 \ > >> -bios u-boot.bin -cpu $(CPU) -nographic -gdb tcp::1234 \ > >> -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \ > >> -drive if=none,file=arm64.img,format=raw,id=mydisk \ > >> -drive if=pflash,format=raw,index=1,file=envstore.img \ > >> -device virtio-rng-pci \ > >> -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0 > >> > >> => scsi bind > >> => dm tree > >>Class Index Probed DriverName > >> --- > >>root 0 [ + ] root_driver root_driver > >>pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > >>pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > >>ahci 0 [ + ] ahci_pci | `-- ahci_pci > >>scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > >>blk 0 [ + ] scsi_blk | |-- > >> ahci_scsi.id0lun0 > >>partition 0 [ + ] blk_partition | | |-- > >> ahci_scsi.id0lun0:1 > >>partition 1 [ + ] blk_partition | | `-- > >> ahci_scsi.id0lun0:15 > >>bootdev 2 [ ] scsi_bootdev | `-- > >> ahci_scsi.id0lun0.bootdev > >> => unbind blk 0 > >> => dm tree > >>Class Index Probed DriverName > >> --- > >>root 0 [ + ] root_driver root_driver > >>pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > >>pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > >>ahci 0 [ + ] ahci_pci | `-- ahci_pci > >>scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > >>bootdev 2 [ ] scsi_bootdev | `-- > >> ahci_scsi.id0lun0.bootdev > >> => bind scsi 0 scsi_blk > >> => dm tree > >>Class Index Probed DriverName > >> --- > >>root 0 [ + ] root_driver root_driver > >>pci 0 [ + ] pci_generic_ecam |-- pcie@1000 > >>pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 > >>ahci 0 [ + ] ahci_pci | `-- ahci_pci > >>scsi 0 [ + ] ahci_scsi | `-- ahci_scsi > >>bootdev 2 [ ] scsi_bootdev | |-- > >> ahci_scsi.id0lun0.bootdev > >>blk 0 [ ] scsi_blk | `-- scsi_blk > >> => part list scsi 0 > >> dev_get_uclass_plat: null device > >> "Synchronous Abort" handler, esr 0x9604, far 0xd503201f1472 > >> elr: 0003392c lr : 00033924 (reloc) > >> elr: 7ff1492c lr : 7ff14924 > >> x0 : d503201f140a x1 : > >> x2 : 0090 x3 : 0013 > >> x4 : 7eda0419 x5 : 0020 > >> x6 : ffd0 x7 : 7eda06b0 > >> x8 : 0010 x9 : > >> x10: 0006 x11: 0001869f > >> x12: 0016 x13: 0004 > >> x14: x15: 7eda073c > >> x16: 7ff2547c x17: > >> x18: 7eea0db0 x19: > >> x20: x21: 0001 > >> x22: x23: 7eda0740 > >> x24: x25: > >> x26: x27: > >> x28: x29: 7eda06d0 > >> > >> Code: aa0303f7 94000287 aa0003f4 f9400260 (f9403418) > >> Resetting CPU ... > >> > >> The crash occurs in blk_read(). blk_read() does not check if desc == NULL. > >> > >> I would have expected "bind scsi 0 scsi_blk" to create a valid state > >> where the platform data is set up. > >> > >> Best regards > >> > >> Heinrich > > > > I am not sure what 'scsi bind' does. > > > > But binding a blk device needs to be done by its parent device. I > > suspect the 'bind' command does not know how to set up things > > correctly. See for example blk_create_device() which does quite a lot > > of setup. > > > > What are you trying to fix / do? > > I just wanted to have an example for creating a man-page for the bind > command. Unfortunately new commands were created without man-page. > > I have an example for " ". Do you have a better > example for "bind "? I suggest a simple device that needs no s
Re: [BUG] bind command leads to invalid state where plaform data is NULL
On 6/19/23 14:37, Simon Glass wrote: Hi Heinrich, On Sun, 18 Jun 2023 at 11:12, Heinrich Schuchardt wrote: Hello Simon, from origin/next I build qemu_arm64_defconfig with CONFIG_CMD_BIND=yes. I ran the image with: qemu-system-aarch64 -semihosting \ -machine virt,gic-version=max -accel $(ACCEL) -m 1G -smp cores=2 \ -bios u-boot.bin -cpu $(CPU) -nographic -gdb tcp::1234 \ -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \ -drive if=none,file=arm64.img,format=raw,id=mydisk \ -drive if=pflash,format=raw,index=1,file=envstore.img \ -device virtio-rng-pci \ -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0 => scsi bind => dm tree Class Index Probed DriverName --- root 0 [ + ] root_driver root_driver pci 0 [ + ] pci_generic_ecam |-- pcie@1000 pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 ahci 0 [ + ] ahci_pci | `-- ahci_pci scsi 0 [ + ] ahci_scsi | `-- ahci_scsi blk 0 [ + ] scsi_blk | |-- ahci_scsi.id0lun0 partition 0 [ + ] blk_partition | | |-- ahci_scsi.id0lun0:1 partition 1 [ + ] blk_partition | | `-- ahci_scsi.id0lun0:15 bootdev 2 [ ] scsi_bootdev | `-- ahci_scsi.id0lun0.bootdev => unbind blk 0 => dm tree Class Index Probed DriverName --- root 0 [ + ] root_driver root_driver pci 0 [ + ] pci_generic_ecam |-- pcie@1000 pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 ahci 0 [ + ] ahci_pci | `-- ahci_pci scsi 0 [ + ] ahci_scsi | `-- ahci_scsi bootdev 2 [ ] scsi_bootdev | `-- ahci_scsi.id0lun0.bootdev => bind scsi 0 scsi_blk => dm tree Class Index Probed DriverName --- root 0 [ + ] root_driver root_driver pci 0 [ + ] pci_generic_ecam |-- pcie@1000 pci_generi0 [ ] pci_generic_drv | |-- pci_0:0.0 ahci 0 [ + ] ahci_pci | `-- ahci_pci scsi 0 [ + ] ahci_scsi | `-- ahci_scsi bootdev 2 [ ] scsi_bootdev | |-- ahci_scsi.id0lun0.bootdev blk 0 [ ] scsi_blk | `-- scsi_blk => part list scsi 0 dev_get_uclass_plat: null device "Synchronous Abort" handler, esr 0x9604, far 0xd503201f1472 elr: 0003392c lr : 00033924 (reloc) elr: 7ff1492c lr : 7ff14924 x0 : d503201f140a x1 : x2 : 0090 x3 : 0013 x4 : 7eda0419 x5 : 0020 x6 : ffd0 x7 : 7eda06b0 x8 : 0010 x9 : x10: 0006 x11: 0001869f x12: 0016 x13: 0004 x14: x15: 7eda073c x16: 7ff2547c x17: x18: 7eea0db0 x19: x20: x21: 0001 x22: x23: 7eda0740 x24: x25: x26: x27: x28: x29: 7eda06d0 Code: aa0303f7 94000287 aa0003f4 f9400260 (f9403418) Resetting CPU ... The crash occurs in blk_read(). blk_read() does not check if desc == NULL. I would have expected "bind scsi 0 scsi_blk" to create a valid state where the platform data is set up. Best regards Heinrich I am not sure what 'scsi bind' does. But binding a blk device needs to be done by its parent device. I suspect the 'bind' command does not know how to set up things correctly. See for example blk_create_device() which does quite a lot of setup. What are you trying to fix / do? I just wanted to have an example for creating a man-page for the bind command. Unfortunately new commands were created without man-page. I have an example for " ". Do you have a better example for "bind "? Best regards Heinrich