clarifying i2c_get_chip_for_busnum()

[Simon Glass]

Hi Quentin,

On Wed, 21 Aug 2024 at 02:19, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> 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

Yes that's right.

Regards,
Simon