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

[Trent Piepho <tpiepho@xxxxxxxxxx>]

On Wed, 2018-09-19 at 23:17 +0200, Geert Uytterhoeven wrote:
> Hi Trent,
> 
> On Wed, Sep 19, 2018 at 10:54 PM Trent Piepho <tpiepho@xxxxxxxxxx> wrote:
> > On Wed, 2018-09-19 at 10:39 -0700, Mark Brown wrote:
> > > On Wed, Sep 19, 2018 at 08:23:09AM +0200, Geert Uytterhoeven wrote:
> > > > On Wed, Sep 19, 2018 at 12:50 AM Trent Piepho <tpiepho@xxxxxxxxxx> wrote:
> > > > > +       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);
> > > 
> > > Yes, that'd be neater.
> > 
> > How about:
> > 
> > -       if (spi->dev.of_node && !of_match_device(spidev_dt_ids, &spi->dev)) {
> > -               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));
> > -       }
> > +       WARN_ON(spi->dev.of_node &&
> > +               of_device_is_compatible(spi->dev.of_node, "spidev"),
> > +               "%pOF: buggy DT: spidev listed directly in DT\n", spi->dev.of_node);
> 
> s/WARN_ON/WARN/?
> 
> The former doesn't take a message, the latter does.

Yes, that's what I meant.  If the dev_err is to be removed, no reason
to use WARN(1, ) instead of putting the condition in the WARN.