linux-kernel - Re: [PATCH 2/2] iio: light: bh1750: Add device tree support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180921013721.GA4268@r>
Date:   Thu, 20 Sep 2018 21:37:21 -0400
From:   r yang <decatf@...il.com>
To:     Tomasz Duszynski <tduszyns@...il.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: light: bh1750: Add device tree support

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@...il.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
> >