Re: [PATCH 2/2] spi: spidev: Fix OF tree warning logic

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

On Wed, Sep 19, 2018 at 12:50 AM Trent Piepho <tpiepho@xxxxxxxxxx> wrote:
> spidev will make a big fuss if a device tree node binds a device by
> using "spidev" as the node's compatible property.
>
> However, the logic for this isn't looking for "spidev" in the
> compatible, but rather checking that the device is NOT compatible with
> spidev's list of devices.
>
> This causes a false positive if a device not named "rohm,dh2228fv", etc.
> binds to spidev, even if a means other than putting "spidev" in the
> device tree was used.  E.g., the sysfs driver_override attribute.
>
> Signed-off-by: Trent Piepho <tpiepho@xxxxxxxxxx>

Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -724,10 +724,11 @@ static int spidev_probe(struct spi_device *spi)
>          * compatible string, it is a Linux implementation thing
>          * rather than a description of the hardware.
>          */
> -       if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) {
> +       if (spi->dev.of_node &&
> +           of_device_is_compatible(spi->dev.of_node, "spidev")) {
>                 dev_err(&spi->dev, "buggy DT: spidev listed directly in DT\n");
>                 WARN_ON(spi->dev.of_node &&
> -                       !of_match_device(spidev_dt_ids, &spi->dev));
> +                       of_device_is_compatible(spi->dev.of_node, "spidev"));

To avoid having the same conditional twice, perhaps the dev_err() and WARN_ON()
should just be replaced by

     WARN(1, "%pOF: buggy DT: spidev listed directly in DT\n",
spi->dev.of_node);

>         }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds