Re: [BUG] bind command leads to invalid state where plaform data is NULL

2023-06-20 Thread Simon Glass
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

2023-06-19 Thread Heinrich Schuchardt

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

2023-06-19 Thread Simon Glass
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

2023-06-19 Thread Simon Glass
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 

Re: [BUG] bind command leads to invalid state where plaform data is NULL

2023-06-19 Thread Heinrich Schuchardt

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


[BUG] bind command leads to invalid state where plaform data is NULL

2023-06-18 Thread Heinrich Schuchardt

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