clarifying i2c_get_chip_for_busnum()

[Quentin Schulz]

Hi,

On 8/20/24 3:01 PM, Sahaj Sarup wrote:
> [You don't often get email from sahaj.sarup at linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Tue, 20 Aug 2024 at 17:21, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>
>> Hi,
>>
>> On 8/20/24 11:12 AM, Sahaj Sarup wrote:
>>> [You don't often get email from sahaj.sarup at linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Hi,
>>>
>>> In `include/i2c.h` , the udevice pointer and return value definition
>>> seems to be confusing.
>>>
>>> ```
>>> /**
>>>    * i2c_get_chip_for_busnum() - get a device to use to access a chip on
>>> .
>>> .
>>> .
>>>    * @devp: Returns pointer to new device if found or -ENODEV if not
>>>    * found
>>>    */
>>> ```
>>>
>>> Should this instead be:
>>>
>>> ```
>>>    * @devp:   Returns pointer to new device or NULL if not found
>>>    * Return:  0 on success, -ENODEV on failure
>>> ```
>>>
>>
>> For the @devp part, seems like it as uclass_get_device_by_seq sets it to
>> NULL and i2c_get_chip only modifies it when a device is found.
>>
>> For the return part... not sure. We don't overwrite the return value we
>> get from functions we call, so not sure we can guarantee that only
>> ENODEV will be returned?
> 
> make sense, devp can't return -ENODEV, ofc, and Return returns 0 or -ve.
> So it can be closer to:
> 
> ```
> * @devp:   Returns pointer to new device or NULL if chip is not found
> * Return:  0 on success, -ve on failure to find bus or chip
> ```
> 

Nothing guarantees we'll return a negative value either though, c.f. the 
if conditions in i2c_get_chip_for_busnum(), where we only check for ret 
(i.e. ret != 0).

Cheers,
Quentin