| Message ID | 20210804142959.67981-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] serdev: Split and export serdev_acpi_get_uart_resource() | expand |
On 04. 08. 21, 16:29, Andy Shevchenko wrote: > The same as for I²C Serial Bus resource split and export > serdev_acpi_get_uart_resource(). We have already a few users > one of which is converted here. > > Rationale of this is to consolidate parsing UART Serial Bus > resource in one place as it's done, e.g., for I²C Serial Bus. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > --- > v2: added Rb tag (Hans) > drivers/tty/serdev/core.c | 36 +++++++++++++++++++++++++++++------- > include/linux/serdev.h | 14 ++++++++++++++ > 2 files changed, 43 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > index 92498961fd92..436e3d1ba92c 100644 > --- a/drivers/tty/serdev/core.c > +++ b/drivers/tty/serdev/core.c > @@ -562,23 +562,45 @@ struct acpi_serdev_lookup { > int index; > }; > > +/** > + * 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. > + */ > +bool serdev_acpi_get_uart_resource(struct acpi_resource *ares, > + struct acpi_resource_uart_serialbus **uart) > +{ > + struct acpi_resource_uart_serialbus *sb; > + > + if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) > + return false; > + > + sb = &ares->data.uart_serial_bus; > + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_UART) > + return false; > + > + *uart = sb; > + return true; Why don't you return NULL, or sb, thus eliminating the parameter? > +} > +EXPORT_SYMBOL_GPL(serdev_acpi_get_uart_resource); thanks, -- js suse labs
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
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index 92498961fd92..436e3d1ba92c 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -562,23 +562,45 @@ struct acpi_serdev_lookup { int index; }; +/** + * 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. + */ +bool serdev_acpi_get_uart_resource(struct acpi_resource *ares, + struct acpi_resource_uart_serialbus **uart) +{ + struct acpi_resource_uart_serialbus *sb; + + if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) + return false; + + sb = &ares->data.uart_serial_bus; + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_UART) + return false; + + *uart = sb; + return true; +} +EXPORT_SYMBOL_GPL(serdev_acpi_get_uart_resource); + static int acpi_serdev_parse_resource(struct acpi_resource *ares, void *data) { struct acpi_serdev_lookup *lookup = data; struct acpi_resource_uart_serialbus *sb; acpi_status status; - if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) - return 1; - - if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART) + if (!serdev_acpi_get_uart_resource(ares, &sb)) return 1; if (lookup->index != -1 && lookup->n++ != lookup->index) return 1; - sb = &ares->data.uart_serial_bus; - status = acpi_get_handle(lookup->device_handle, sb->resource_source.string_ptr, &lookup->controller_handle); @@ -586,7 +608,7 @@ static int acpi_serdev_parse_resource(struct acpi_resource *ares, void *data) return 1; /* - * NOTE: Ideally, we would also want to retreive other properties here, + * NOTE: Ideally, we would also want to retrieve other properties here, * once setting them before opening the device is supported by serdev. */ diff --git a/include/linux/serdev.h b/include/linux/serdev.h index 9f14f9c12ec4..3368c261ab62 100644 --- a/include/linux/serdev.h +++ b/include/linux/serdev.h @@ -327,4 +327,18 @@ static inline int serdev_tty_port_unregister(struct tty_port *port) } #endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */ +struct acpi_resource; +struct acpi_resource_uart_serialbus; + +#ifdef CONFIG_ACPI +bool serdev_acpi_get_uart_resource(struct acpi_resource *ares, + struct acpi_resource_uart_serialbus **uart); +#else +static inline bool serdev_acpi_get_uart_resource(struct acpi_resource *ares, + struct acpi_resource_uart_serialbus **uart) +{ + return false; +} +#endif /* CONFIG_ACPI */ + #endif /*_LINUX_SERDEV_H */