Re: [PATCH 5/8] platform: add platform_get_device_match_data() helper

Jonathan Cameron
2024-08-17T15:19:09Z

On Thu, 15 Aug 2024 12:11:59 +0000
Guillaume Stols <gstols@baylibre.com> wrote:

> Inspired from spi_get_device_match_data() helper: if OF or ACPI driver
> data is available, platform_get_device_id will return NULL because
> platform_match_id is not called, and pdev->id_entry is never populated.
> This helper function checks if match data is available, and otherwise
> returns the ID table data.

Likely no one will read the patch description but for these it's worth
some scary text to point out that the driver_data must be a pointer.
If people use an enum we need to do a dance to avoid the value 0.

We get quite a few buggy conversions of i2c/spi drivers as a result
of that little gotcha.

Otherwise this seems like a sensible addition to me.

Potentially worth pulling out as a precursor series with a few examples
of how it saves on repeating this same code block in a bunch
of existing drivers. Up to GregKH I think for how he'd like this
(assuming he approves!)

Jonathan

>
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> ---
> drivers/base/platform.c | 12 ++++++++++++
> include/linux/platform_device.h | 1 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4c3ee6521ba5..26e9a026eb05 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1090,6 +1090,18 @@ static const struct platform_device_id *platform_match_id(
> return NULL;
> }
>
> +const void *platform_get_device_match_data(const struct platform_device *pdev)
> +{
> + const struct platform_device_id *match;
> +
> + match = device_get_match_data(&pdev->dev);
> + if (match)
> + return match;
> +
> + return (const void *)platform_get_device_id(pdev)->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(platform_get_device_match_data);
> +
> #ifdef CONFIG_PM_SLEEP
>
> static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index d422db6eec63..e2cc09ecc447 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -45,6 +45,7 @@ struct platform_device {
> };
>
> #define platform_get_device_id(pdev) ((pdev)->id_entry)
> +extern const void *platform_get_device_match_data(const struct platform_device *pdev);
>
> #define dev_is_platform(dev) ((dev)->bus == &platform_bus_type)
> #define to_platform_device(x) container_of((x), struct platform_device, dev)
>