[PATCH v3] iio: light: new driver for the ROHM BH1780

[PATCH v3] iio: light: new driver for the ROHM BH1780

[Linus Walleij]

This is a reimplementation of the old misc device driver for the
ROHM BH1780 ambient light sensor (drivers/misc/bh1780gli.c).

Differences from the old driver:
- Uses the IIO framework
- Uses runtime PM to idle the hardware after 5 seconds
- No weird custom power management from userspace
- No homebrewn values in sysfs

This uses the same (undocumented) device tree compatible-string
as the old driver ("rohm,bh1780gli").

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Daniel Mack <daniel@caiaq.de>
Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Dropped the _scale information just returning "1" from
  sysfs, userspace should assume scale by 1 if not present.
- Dropped CC to maintainers who seemingly fell off the planet.
ChangeLog v1->v2:
- Use <linux/bitops.h>, BIT(), GENMASK()
- Do not read state first when suspending.
- Do not cast debug reg access return value.
- Whitespace, comments.

I want to phase out the old misc driver, but don't know how that
should properly be handled. The driver has an old (totally
undocumented) sysfs ABI for raw measurements, should I add
this as backward-compatibility and symlink the device to the old
location?
---
 drivers/iio/light/Kconfig  |  11 ++
 drivers/iio/light/Makefile |   1 +
 drivers/iio/light/bh1780.c | 276 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 drivers/iio/light/bh1780.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index cfd3df8416bb..5ee1d453b3d8 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -73,6 +73,17 @@ config BH1750
 	 To compile this driver as a module, choose M here: the module will
 	 be called bh1750.
 
+config BH1780
+	tristate "ROHM BH1780 ambient light sensor"
+	depends on I2C
+	depends on !SENSORS_BH1780
+	help
+	 Say Y here to build support for the ROHM BH1780GLI ambient
+	 light sensor.
+
+	 To compile this driver as a module, choose M here: the module will
+	 be called bh1780.
+
 config CM32181
 	depends on I2C
 	tristate "CM32181 driver"
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index b2c31053db0c..4aeee2bd8f49 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_AL3320A)		+= al3320a.o
 obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_APDS9960)		+= apds9960.o
 obj-$(CONFIG_BH1750)		+= bh1750.o
+obj-$(CONFIG_BH1780)		+= bh1780.o
 obj-$(CONFIG_CM32181)		+= cm32181.o
 obj-$(CONFIG_CM3232)		+= cm3232.o
 obj-$(CONFIG_CM3323)		+= cm3323.o
diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
new file mode 100644
index 000000000000..ff6cc55159fd
--- /dev/null
+++ b/drivers/iio/light/bh1780.c
@@ -0,0 +1,276 @@
+/*
+ * ROHM 1780GLI Ambient Light Sensor Driver
+ *
+ * Copyright (C) 2016 Linaro Ltd.
+ * Author: Linus Walleij <linus.walleij@linaro.org>
+ * Loosely based on the previous BH1780 ALS misc driver
+ * Copyright (C) 2010 Texas Instruments
+ * Author: Hemanth V <hemanthv@ti.com>
+ */
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/bitops.h>
+
+#define BH1780_CMD_BIT		BIT(7)
+#define BH1780_REG_CONTROL	0x00
+#define BH1780_REG_PARTID	0x0A
+#define BH1780_REG_MANFID	0x0B
+#define BH1780_REG_DLOW		0x0C
+#define BH1780_REG_DHIGH	0x0D
+
+#define BH1780_REVMASK		GENMASK(3,0)
+#define BH1780_POWMASK		GENMASK(1,0)
+#define BH1780_POFF		(0x0)
+#define BH1780_PON		(0x3)
+
+/* power on settling time in ms */
+#define BH1780_PON_DELAY	2
+/* max time before value available in ms */
+#define BH1780_INTERVAL		250
+
+struct bh1780_data {
+	struct i2c_client *client;
+};
+
+static int bh1780_write(struct bh1780_data *bh1780, u8 reg, u8 val)
+{
+	int ret = i2c_smbus_write_byte_data(bh1780->client,
+					    BH1780_CMD_BIT | reg,
+					    val);
+	if (ret < 0)
+		dev_err(&bh1780->client->dev,
+			"i2c_smbus_write_byte_data failed error "
+			"%d, register %01x\n",
+			ret, reg);
+	return ret;
+}
+
+static int bh1780_read(struct bh1780_data *bh1780, u8 reg)
+{
+	int ret = i2c_smbus_read_byte_data(bh1780->client,
+					   BH1780_CMD_BIT | reg);
+	if (ret < 0)
+		dev_err(&bh1780->client->dev,
+			"i2c_smbus_read_byte_data failed error "
+			"%d, register %01x\n",
+			ret, reg);
+	return ret;
+}
+
+int bh1780_debugfs_reg_access(struct iio_dev *indio_dev,
+			      unsigned int reg, unsigned int writeval,
+			      unsigned int *readval)
+{
+	struct bh1780_data *bh1780 = iio_priv(indio_dev);
+	int ret;
+
+	if (!readval)
+		bh1780_write(bh1780, (u8)reg, (u8)writeval);
+
+	ret = bh1780_read(bh1780, (u8)reg);
+	if (ret < 0)
+		return ret;
+
+	*readval = ret;
+
+	return 0;
+}
+
+static int bh1780_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct bh1780_data *bh1780 = iio_priv(indio_dev);
+	int lsb, msb;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			pm_runtime_get_sync(&bh1780->client->dev);
+			lsb = bh1780_read(bh1780, BH1780_REG_DLOW);
+			if (lsb < 0)
+				return lsb;
+			msb = bh1780_read(bh1780, BH1780_REG_DHIGH);
+			if (msb < 0)
+				return msb;
+			pm_runtime_mark_last_busy(&bh1780->client->dev);
+			pm_runtime_put_autosuspend(&bh1780->client->dev);
+			*val = (msb << 8 | lsb);
+
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = BH1780_INTERVAL * 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info bh1780_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = bh1780_read_raw,
+	.debugfs_reg_access = bh1780_debugfs_reg_access,
+};
+
+static const struct iio_chan_spec bh1780_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_INT_TIME)
+	}
+};
+
+static int bh1780_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int ret;
+	struct bh1780_data *bh1780;
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct iio_dev *indio_dev;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		return -EIO;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*bh1780));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	bh1780 = iio_priv(indio_dev);
+	bh1780->client = client;
+	i2c_set_clientdata(client, bh1780);
+
+	/* Power up the device */
+	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
+	if (ret < 0)
+		return ret;
+	msleep(BH1780_PON_DELAY);
+	pm_runtime_get_noresume(&client->dev);
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+
+	ret = bh1780_read(bh1780, BH1780_REG_PARTID);
+	if (ret < 0)
+		goto out_disable_pm;
+	dev_info(&client->dev,
+		 "Ambient Light Sensor, Rev : %lu\n",
+		 (ret & BH1780_REVMASK));
+
+	/*
+	 * As the device takes 250 ms to even come up with a fresh
+	 * measurement after power-on, do not shut it down unnecessarily.
+	 * Set autosuspend to a five seconds.
+	 */
+	pm_runtime_set_autosuspend_delay(&client->dev, 5000);
+	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_put(&client->dev);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &bh1780_info;
+	indio_dev->name = id->name;
+	indio_dev->channels = bh1780_channels;
+	indio_dev->num_channels = ARRAY_SIZE(bh1780_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto out_disable_pm;
+	return 0;
+
+out_disable_pm:
+	pm_runtime_put_noidle(&client->dev);
+	pm_runtime_disable(&client->dev);
+	return ret;
+}
+
+static int bh1780_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	pm_runtime_get_sync(&client->dev);
+	pm_runtime_put_noidle(&client->dev);
+	pm_runtime_disable(&client->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int bh1780_runtime_suspend(struct device *dev)
+{
+	struct bh1780_data *bh1780;
+	int ret;
+	struct i2c_client *client = to_i2c_client(dev);
+
+	bh1780 = i2c_get_clientdata(client);
+	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_POFF);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int bh1780_runtime_resume(struct device *dev)
+{
+	struct bh1780_data *bh1780;
+	int ret;
+	struct i2c_client *client = to_i2c_client(dev);
+
+	bh1780 = i2c_get_clientdata(client);
+	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
+	if (ret < 0)
+		return ret;
+	/* Wait for power on, then for a value to be available */
+	msleep(BH1780_PON_DELAY + BH1780_INTERVAL);
+
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops bh1780_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(bh1780_runtime_suspend,
+			   bh1780_runtime_resume, NULL)
+};
+
+static const struct i2c_device_id bh1780_id[] = {
+	{ "bh1780", 0 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(i2c, bh1780_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_bh1780_match[] = {
+	{ .compatible = "rohm,bh1780gli", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_bh1780_match);
+#endif
+
+static struct i2c_driver bh1780_driver = {
+	.probe		= bh1780_probe,
+	.remove		= bh1780_remove,
+	.id_table	= bh1780_id,
+	.driver = {
+		.name = "bh1780",
+		.pm = &bh1780_dev_pm_ops,
+		.of_match_table = of_match_ptr(of_bh1780_match),
+	},
+};
+
+module_i2c_driver(bh1780_driver);
+
+MODULE_DESCRIPTION("ROHM BH1780GLI Ambient Light Sensor Driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
-- 
2.4.3

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Jonathan Cameron]

On 30/03/16 09:34, Linus Walleij wrote:
> This is a reimplementation of the old misc device driver for the
> ROHM BH1780 ambient light sensor (drivers/misc/bh1780gli.c).
> 
> Differences from the old driver:
> - Uses the IIO framework
> - Uses runtime PM to idle the hardware after 5 seconds
> - No weird custom power management from userspace
> - No homebrewn values in sysfs
> 
> This uses the same (undocumented) device tree compatible-string
> as the old driver ("rohm,bh1780gli").
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Dropped the _scale information just returning "1" from
>   sysfs, userspace should assume scale by 1 if not present.
> - Dropped CC to maintainers who seemingly fell off the planet.
> ChangeLog v1->v2:
> - Use <linux/bitops.h>, BIT(), GENMASK()
> - Do not read state first when suspending.
> - Do not cast debug reg access return value.
> - Whitespace, comments.
> 
> I want to phase out the old misc driver, but don't know how that
> should properly be handled. The driver has an old (totally
> undocumented) sysfs ABI for raw measurements, should I add
> this as backward-compatibility and symlink the device to the old
> location?

Good question.  I guess we unfortunately do need to jump through the hoops as
who knows what userspace code there is out there using that interface.
At least it is small!  


Other than this, looks good to me.

Jonathan
> ---
>  drivers/iio/light/Kconfig  |  11 ++
>  drivers/iio/light/Makefile |   1 +
>  drivers/iio/light/bh1780.c | 276 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 288 insertions(+)
>  create mode 100644 drivers/iio/light/bh1780.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index cfd3df8416bb..5ee1d453b3d8 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -73,6 +73,17 @@ config BH1750
>  	 To compile this driver as a module, choose M here: the module will
>  	 be called bh1750.
>  
> +config BH1780
> +	tristate "ROHM BH1780 ambient light sensor"
> +	depends on I2C
> +	depends on !SENSORS_BH1780
> +	help
> +	 Say Y here to build support for the ROHM BH1780GLI ambient
> +	 light sensor.
> +
> +	 To compile this driver as a module, choose M here: the module will
> +	 be called bh1780.
> +
>  config CM32181
>  	depends on I2C
>  	tristate "CM32181 driver"
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index b2c31053db0c..4aeee2bd8f49 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_AL3320A)		+= al3320a.o
>  obj-$(CONFIG_APDS9300)		+= apds9300.o
>  obj-$(CONFIG_APDS9960)		+= apds9960.o
>  obj-$(CONFIG_BH1750)		+= bh1750.o
> +obj-$(CONFIG_BH1780)		+= bh1780.o
>  obj-$(CONFIG_CM32181)		+= cm32181.o
>  obj-$(CONFIG_CM3232)		+= cm3232.o
>  obj-$(CONFIG_CM3323)		+= cm3323.o
> diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
> new file mode 100644
> index 000000000000..ff6cc55159fd
> --- /dev/null
> +++ b/drivers/iio/light/bh1780.c
> @@ -0,0 +1,276 @@
> +/*
> + * ROHM 1780GLI Ambient Light Sensor Driver
> + *
> + * Copyright (C) 2016 Linaro Ltd.
> + * Author: Linus Walleij <linus.walleij@linaro.org>
> + * Loosely based on the previous BH1780 ALS misc driver
> + * Copyright (C) 2010 Texas Instruments
> + * Author: Hemanth V <hemanthv@ti.com>
> + */
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/bitops.h>
> +
> +#define BH1780_CMD_BIT		BIT(7)
> +#define BH1780_REG_CONTROL	0x00
> +#define BH1780_REG_PARTID	0x0A
> +#define BH1780_REG_MANFID	0x0B
> +#define BH1780_REG_DLOW		0x0C
> +#define BH1780_REG_DHIGH	0x0D
> +
> +#define BH1780_REVMASK		GENMASK(3,0)
> +#define BH1780_POWMASK		GENMASK(1,0)
> +#define BH1780_POFF		(0x0)
> +#define BH1780_PON		(0x3)
> +
> +/* power on settling time in ms */
> +#define BH1780_PON_DELAY	2
> +/* max time before value available in ms */
> +#define BH1780_INTERVAL		250
> +
> +struct bh1780_data {
> +	struct i2c_client *client;
That's minimal ;)
> +};
> +
> +static int bh1780_write(struct bh1780_data *bh1780, u8 reg, u8 val)
> +{
> +	int ret = i2c_smbus_write_byte_data(bh1780->client,
> +					    BH1780_CMD_BIT | reg,
> +					    val);
> +	if (ret < 0)
> +		dev_err(&bh1780->client->dev,
> +			"i2c_smbus_write_byte_data failed error "
> +			"%d, register %01x\n",
> +			ret, reg);
> +	return ret;
> +}
> +
> +static int bh1780_read(struct bh1780_data *bh1780, u8 reg)
> +{
> +	int ret = i2c_smbus_read_byte_data(bh1780->client,
> +					   BH1780_CMD_BIT | reg);
> +	if (ret < 0)
> +		dev_err(&bh1780->client->dev,
> +			"i2c_smbus_read_byte_data failed error "
> +			"%d, register %01x\n",
> +			ret, reg);
> +	return ret;
> +}
> +
> +int bh1780_debugfs_reg_access(struct iio_dev *indio_dev,
> +			      unsigned int reg, unsigned int writeval,
> +			      unsigned int *readval)
> +{
> +	struct bh1780_data *bh1780 = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!readval)
> +		bh1780_write(bh1780, (u8)reg, (u8)writeval);
> +
> +	ret = bh1780_read(bh1780, (u8)reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	*readval = ret;
> +
> +	return 0;
> +}
> +
> +static int bh1780_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct bh1780_data *bh1780 = iio_priv(indio_dev);
> +	int lsb, msb;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			pm_runtime_get_sync(&bh1780->client->dev);
> +			lsb = bh1780_read(bh1780, BH1780_REG_DLOW);
> +			if (lsb < 0)
> +				return lsb;
> +			msb = bh1780_read(bh1780, BH1780_REG_DHIGH);
> +			if (msb < 0)
> +				return msb;
> +			pm_runtime_mark_last_busy(&bh1780->client->dev);
> +			pm_runtime_put_autosuspend(&bh1780->client->dev);
> +			*val = (msb << 8 | lsb);
> +
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = 0;
> +		*val2 = BH1780_INTERVAL * 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info bh1780_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = bh1780_read_raw,
> +	.debugfs_reg_access = bh1780_debugfs_reg_access,
> +};
> +
> +static const struct iio_chan_spec bh1780_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_INT_TIME)
> +	}
> +};
> +
> +static int bh1780_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct bh1780_data *bh1780;
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	struct iio_dev *indio_dev;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
> +		return -EIO;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*bh1780));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	bh1780 = iio_priv(indio_dev);
> +	bh1780->client = client;
> +	i2c_set_clientdata(client, bh1780);
> +
> +	/* Power up the device */
> +	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
> +	if (ret < 0)
> +		return ret;
> +	msleep(BH1780_PON_DELAY);
> +	pm_runtime_get_noresume(&client->dev);
> +	pm_runtime_set_active(&client->dev);
> +	pm_runtime_enable(&client->dev);
> +
> +	ret = bh1780_read(bh1780, BH1780_REG_PARTID);
> +	if (ret < 0)
> +		goto out_disable_pm;
> +	dev_info(&client->dev,
> +		 "Ambient Light Sensor, Rev : %lu\n",
> +		 (ret & BH1780_REVMASK));
> +
> +	/*
> +	 * As the device takes 250 ms to even come up with a fresh
> +	 * measurement after power-on, do not shut it down unnecessarily.
> +	 * Set autosuspend to a five seconds.
> +	 */
> +	pm_runtime_set_autosuspend_delay(&client->dev, 5000);
> +	pm_runtime_use_autosuspend(&client->dev);
> +	pm_runtime_put(&client->dev);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &bh1780_info;
> +	indio_dev->name = id->name;
> +	indio_dev->channels = bh1780_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(bh1780_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto out_disable_pm;
> +	return 0;
> +
> +out_disable_pm:
> +	pm_runtime_put_noidle(&client->dev);
> +	pm_runtime_disable(&client->dev);
> +	return ret;
> +}
> +
> +static int bh1780_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	pm_runtime_get_sync(&client->dev);
> +	pm_runtime_put_noidle(&client->dev);
> +	pm_runtime_disable(&client->dev);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int bh1780_runtime_suspend(struct device *dev)
> +{
> +	struct bh1780_data *bh1780;
> +	int ret;
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	bh1780 = i2c_get_clientdata(client);
> +	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_POFF);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int bh1780_runtime_resume(struct device *dev)
> +{
> +	struct bh1780_data *bh1780;
> +	int ret;
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	bh1780 = i2c_get_clientdata(client);
> +	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
> +	if (ret < 0)
> +		return ret;
> +	/* Wait for power on, then for a value to be available */
> +	msleep(BH1780_PON_DELAY + BH1780_INTERVAL);
> +
> +	return 0;
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops bh1780_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(bh1780_runtime_suspend,
> +			   bh1780_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id bh1780_id[] = {
> +	{ "bh1780", 0 },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, bh1780_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id of_bh1780_match[] = {
> +	{ .compatible = "rohm,bh1780gli", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, of_bh1780_match);
> +#endif
> +
> +static struct i2c_driver bh1780_driver = {
> +	.probe		= bh1780_probe,
> +	.remove		= bh1780_remove,
> +	.id_table	= bh1780_id,
> +	.driver = {
> +		.name = "bh1780",
> +		.pm = &bh1780_dev_pm_ops,
> +		.of_match_table = of_match_ptr(of_bh1780_match),
> +	},
> +};
> +
> +module_i2c_driver(bh1780_driver);
> +
> +MODULE_DESCRIPTION("ROHM BH1780GLI Ambient Light Sensor Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
> 

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Linus Walleij]

On Sun, Apr 3, 2016 at 11:51 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 30/03/16 09:34, Linus Walleij wrote:

>> I want to phase out the old misc driver, but don't know how that
>> should properly be handled. The driver has an old (totally
>> undocumented) sysfs ABI for raw measurements, should I add
>> this as backward-compatibility and symlink the device to the old
>> location?
>
> Good question.  I guess we unfortunately do need to jump through the hoops as
> who knows what userspace code there is out there using that interface.
> At least it is small!

The maintainer Hemanth seems to have disappeared.

Let's ask some of the OMAP and ex-OMAP people, I think they know
or know who knows...

Tony, Peter, Paul: drivers/misc/bh1780gli.c has a funky sysfs ABI.
It is only used in arch/arm/boot/dts/omap4-sdp.dts as far as I can
tell. What device is this, and more importantly, who is running it
and with what userspace? I'm happy to go and patch that userspace
if I can just get a pointer to whoever is developing it. Sailfish?

Yours,
Linus Walleij

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Peter De Schrijver]

On Sun, Apr 03, 2016 at 06:30:49PM +0200, Linus Walleij wrote:
> On Sun, Apr 3, 2016 at 11:51 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> > On 30/03/16 09:34, Linus Walleij wrote:
> 
> >> I want to phase out the old misc driver, but don't know how that
> >> should properly be handled. The driver has an old (totally
> >> undocumented) sysfs ABI for raw measurements, should I add
> >> this as backward-compatibility and symlink the device to the old
> >> location?
> >
> > Good question.  I guess we unfortunately do need to jump through the hoops as
> > who knows what userspace code there is out there using that interface.
> > At least it is small!
> 
> The maintainer Hemanth seems to have disappeared.
> 
> Let's ask some of the OMAP and ex-OMAP people, I think they know
> or know who knows...
> 
> Tony, Peter, Paul: drivers/misc/bh1780gli.c has a funky sysfs ABI.
> It is only used in arch/arm/boot/dts/omap4-sdp.dts as far as I can
> tell. What device is this, and more importantly, who is running it
> and with what userspace? I'm happy to go and patch that userspace
> if I can just get a pointer to whoever is developing it. Sailfish?

Sailfish has never used OMAP4 afaik. OMAP4 sdp is a TI dev board, so maybe
someone from TI knows what this is for?

Peter.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Linus Walleij]

Looping in Ulf Hansson to double-check the runtime
PM and suspend/resume callbacks.

He doesn't have the original patch so top-posting...

Linus Walleij

On Wed, Mar 30, 2016 at 10:34 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> This is a reimplementation of the old misc device driver for the
> ROHM BH1780 ambient light sensor (drivers/misc/bh1780gli.c).
>
> Differences from the old driver:
> - Uses the IIO framework
> - Uses runtime PM to idle the hardware after 5 seconds
> - No weird custom power management from userspace
> - No homebrewn values in sysfs
>
> This uses the same (undocumented) device tree compatible-string
> as the old driver ("rohm,bh1780gli").
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Dropped the _scale information just returning "1" from
>   sysfs, userspace should assume scale by 1 if not present.
> - Dropped CC to maintainers who seemingly fell off the planet.
> ChangeLog v1->v2:
> - Use <linux/bitops.h>, BIT(), GENMASK()
> - Do not read state first when suspending.
> - Do not cast debug reg access return value.
> - Whitespace, comments.
>
> I want to phase out the old misc driver, but don't know how that
> should properly be handled. The driver has an old (totally
> undocumented) sysfs ABI for raw measurements, should I add
> this as backward-compatibility and symlink the device to the old
> location?
> ---
>  drivers/iio/light/Kconfig  |  11 ++
>  drivers/iio/light/Makefile |   1 +
>  drivers/iio/light/bh1780.c | 276 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 288 insertions(+)
>  create mode 100644 drivers/iio/light/bh1780.c
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index cfd3df8416bb..5ee1d453b3d8 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -73,6 +73,17 @@ config BH1750
>          To compile this driver as a module, choose M here: the module will
>          be called bh1750.
>
> +config BH1780
> +       tristate "ROHM BH1780 ambient light sensor"
> +       depends on I2C
> +       depends on !SENSORS_BH1780
> +       help
> +        Say Y here to build support for the ROHM BH1780GLI ambient
> +        light sensor.
> +
> +        To compile this driver as a module, choose M here: the module will
> +        be called bh1780.
> +
>  config CM32181
>         depends on I2C
>         tristate "CM32181 driver"
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index b2c31053db0c..4aeee2bd8f49 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_AL3320A)           += al3320a.o
>  obj-$(CONFIG_APDS9300)         += apds9300.o
>  obj-$(CONFIG_APDS9960)         += apds9960.o
>  obj-$(CONFIG_BH1750)           += bh1750.o
> +obj-$(CONFIG_BH1780)           += bh1780.o
>  obj-$(CONFIG_CM32181)          += cm32181.o
>  obj-$(CONFIG_CM3232)           += cm3232.o
>  obj-$(CONFIG_CM3323)           += cm3323.o
> diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
> new file mode 100644
> index 000000000000..ff6cc55159fd
> --- /dev/null
> +++ b/drivers/iio/light/bh1780.c
> @@ -0,0 +1,276 @@
> +/*
> + * ROHM 1780GLI Ambient Light Sensor Driver
> + *
> + * Copyright (C) 2016 Linaro Ltd.
> + * Author: Linus Walleij <linus.walleij@linaro.org>
> + * Loosely based on the previous BH1780 ALS misc driver
> + * Copyright (C) 2010 Texas Instruments
> + * Author: Hemanth V <hemanthv@ti.com>
> + */
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/bitops.h>
> +
> +#define BH1780_CMD_BIT         BIT(7)
> +#define BH1780_REG_CONTROL     0x00
> +#define BH1780_REG_PARTID      0x0A
> +#define BH1780_REG_MANFID      0x0B
> +#define BH1780_REG_DLOW                0x0C
> +#define BH1780_REG_DHIGH       0x0D
> +
> +#define BH1780_REVMASK         GENMASK(3,0)
> +#define BH1780_POWMASK         GENMASK(1,0)
> +#define BH1780_POFF            (0x0)
> +#define BH1780_PON             (0x3)
> +
> +/* power on settling time in ms */
> +#define BH1780_PON_DELAY       2
> +/* max time before value available in ms */
> +#define BH1780_INTERVAL                250
> +
> +struct bh1780_data {
> +       struct i2c_client *client;
> +};
> +
> +static int bh1780_write(struct bh1780_data *bh1780, u8 reg, u8 val)
> +{
> +       int ret = i2c_smbus_write_byte_data(bh1780->client,
> +                                           BH1780_CMD_BIT | reg,
> +                                           val);
> +       if (ret < 0)
> +               dev_err(&bh1780->client->dev,
> +                       "i2c_smbus_write_byte_data failed error "
> +                       "%d, register %01x\n",
> +                       ret, reg);
> +       return ret;
> +}
> +
> +static int bh1780_read(struct bh1780_data *bh1780, u8 reg)
> +{
> +       int ret = i2c_smbus_read_byte_data(bh1780->client,
> +                                          BH1780_CMD_BIT | reg);
> +       if (ret < 0)
> +               dev_err(&bh1780->client->dev,
> +                       "i2c_smbus_read_byte_data failed error "
> +                       "%d, register %01x\n",
> +                       ret, reg);
> +       return ret;
> +}
> +
> +int bh1780_debugfs_reg_access(struct iio_dev *indio_dev,
> +                             unsigned int reg, unsigned int writeval,
> +                             unsigned int *readval)
> +{
> +       struct bh1780_data *bh1780 = iio_priv(indio_dev);
> +       int ret;
> +
> +       if (!readval)
> +               bh1780_write(bh1780, (u8)reg, (u8)writeval);
> +
> +       ret = bh1780_read(bh1780, (u8)reg);
> +       if (ret < 0)
> +               return ret;
> +
> +       *readval = ret;
> +
> +       return 0;
> +}
> +
> +static int bh1780_read_raw(struct iio_dev *indio_dev,
> +                          struct iio_chan_spec const *chan,
> +                          int *val, int *val2, long mask)
> +{
> +       struct bh1780_data *bh1780 = iio_priv(indio_dev);
> +       int lsb, msb;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               switch (chan->type) {
> +               case IIO_LIGHT:
> +                       pm_runtime_get_sync(&bh1780->client->dev);
> +                       lsb = bh1780_read(bh1780, BH1780_REG_DLOW);
> +                       if (lsb < 0)
> +                               return lsb;
> +                       msb = bh1780_read(bh1780, BH1780_REG_DHIGH);
> +                       if (msb < 0)
> +                               return msb;
> +                       pm_runtime_mark_last_busy(&bh1780->client->dev);
> +                       pm_runtime_put_autosuspend(&bh1780->client->dev);
> +                       *val = (msb << 8 | lsb);
> +
> +                       return IIO_VAL_INT;
> +               default:
> +                       return -EINVAL;
> +               }
> +       case IIO_CHAN_INFO_INT_TIME:
> +               *val = 0;
> +               *val2 = BH1780_INTERVAL * 1000;
> +               return IIO_VAL_INT_PLUS_MICRO;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static const struct iio_info bh1780_info = {
> +       .driver_module = THIS_MODULE,
> +       .read_raw = bh1780_read_raw,
> +       .debugfs_reg_access = bh1780_debugfs_reg_access,
> +};
> +
> +static const struct iio_chan_spec bh1780_channels[] = {
> +       {
> +               .type = IIO_LIGHT,
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +                                     BIT(IIO_CHAN_INFO_INT_TIME)
> +       }
> +};
> +
> +static int bh1780_probe(struct i2c_client *client,
> +                       const struct i2c_device_id *id)
> +{
> +       int ret;
> +       struct bh1780_data *bh1780;
> +       struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +       struct iio_dev *indio_dev;
> +
> +       if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
> +               return -EIO;
> +
> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*bh1780));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       bh1780 = iio_priv(indio_dev);
> +       bh1780->client = client;
> +       i2c_set_clientdata(client, bh1780);
> +
> +       /* Power up the device */
> +       ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
> +       if (ret < 0)
> +               return ret;
> +       msleep(BH1780_PON_DELAY);
> +       pm_runtime_get_noresume(&client->dev);
> +       pm_runtime_set_active(&client->dev);
> +       pm_runtime_enable(&client->dev);
> +
> +       ret = bh1780_read(bh1780, BH1780_REG_PARTID);
> +       if (ret < 0)
> +               goto out_disable_pm;
> +       dev_info(&client->dev,
> +                "Ambient Light Sensor, Rev : %lu\n",
> +                (ret & BH1780_REVMASK));
> +
> +       /*
> +        * As the device takes 250 ms to even come up with a fresh
> +        * measurement after power-on, do not shut it down unnecessarily.
> +        * Set autosuspend to a five seconds.
> +        */
> +       pm_runtime_set_autosuspend_delay(&client->dev, 5000);
> +       pm_runtime_use_autosuspend(&client->dev);
> +       pm_runtime_put(&client->dev);
> +
> +       indio_dev->dev.parent = &client->dev;
> +       indio_dev->info = &bh1780_info;
> +       indio_dev->name = id->name;
> +       indio_dev->channels = bh1780_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(bh1780_channels);
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +       ret = iio_device_register(indio_dev);
> +       if (ret)
> +               goto out_disable_pm;
> +       return 0;
> +
> +out_disable_pm:
> +       pm_runtime_put_noidle(&client->dev);
> +       pm_runtime_disable(&client->dev);
> +       return ret;
> +}
> +
> +static int bh1780_remove(struct i2c_client *client)
> +{
> +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +       iio_device_unregister(indio_dev);
> +       pm_runtime_get_sync(&client->dev);
> +       pm_runtime_put_noidle(&client->dev);
> +       pm_runtime_disable(&client->dev);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int bh1780_runtime_suspend(struct device *dev)
> +{
> +       struct bh1780_data *bh1780;
> +       int ret;
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       bh1780 = i2c_get_clientdata(client);
> +       ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_POFF);
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int bh1780_runtime_resume(struct device *dev)
> +{
> +       struct bh1780_data *bh1780;
> +       int ret;
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       bh1780 = i2c_get_clientdata(client);
> +       ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_PON);
> +       if (ret < 0)
> +               return ret;
> +       /* Wait for power on, then for a value to be available */
> +       msleep(BH1780_PON_DELAY + BH1780_INTERVAL);
> +
> +       return 0;
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops bh1780_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(bh1780_runtime_suspend,
> +                          bh1780_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id bh1780_id[] = {
> +       { "bh1780", 0 },
> +       { },
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, bh1780_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id of_bh1780_match[] = {
> +       { .compatible = "rohm,bh1780gli", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, of_bh1780_match);
> +#endif
> +
> +static struct i2c_driver bh1780_driver = {
> +       .probe          = bh1780_probe,
> +       .remove         = bh1780_remove,
> +       .id_table       = bh1780_id,
> +       .driver = {
> +               .name = "bh1780",
> +               .pm = &bh1780_dev_pm_ops,
> +               .of_match_table = of_match_ptr(of_bh1780_match),
> +       },
> +};
> +
> +module_i2c_driver(bh1780_driver);
> +
> +MODULE_DESCRIPTION("ROHM BH1780GLI Ambient Light Sensor Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
> --
> 2.4.3
>

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Tony Lindgren]

* Peter De Schrijver <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [160404 00:43]:
> On Sun, Apr 03, 2016 at 06:30:49PM +0200, Linus Walleij wrote:
> > On Sun, Apr 3, 2016 at 11:51 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > On 30/03/16 09:34, Linus Walleij wrote:
> > 
> > >> I want to phase out the old misc driver, but don't know how that
> > >> should properly be handled. The driver has an old (totally
> > >> undocumented) sysfs ABI for raw measurements, should I add
> > >> this as backward-compatibility and symlink the device to the old
> > >> location?
> > >
> > > Good question.  I guess we unfortunately do need to jump through the hoops as
> > > who knows what userspace code there is out there using that interface.
> > > At least it is small!
> > 
> > The maintainer Hemanth seems to have disappeared.
> > 
> > Let's ask some of the OMAP and ex-OMAP people, I think they know
> > or know who knows...
> > 
> > Tony, Peter, Paul: drivers/misc/bh1780gli.c has a funky sysfs ABI.
> > It is only used in arch/arm/boot/dts/omap4-sdp.dts as far as I can
> > tell. What device is this, and more importantly, who is running it
> > and with what userspace? I'm happy to go and patch that userspace
> > if I can just get a pointer to whoever is developing it. Sailfish?
> 
> Sailfish has never used OMAP4 afaik. OMAP4 sdp is a TI dev board, so maybe
> someone from TI knows what this is for?

Adding linux-omap to Cc. I suggest do a minimal patch to remove the
sysfs interface, then revert the patch if needed. I don't think
people are using it.

Regards,

Tony

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Tony Lindgren]

* Peter De Schrijver <pdeschrijver@nvidia.com> [160404 00:43]:
> On Sun, Apr 03, 2016 at 06:30:49PM +0200, Linus Walleij wrote:
> > On Sun, Apr 3, 2016 at 11:51 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> > > On 30/03/16 09:34, Linus Walleij wrote:
> > 
> > >> I want to phase out the old misc driver, but don't know how that
> > >> should properly be handled. The driver has an old (totally
> > >> undocumented) sysfs ABI for raw measurements, should I add
> > >> this as backward-compatibility and symlink the device to the old
> > >> location?
> > >
> > > Good question.  I guess we unfortunately do need to jump through the hoops as
> > > who knows what userspace code there is out there using that interface.
> > > At least it is small!
> > 
> > The maintainer Hemanth seems to have disappeared.
> > 
> > Let's ask some of the OMAP and ex-OMAP people, I think they know
> > or know who knows...
> > 
> > Tony, Peter, Paul: drivers/misc/bh1780gli.c has a funky sysfs ABI.
> > It is only used in arch/arm/boot/dts/omap4-sdp.dts as far as I can
> > tell. What device is this, and more importantly, who is running it
> > and with what userspace? I'm happy to go and patch that userspace
> > if I can just get a pointer to whoever is developing it. Sailfish?
> 
> Sailfish has never used OMAP4 afaik. OMAP4 sdp is a TI dev board, so maybe
> someone from TI knows what this is for?

Adding linux-omap to Cc. I suggest do a minimal patch to remove the
sysfs interface, then revert the patch if needed. I don't think
people are using it.

Regards,

Tony

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Linus Walleij]

On Tue, Apr 5, 2016 at 12:35 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:

> Adding linux-omap to Cc. I suggest do a minimal patch to remove the
> sysfs interface, then revert the patch if needed. I don't think
> people are using it.

The old driver in misc is just using sysfs for everything, so removing the
sysfs is like deleting the driver. Once the IIO driver is merged I guess
I'll boldly post a patch to delete it and then a patch to activate the new
driver in the OMAP defconfig.

Yours,
Linus Walleij

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Linus Walleij]

On Tue, Apr 5, 2016 at 12:35 AM, Tony Lindgren <tony@atomide.com> wrote:

> Adding linux-omap to Cc. I suggest do a minimal patch to remove the
> sysfs interface, then revert the patch if needed. I don't think
> people are using it.

The old driver in misc is just using sysfs for everything, so removing the
sysfs is like deleting the driver. Once the IIO driver is merged I guess
I'll boldly post a patch to delete it and then a patch to activate the new
driver in the OMAP defconfig.

Yours,
Linus Walleij

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Tony Lindgren]

* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [160405 00:51]:
> On Tue, Apr 5, 2016 at 12:35 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> > Adding linux-omap to Cc. I suggest do a minimal patch to remove the
> > sysfs interface, then revert the patch if needed. I don't think
> > people are using it.
> 
> The old driver in misc is just using sysfs for everything, so removing the
> sysfs is like deleting the driver. Once the IIO driver is merged I guess
> I'll boldly post a patch to delete it and then a patch to activate the new
> driver in the OMAP defconfig.

OK

Tony

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [160405 00:51]:
> On Tue, Apr 5, 2016 at 12:35 AM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > Adding linux-omap to Cc. I suggest do a minimal patch to remove the
> > sysfs interface, then revert the patch if needed. I don't think
> > people are using it.
> 
> The old driver in misc is just using sysfs for everything, so removing the
> sysfs is like deleting the driver. Once the IIO driver is merged I guess
> I'll boldly post a patch to delete it and then a patch to activate the new
> driver in the OMAP defconfig.

OK

Tony

Re: [PATCH v3] iio: light: new driver for the ROHM BH1780

[Paul Walmsley]

On Sun, 3 Apr 2016, Linus Walleij wrote:

> On Sun, Apr 3, 2016 at 11:51 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> > On 30/03/16 09:34, Linus Walleij wrote:
> 
> >> I want to phase out the old misc driver, but don't know how that
> >> should properly be handled. The driver has an old (totally
> >> undocumented) sysfs ABI for raw measurements, should I add
> >> this as backward-compatibility and symlink the device to the old
> >> location?
> >
> > Good question.  I guess we unfortunately do need to jump through the hoops as
> > who knows what userspace code there is out there using that interface.
> > At least it is small!
> 
> The maintainer Hemanth seems to have disappeared.
> 
> Let's ask some of the OMAP and ex-OMAP people, I think they know
> or know who knows...
> 
> Tony, Peter, Paul: drivers/misc/bh1780gli.c has a funky sysfs ABI.
> It is only used in arch/arm/boot/dts/omap4-sdp.dts as far as I can
> tell. What device is this, and more importantly, who is running it
> and with what userspace? I'm happy to go and patch that userspace
> if I can just get a pointer to whoever is developing it. Sailfish?

If it's only used in the OMAP4 SDP DT file in upstream, then you probably 
don't need to worry much about who's using it... Sounds like you should 
just fix it.


- Paul