On Wed, 21 Sep 2022 23:45:20 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> The proposed spi_get_device_match_data() helper is for retrieving
> a driver data associated with the ID in an ID table. First, it tries
> to get driver data of the device enumerated by firmware interface
> (usually Device Tree or ACPI). If none is found it falls back to
> the SPI ID table matching.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
I like this in general, but we need to keep a close eye on usecases
that are introduced to make sure that there is alignment between the
contents of the tables.
Perhaps the text should also explain a bit about why we would allow
both paths (in many case, the fallback will work fine)...
Previously a strong reason for that was that there was nothing to ensure
that all dt table entries were mirrored in the spi_device_id table.
As a side effect of the check to ensure module autoloading works, there
is now a check for that.
My personal view is we should still use the more generic path, even if
magic under the hood puts the data in spi_id->driver_data.
Jonathan
> ---
> drivers/spi/spi.c | 12 ++++++++++++
> include/linux/spi/spi.h | 3 +++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ad254b94308e..a0947d63afbc 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -360,6 +360,18 @@ const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev)
> }
> EXPORT_SYMBOL_GPL(spi_get_device_id);
>
> +const void *spi_get_device_match_data(const struct spi_device *sdev)
> +{
> + const void *match;
> +
> + match = device_get_match_data(&sdev->dev);
> + if (match)
> + return match;
> +
> + return (const void *)spi_get_device_id(sdev)->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(spi_get_device_match_data);
> +
> static int spi_match_device(struct device *dev, struct device_driver *drv)
> {
> const struct spi_device *spi = to_spi_device(dev);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 6ea889df0813..f2565c24ef27 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1510,6 +1510,9 @@ extern void spi_unregister_device(struct spi_device *spi);
> extern const struct spi_device_id *
> spi_get_device_id(const struct spi_device *sdev);
>
> +extern const void *
> +spi_get_device_match_data(const struct spi_device *sdev);
> +
> static inline bool
> spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
> {
