From: r yang <decatf@gmail.com>
To: Tomasz Duszynski <tduszyns@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] iio: light: bh1750: Add device tree support
Date: Thu, 20 Sep 2018 21:37:21 -0400 [thread overview]
Message-ID: <20180921013721.GA4268@r> (raw)
In-Reply-To: <20180919192238.GA19391@arch>
On Wed, Sep 19, 2018 at 09:22:39PM +0200, Tomasz Duszynski wrote:
> Comments inline.
>
> On Sat, Sep 15, 2018 at 01:42:14PM -0400, ryang wrote:
> > Add device tree support for ROHM BH1750 series ambient light sensors.
> >
> > Signed-off-by: ryang <decatf@gmail.com>
> > ---
> > drivers/iio/light/bh1750.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c
> > index a814828e69f5..50b599abb383 100644
> > --- a/drivers/iio/light/bh1750.c
> > +++ b/drivers/iio/light/bh1750.c
> > @@ -315,9 +315,19 @@ static const struct i2c_device_id bh1750_id[] = {
> > };
> > MODULE_DEVICE_TABLE(i2c, bh1750_id);
> >
> > +static const struct of_device_id bh1750_of_match[] = {
> > + { .compatible = "rohm,bh1710", },
> > + { .compatible = "rohm,bh1715", },
> > + { .compatible = "rohm,bh1721", },
> > + { .compatible = "rohm,bh1750", },
> > + { .compatible = "rohm,bh1751", },
> > + { }
> > +};
> > +
>
> Hmm, I believe this should be exported additionally using
> MODULE_DEVICE_TABLE(of, bh1750_of_match). Otherwise module loading
> will not work as expected.
>
> Moreover, we can do without DT and if you want to avoid
> warning about unused variables you may consider enclosing everything
> with:
>
> #ifdef CONFIG_OF
> ...
> #endif
The reason for this DT patch is because I've ported the Samsung Galaxy
Tab 10.1 to the mainline kernel. The device is working on the Tegra
platform with the right DTS file. Some peripherals on this device have
existing drivers such as this one which only need the DT bindings for
it be usuable.
>
> > static struct i2c_driver bh1750_driver = {
> > .driver = {
> > .name = "bh1750",
> > + .of_match_table = of_match_ptr(bh1750_of_match),
> > .pm = BH1750_PM_OPS,
>
> I've recently simplified setting pm ops so this would not apply to
> iio/togreg cleanly.
>
Does this mean I should wait until that is merged before submitting a V2
patch?
> > },
> > .probe = bh1750_probe,
> > --
> > 2.17.1
> >
next prev parent reply other threads:[~2018-09-21 1:37 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-15 17:42 [PATCH 1/2] dt-bindings: iio: light: bh1750: Add device tree binding documentation ryang
2018-09-15 17:42 ` [PATCH 2/2] iio: light: bh1750: Add device tree support ryang
2018-09-16 9:45 ` Jonathan Cameron
2018-09-18 1:57 ` r yang
2018-09-22 15:22 ` Jonathan Cameron
2018-09-19 19:22 ` Tomasz Duszynski
2018-09-21 1:37 ` r yang [this message]
2018-09-21 15:34 ` Tomasz Duszynski
2018-09-22 15:20 ` Jonathan Cameron
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=20180921013721.GA4268@r \
--to=decatf@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
--cc=tduszyns@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).