Hi Alvaro,
On 19 April 2017 at 03:18, Álvaro Fernández Rojas <noltari at gmail.com> wrote:
> This is what I think it's going on:
>
> sysreset_walk():
> - calls uclass_first_device():
> - Calls uclass_find_first_device():
> - device is found.
> - ret is set to 0.
> - Calls uclass_get_device_tail():
> - ret == 0 -> doesn't return.
> - dev != null -> assert is true.
> - Calls device_probe():
> - It fails *somewhere* and goes to fail WITHOUT setting dev to NULL.
> - ret != 0 -> returns without setting devp = dev.
> - dev is NOT NULL but device is not probed.
> - Tries to get ops from a not probed device -> Exception.
>
>
> So basically it's a bug related to device_probe not nulling dev when failing
> and sysreset_walk() checking only if the device is not null and not the
> actual return of uclass_first_device() and uclass_next_device()...
OK thanks for the explanation. Yes this is a bug and there are no
tests to catch it.
In this case uclass_first/next_device() should return dev = NULL.
I'll take a look and see if I can create a few patches.
Regards,
Simon
>
>
>
>
> On Wed, Apr 19, 2017 at 7:28 AM +0200, "Álvaro Fernández Rojas"
> <noltari at gmail.com> wrote:
>
>> Hi Simon,
>>
>> El 19/04/2017 a las 2:13, Simon Glass escribió:
>> > Hi Alvaro,
>> >
>> > On 18 April 2017 at 14:38, Álvaro Fernández Rojas wrote:
>> >> This causes exceptions for drivers that aren't probed when reboot is
>> >> requested.
>> >>
>> >> Signed-off-by: Álvaro Fernández Rojas
>> >> ---
>> >> v3: add new patch to ensure that the device is probed
>> >>
>> >> drivers/sysreset/sysreset-uclass.c | 3 +++
>> >> 1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/drivers/sysreset/sysreset-uclass.c
>> >> b/drivers/sysreset/sysreset-uclass.c
>> >> index 3566d17..329dc2e 100644
>> >> --- a/drivers/sysreset/sysreset-uclass.c
>> >> +++ b/drivers/sysreset/sysreset-uclass.c
>> >> @@ -34,6 +34,9 @@ int sysreset_walk(enum sysreset_t type)
>> >> for (uclass_first_device(UCLASS_SYSRESET, &dev);
>> >> dev;
>> >> uclass_next_device(&dev)) {
>> >> + if (!device_active(dev) && device_probe(dev))
>> >> + continue;
>> >
>> > uclass_first_device() should activate the device. Can you please dig
>> > into what is going on here?
>> I'll try to investigate it later today or tomorrow...
>> Could this be related to core/uclass: uclass_get_device_tail: always set
>> devp?
>> http://patchwork.ozlabs.org/patch/751929/
>>
>> >
>> >> +
>> >> ret = sysreset_request(dev, type);
>> >> if (ret == -EINPROGRESS)
>> >> break;
>> >> --
>> >> 2.1.4
>> >>
>> >
>> > Regards,
>> > Simon
>> >
>> Regards,
>> Álvaro.