On 23.10.2018 11:17, Martin Fuzzey wrote:
> Hi Eugen,
>
> On 23/10/18 09:05, eugen.hris...@microchip.com wrote:
>>
>> On 22.10.2018 19:31, Martin Fuzzey wrote:
>>> When the "w1 bus" command is used with no bus master present
>>> a data abort may occur.
>>>
>>> This is because uclass_first_device() returns zero, but sets the output
>>> struct udevice pointer to NULL in the no device found case.
>>>
>>> Fix w1_get_bus() to account for this and return an error code
>>> as is expected by the callers.
>>>
>>> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
>>> ---
>>> drivers/w1/w1-uclass.c | 14 ++++++++------
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/w1/w1-uclass.c b/drivers/w1/w1-uclass.c
>>> index aecf7fe..cb41b68 100644
>>> --- a/drivers/w1/w1-uclass.c
>>> +++ b/drivers/w1/w1-uclass.c
>>> @@ -115,17 +115,19 @@ int w1_get_bus(int busnum, struct udevice **busp)
>>> struct udevice *dev;
>>> for (ret = uclass_first_device(UCLASS_W1, &dev);
>>> - !ret;
>>> - uclass_next_device(&dev), i++) {
>>> - if (ret) {
>>> - debug("Cannot find w1 bus %d\n", busnum);
>>> - return ret;
>>> - }
>>> + dev && !ret;
>>> + ret = uclass_next_device(&dev), i++) {
>>> if (i == busnum) {
>>> *busp = dev;
>>> return 0;
>>> }
>>> }
>>> +
>>> + if (!ret) {
>> Hi Martin,
>>
>> Does this mean that if ret != 0 , we had errors, but we are not printing
>> this debug message ? Perhaps we should print out here the debug error
>> regardless of the ret value ? Since we exited with a return 0 if we did
>> find the proper bus.
>>
>
> With the current code ret != 0 means that a probe error occurred, not
> that the bus cannot be found,
> if no errors occurred but the requested bus did not exist (either
> because there were no busses at all or
> the index requested was too big) ret from uclass_first|next_device() == 0
>
> In the error case "Cannot find w1 bus %d" is probably not the correct
> error message.
> I would have expected the failing driver to have printed a more explicit
> error message.
> So that is why I only print the debug message if ret == 0 but the device
> was not found.
Good for me.
In cmd/w1.c there is an error message printed regardless of the ret, so,
return -ENODEV here if there is no dev is a good way of fixing it.
Reviewed-by: Eugen Hristev <eugen.hris...@microchip.com>
Thanks again for your fix
>
>> May I also ask on which board setup you tested this ? And which
>> defconfig.
>
> Tested on an i.MX53 based custom board.
> I am working on the W1 host controller driver for i.MX53 and plan to
> submit it soon.
>
> I ran into this problem by accident when I forgot to add the DT entry
> for my new driver.
Great news, hope that the w1 framework implemented here will help you
get on track easily.
>
> Regards,
>
> Martin
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
