Hi Simon,
On 8/21/24 4:11 AM, Simon Glass wrote:
Hi,
On Tue, 20 Aug 2024 at 07:34, Quentin Schulz <[email protected]> wrote:
Hi,
On 8/20/24 3:01 PM, Sahaj Sarup wrote:
[You don't often get email from [email protected]. Learn why this is
important at https://aka.ms/LearnAboutSenderIdentification ]
On Tue, 20 Aug 2024 at 17:21, Quentin Schulz <[email protected]> wrote:
Hi,
On 8/20/24 11:12 AM, Sahaj Sarup wrote:
[You don't often get email from [email protected]. 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
