All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jeff LaBundy <jeff@labundy.com>
Cc: Roy Im <roy.im.opensource@diasemi.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Input: da7280 - protect OF match table with CONFIG_OF
Date: Sun, 3 Jan 2021 17:58:41 -0800	[thread overview]
Message-ID: <X/J2Ud24zs7ZJdTU@google.com> (raw)
In-Reply-To: <20201220020109.GA13501@labundy.com>

Hi Jeff,

On Sat, Dec 19, 2020 at 08:01:09PM -0600, Jeff LaBundy wrote:
> Hi Dmitry,
> 
> On Fri, Dec 18, 2020 at 04:49:48PM +0000, Roy Im wrote:
> > On Friday, December 18, 2020 3:50 PM, Dmitry Torokhov wrote:
> > 
> > > The OF match table is only used when OF is enabled.
> > > 
> > > Fixes: cd3f609823a5 ("Input: new da7280 haptic driver")
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > >  drivers/input/misc/da7280.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c index 2f698a8c1d65..b08610d6e575 100644
> > > --- a/drivers/input/misc/da7280.c
> > > +++ b/drivers/input/misc/da7280.c
> > > @@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
> > >  	return retval;
> > >  }
> > > 
> > > +#ifdef CONFIG_OF
> > >  static const struct of_device_id da7280_of_match[] = {
> > >  	{ .compatible = "dlg,da7280", },
> > >  	{ }
> > >  };
> > >  MODULE_DEVICE_TABLE(of, da7280_of_match);
> > > +#endif
> 
> Just for my own understanding, would it not work just as well
> to include of_device.h? This includes mod_devicetable.h which
> in turn defines the of_device_id struct (even if CONFIG_OF is
> not set).

The issue here is not that the structure is undefined, but the variable
is unused. We could also fix this by not using of_match_ptr() when
assigning the match table to the driver structure, making the variable
referenced even if CONFIG_OF is off.

> 
> The reason for asking is because it seems many drivers do not
> include these guards.

It could be that they are either only compiled with OF, or they decided
it is not worth saving a few bytes, or maybe they are used on ACPI-based
systems with PRP0001 bindings in which case the match table in the
driver might still be needed.

Thanks.

-- 
Dmitry

  reply	other threads:[~2021-01-04  1:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18  6:50 [PATCH] Input: da7280 - protect OF match table with CONFIG_OF Dmitry Torokhov
2020-12-18 16:49 ` Roy Im
2020-12-20  2:01   ` Jeff LaBundy
2021-01-04  1:58     ` Dmitry Torokhov [this message]
2021-01-04  4:03       ` Jeff LaBundy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=X/J2Ud24zs7ZJdTU@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=jeff@labundy.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roy.im.opensource@diasemi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.