[U-Boot] [PATCH] w1: fix data abort if no one wire bus master present

[Martin Fuzzey]

Hi Eugen,

On 23/10/18 09:05, Eugen.Hristev at 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 at 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.

> 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.

Regards,

Martin