Hi Quentin, On Wed, 21 Aug 2024 at 02:19, Quentin Schulz <[email protected]> wrote: > > 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
Yes that's right. Regards, Simon
