Re: [PATCH v4] platform/x86: dell-uart-backlight: Bind through normal serdev matching

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] platform/x86: dell-uart-backlight: use of nonexistent serdev modalias API
--

commit 6fdb948003156a6919ba6eb5db62dfc9148ff2fc
Author: ChunAn Wu <an.wu@canonical.com>

platform/x86: dell-uart-backlight: Bind through normal serdev matching

This patch converts dell-uart-backlight from manual device_driver_attach()
binding to serdev id_table and modalias matching via serdev_device_add().

> diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
> index f323a667dc2d2..9b8ef91bb0a20 100644
> --- a/drivers/platform/x86/dell/dell-uart-backlight.c
> +++ b/drivers/platform/x86/dell/dell-uart-backlight.c
> @@ -325,11 +325,17 @@ static int dell_uart_bl_serdev_probe(struct serdev_device *serdev)
>  	return PTR_ERR_OR_ZERO(dell_bl->bl);
>  }
>  
> +static const struct serdev_device_id dell_uart_bl_serdev_id[] = {
> +	{ KBUILD_MODNAME },
> +	{ }
> +};
> +
>  static struct serdev_device_driver dell_uart_bl_serdev_driver = {
>  	.probe = dell_uart_bl_serdev_probe,
>  	.driver = {
>  		.name = KBUILD_MODNAME,
>  	},
> +	.id_table = dell_uart_bl_serdev_id,
>  };
>  
>  static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
> @@ -354,7 +360,7 @@ static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
>  	if (!serdev)
>  		return -ENOMEM;
>  
> -	ret = serdev_device_add(serdev);
> +	ret = serdev_device_add(serdev, KBUILD_MODNAME);
>  	if (ret) {
>  		dev_err(&pdev->dev, "error %d adding serdev\n", ret);
>  		serdev_device_put(serdev);
> @@ -365,20 +371,10 @@ static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_remove_serdev;
>  
> -	/*
> -	 * serdev device <-> driver matching relies on OF or ACPI matches and
> -	 * neither is available here, manually bind the driver.
> -	 */
> -	ret = device_driver_attach(&dell_uart_bl_serdev_driver.driver, &serdev->dev);
> -	if (ret)
> -		goto err_unregister_serdev_driver;

[Severity: High]
Does this depend on a prerequisite patch adding modalias and id_table support
to the serdev core?

When building with CONFIG_DELL_UART_BACKLIGHT enabled, compilation fails in
drivers/platform/x86/dell/dell-uart-backlight.c because:

1. dell_uart_bl_serdev_id uses struct serdev_device_id, which is not defined
   in include/linux/serdev.h or include/linux/mod_devicetable.h.

2. dell_uart_bl_serdev_driver initializes .id_table, which does not exist in
   include/linux/serdev.h:

struct serdev_device_driver {
	struct device_driver driver;
	int	(*probe)(struct serdev_device *);
	void	(*remove)(struct serdev_device *);
	void	(*shutdown)(struct serdev_device *);
};

3. dell_uart_bl_pdev_probe() passes two arguments to serdev_device_add(),
   whereas drivers/tty/serdev/core.c defines it with a single parameter:

int serdev_device_add(struct serdev_device *serdev)

In addition, serdev_device_match() in drivers/tty/serdev/core.c only matches
via ACPI and OF:

static int serdev_device_match(struct device *dev, const struct device_driver *drv)
{
	if (!is_serdev_device(dev))
		return 0;

	/* TODO: platform matching */
	if (acpi_driver_match_device(dev, drv))
		return 1;

	return of_driver_match_device(dev, drv);
}

Without serdev id_table matching in the core, removing device_driver_attach()
from dell_uart_bl_pdev_probe() also leaves dell_uart_bl_serdev_driver unable
to bind when probing Dell UART backlight systems.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024022930.2704030-3-an.wu@canonical.com?part=1