clarifying i2c_get_chip_for_busnum()

[Quentin Schulz]

Hi Simon,

On 8/21/24 4:11 AM, Simon Glass wrote:
> Hi,
> 
> On Tue, 20 Aug 2024 at 07:34, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>
>> 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).
> 
> That is pretty common in driver model. Few things return a positive
> value. Mostly they return 0 on success or a -ve error code.
> 
> Note though that *devp is generally not assigned if there is an error.
> It is left unset, so it could be something like:
> 
> @devp: Returns pointer to new device on success
> 

I assume you're suggesting

```
* @devp:   Returns pointer to new device on success
* Return:  0 on success, -ve on failure to find bus or chip
```

?

It isn't clear to me what's the path forward with this patch, hence the 
question.

Cheers,
Quentin