Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030815AbbDWTrd (ORCPT ); Thu, 23 Apr 2015 15:47:33 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:35622 "EHLO mail-lb0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030581AbbDWTr3 (ORCPT ); Thu, 23 Apr 2015 15:47:29 -0400 Date: Thu, 23 Apr 2015 21:42:17 +0200 From: Tomasz Duszynski To: Daniel Baluta Cc: Tomasz Duszynski , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Linux Kernel Mailing List , "linux-iio@vger.kernel.org" Subject: Re: [PATCH] iio: light: add support for ROHM BH1710/BH1715/BH1721/BH1750/BH1751 ambient light sensors Message-ID: <20150423194217.GA24769@Arch.lan> References: <1429641822-2007-1-git-send-email-tduszyns@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3562 Lines: 94 On Thu, Apr 23, 2015 at 10:30:09AM +0300, Daniel Baluta wrote: > On Tue, Apr 21, 2015 at 9:43 PM, Tomasz Duszynski wrote: > > Add support for ROHM BH1710/BH1715/BH1721/BH1750/BH1751 ambient light > > sensors. > > > > Signed-off-by: Tomasz Duszynski > > --- > > drivers/iio/light/Kconfig | 10 ++ > > drivers/iio/light/Makefile | 1 + > > drivers/iio/light/bh1750.c | 322 +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 333 insertions(+) > > create mode 100644 drivers/iio/light/bh1750.c > > > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > > index 5a3237b..9fb79ca 100644 > > --- a/drivers/iio/light/Kconfig > > +++ b/drivers/iio/light/Kconfig > > @@ -37,6 +37,16 @@ config APDS9300 > > To compile this driver as a module, choose M here: the > > module will be called apds9300. > > > > +config BH1750 > > + tristate "BH1750 ambient light sensor" > > Better use ROHM BH1750 .. here. > > Did git blame on Kconfig and among newest sensors some have this manufacturer prefix while others don't, but still no problem for me to add that. > > +static const struct iio_chan_spec bh1750_channels[] = { > > + { > > + .type = IIO_INTENSITY, > > Shouldn't this be IIO_LIGHT channel type? > > Users will get illuminance after multiplying raw value with scale. > That naming is misleading. Thanks for pointing this out. > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > + BIT(IIO_CHAN_INFO_SCALE) | > > + BIT(IIO_CHAN_INFO_INT_TIME) > > + } > > +}; > > + > > +static int bh1750_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + int ret, usec; > > + struct bh1750_data *data; > > + struct iio_dev *indio_dev; > > + > > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C | > > + I2C_FUNC_SMBUS_WRITE_BYTE)) > > + return -ENODEV; > > + > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + i2c_set_clientdata(client, indio_dev); > > + data->client = client; > > + data->chip_info = &chip_info_tbl[id->driver_data]; > > + > > + usec = data->chip_info->mtreg_to_usec * data->chip_info->mtreg_default; > > + ret = bh1750_change_int_time(data, usec); > > + if (ret < 0) > > + return ret; > > + > > + mutex_init(&data->lock); > > + indio_dev->dev.parent = &client->dev; > > + indio_dev->info = &bh1750_info; > > + indio_dev->name = id->name; > > + indio_dev->channels = bh1750_channels; > > + indio_dev->num_channels = ARRAY_SIZE(bh1750_channels); > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + > > + return devm_iio_device_register(&client->dev, indio_dev); > > You need also to add a remove function where the chip is powered down. Then > you shouldn't use devm_iio_device_register here, because you need to control > the order of chip powerdown/ device unregister operations at remove. > Right. > thanks, > Daniel. Thanks for review! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/