Re: [PATCH v2 1/2] serdev: Split and export serdev_acpi_get_uart_resource()

Andy Shevchenko
2021-08-05T08:21:44Z

On Thu, Aug 5, 2021 at 10:36 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> On 04. 08. 21, 16:29, Andy Shevchenko wrote:

...

> > +/**
> > + * serdev_acpi_get_uart_resource - Gets UARTSerialBus resource if type matches
> > + * @ares: ACPI resource
> > + * @uart: Pointer to UARTSerialBus resource will be returned here
> > + *
> > + * Checks if the given ACPI resource is of type UARTSerialBus.
> > + * In this case, returns a pointer to it to the caller.
> > + *
> > + * Returns true if resource type is of UARTSerialBus, otherwise false.
>
> Better to write:
> * Return: True if resource type is of UARTSerialBus, otherwise false.
> which is recognized by sphinx.

Will fix it in v3.

> > + */

...

> Why don't you return NULL, or sb, thus eliminating the parameter?

1. That's how other similar APIs are done.
2. It will save a line of code in the callers. Usual pattern
if (...get_res(..., &sb))
return ERR_or_so;

With your proposal

sb = get_res(...);
if (!sb)
return ERR_or_so;

> > +}

--
With Best Regards,
Andy Shevchenko