Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1373514 > unrolled thread
| Started by | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| First post | 2016-04-07 18:30 +0200 |
| Last post | 2016-04-10 15:20 +0200 |
| Articles | 15 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/5] Support for max44000 Ambient and Infrared Proximity Sensor Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
[PATCH 2/5] max44000: Initial support for proximity reading Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
Re: [PATCH 2/5] max44000: Initial support for proximity reading Jonathan Cameron <jic23@kernel.org> - 2016-04-10 15:20 +0200
[PATCH 5/5] max44000: Initial triggered buffer support Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
Re: [PATCH 5/5] max44000: Initial triggered buffer support Peter Meerwald-Stadler <pmeerw@pmeerw.net> - 2016-04-07 22:00 +0200
Re: [PATCH 5/5] max44000: Initial triggered buffer support Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-11 18:20 +0200
Re: [PATCH 5/5] max44000: Initial triggered buffer support Jonathan Cameron <jic23@kernel.org> - 2016-04-10 15:30 +0200
[PATCH 4/5] max44000: Expose ambient sensor scaling Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
Re: [PATCH 4/5] max44000: Expose ambient sensor scaling Jonathan Cameron <jic23@kernel.org> - 2016-04-10 15:30 +0200
[PATCH 1/5] max44000: Initial commit Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
Re: [PATCH 1/5] max44000: Initial commit Peter Meerwald-Stadler <pmeerw@pmeerw.net> - 2016-04-07 21:50 +0200
Re: [PATCH 1/5] max44000: Initial commit Jonathan Cameron <jic23@kernel.org> - 2016-04-10 15:20 +0200
Re: [PATCH 1/5] max44000: Initial commit Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-11 17:20 +0200
[PATCH 3/5] max44000: Support controlling LED current output Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-07 18:30 +0200
Re: [PATCH 3/5] max44000: Support controlling LED current output Jonathan Cameron <jic23@kernel.org> - 2016-04-10 15:20 +0200
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 0/5] Support for max44000 Ambient and Infrared Proximity Sensor |
| Message-ID | <rlkYF-6F9-1@gated-at.bofh.it> |
The driver supports reporting illuminance and proximity (via raw reads or software-triggered) buffers and controling the scaling factors. There is no support for power management, events or reporting the IR/green channels separately. The device is always set in the ALS/PROX mode in order to report all data. Crestez Dan Leonard (5): max44000: Initial commit max44000: Initial support for proximity reading max44000: Support controlling LED current output max44000: Expose ambient sensor scaling max44000: Initial triggered buffer support drivers/iio/light/Kconfig | 11 + drivers/iio/light/Makefile | 1 + drivers/iio/light/max44000.c | 627 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 639 insertions(+) create mode 100644 drivers/iio/light/max44000.c -- 2.8.0.rc3
[toc] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 2/5] max44000: Initial support for proximity reading |
| Message-ID | <rlkYG-6F9-19@gated-at.bofh.it> |
| In reply to | #1373514 |
The proximity sensor relies on sending pulses to an external IR led and
it is disabled by default on powerup. The driver will enable it with a
default power setting.
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
drivers/iio/light/max44000.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
index 7c12153..10a0fe8 100644
--- a/drivers/iio/light/max44000.c
+++ b/drivers/iio/light/max44000.c
@@ -41,6 +41,23 @@
#define MAX44000_REG_TRIM_GAIN_GREEN 0x0f
#define MAX44000_REG_TRIM_GAIN_IR 0x10
+/* REG_CFG bits */
+#define MAX44000_CFG_ALSINTE 0x01
+#define MAX44000_CFG_PRXINTE 0x02
+#define MAX44000_CFG_MASK 0x1c
+#define MAX44000_CFG_MODE_SHUTDOWN 0x00
+#define MAX44000_CFG_MODE_ALS_GIR 0x04
+#define MAX44000_CFG_MODE_ALS_G 0x08
+#define MAX44000_CFG_MODE_ALS_IR 0x0c
+#define MAX44000_CFG_MODE_ALS_PRX 0x10
+#define MAX44000_CFG_MODE_PRX 0x14
+#define MAX44000_CFG_TRIM 0x20
+
+/* REG_TX bits */
+#define MAX44000_LED_CURRENT_MASK 0xf
+#define MAX44000_LED_CURRENT_MAX 11
+#define MAX44000_LED_CURRENT_DEFAULT 6
+
#define MAX44000_ALSDATA_OVERFLOW 0x4000
#define MAX44000_REGMASK_READABLE 0x419fff
@@ -60,6 +77,12 @@ static const struct iio_chan_spec max44000_channels[] = {
{
.type = IIO_LIGHT,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_INT_TIME),
+ },
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
},
};
@@ -90,11 +113,23 @@ static inline int max44000_read_alsval(struct max44000_data *data)
return regval;
}
+static inline int max44000_write_led_current_raw(struct max44000_data *data, int val)
+{
+ /* Maybe we should clamp the value instead? */
+ if (val < 0 || val > MAX44000_LED_CURRENT_MAX)
+ return -ERANGE;
+ if (val >= 8)
+ val += 4;
+ return regmap_write_bits(data->regmap, MAX44000_REG_CFG_TX,
+ MAX44000_LED_CURRENT_MASK, val);
+}
+
static int max44000_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
{
struct max44000_data *data = iio_priv(indio_dev);
+ unsigned int regval;
int ret;
switch (mask) {
@@ -109,6 +144,15 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
*val = ret;
return IIO_VAL_INT;
+ case IIO_PROXIMITY:
+ mutex_lock(&data->lock);
+ ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val);
+ mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+ *val = regval;
+ return IIO_VAL_INT;
+
default:
return -EINVAL;
}
@@ -244,6 +288,23 @@ static int max44000_probe(struct i2c_client *client,
return ret;
}
+ /* By default the LED pulse used for the proximity sensor is disabled.
+ * Set a middle value so that we get some sort of valid data by default.
+ */
+ ret = max44000_write_led_current_raw(data, MAX44000_LED_CURRENT_DEFAULT);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to write init config: %d\n", ret);
+ return ret;
+ }
+
+ /* By default set in ALS_PRX mode which allows easy reading of both values. */
+ reg = MAX44000_CFG_TRIM | MAX44000_CFG_MODE_ALS_PRX;
+ ret = regmap_write(data->regmap, MAX44000_REG_CFG_MAIN, reg);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to write init config: %d\n", ret);
+ return ret;
+ }
+
return iio_device_register(indio_dev);
}
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 15:20 +0200 |
| Subject | Re: [PATCH 2/5] max44000: Initial support for proximity reading |
| Message-ID | <rmnrt-5Be-19@gated-at.bofh.it> |
| In reply to | #1373520 |
On 07/04/16 17:21, Crestez Dan Leonard wrote:
> The proximity sensor relies on sending pulses to an external IR led and
> it is disabled by default on powerup. The driver will enable it with a
> default power setting.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
One trivial point inline...
> ---
> drivers/iio/light/max44000.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index 7c12153..10a0fe8 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -41,6 +41,23 @@
> #define MAX44000_REG_TRIM_GAIN_GREEN 0x0f
> #define MAX44000_REG_TRIM_GAIN_IR 0x10
>
> +/* REG_CFG bits */
> +#define MAX44000_CFG_ALSINTE 0x01
> +#define MAX44000_CFG_PRXINTE 0x02
> +#define MAX44000_CFG_MASK 0x1c
> +#define MAX44000_CFG_MODE_SHUTDOWN 0x00
> +#define MAX44000_CFG_MODE_ALS_GIR 0x04
> +#define MAX44000_CFG_MODE_ALS_G 0x08
> +#define MAX44000_CFG_MODE_ALS_IR 0x0c
> +#define MAX44000_CFG_MODE_ALS_PRX 0x10
> +#define MAX44000_CFG_MODE_PRX 0x14
> +#define MAX44000_CFG_TRIM 0x20
> +
> +/* REG_TX bits */
> +#define MAX44000_LED_CURRENT_MASK 0xf
> +#define MAX44000_LED_CURRENT_MAX 11
> +#define MAX44000_LED_CURRENT_DEFAULT 6
> +
> #define MAX44000_ALSDATA_OVERFLOW 0x4000
>
> #define MAX44000_REGMASK_READABLE 0x419fff
> @@ -60,6 +77,12 @@ static const struct iio_chan_spec max44000_channels[] = {
> {
> .type = IIO_LIGHT,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_INT_TIME),
> + },
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> },
> };
> @@ -90,11 +113,23 @@ static inline int max44000_read_alsval(struct max44000_data *data)
> return regval;
> }
>
> +static inline int max44000_write_led_current_raw(struct max44000_data *data, int val)
> +{
> + /* Maybe we should clamp the value instead? */
> + if (val < 0 || val > MAX44000_LED_CURRENT_MAX)
> + return -ERANGE;
> + if (val >= 8)
> + val += 4;
> + return regmap_write_bits(data->regmap, MAX44000_REG_CFG_TX,
> + MAX44000_LED_CURRENT_MASK, val);
> +}
> +
> static int max44000_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> {
> struct max44000_data *data = iio_priv(indio_dev);
> + unsigned int regval;
> int ret;
>
> switch (mask) {
> @@ -109,6 +144,15 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
> *val = ret;
> return IIO_VAL_INT;
>
> + case IIO_PROXIMITY:
> + mutex_lock(&data->lock);
> + ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val);
> + mutex_unlock(&data->lock);
> + if (ret < 0)
> + return ret;
> + *val = regval;
> + return IIO_VAL_INT;
> +
> default:
> return -EINVAL;
> }
> @@ -244,6 +288,23 @@ static int max44000_probe(struct i2c_client *client,
> return ret;
> }
>
> + /* By default the LED pulse used for the proximity sensor is disabled.
Please run checkpatch.pl on these. It would probably have complained about
the multiline comment syntax here.
> + * Set a middle value so that we get some sort of valid data by default.
> + */
> + ret = max44000_write_led_current_raw(data, MAX44000_LED_CURRENT_DEFAULT);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "failed to write init config: %d\n", ret);
> + return ret;
> + }
> +
> + /* By default set in ALS_PRX mode which allows easy reading of both values. */
> + reg = MAX44000_CFG_TRIM | MAX44000_CFG_MODE_ALS_PRX;
> + ret = regmap_write(data->regmap, MAX44000_REG_CFG_MAIN, reg);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "failed to write init config: %d\n", ret);
> + return ret;
> + }
> +
> return iio_device_register(indio_dev);
> }
>
>
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 5/5] max44000: Initial triggered buffer support |
| Message-ID | <rlkYH-6F9-35@gated-at.bofh.it> |
| In reply to | #1373514 |
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
drivers/iio/light/max44000.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
index e479c53..7b1f8bc 100644
--- a/drivers/iio/light/max44000.c
+++ b/drivers/iio/light/max44000.c
@@ -19,6 +19,9 @@
#include <linux/util_macros.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
#include <linux/acpi.h>
#define MAX44000_DRV_NAME "max44000"
@@ -123,24 +126,41 @@ static const char max44000_scale_avail_str[] =
"0.5 "
"4";
+#define MAX44000_SCAN_INDEX_ALS 0
+#define MAX44000_SCAN_INDEX_PRX 1
+
static const struct iio_chan_spec max44000_channels[] = {
{
.type = IIO_LIGHT,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_INT_TIME),
+ .scan_index = MAX44000_SCAN_INDEX_ALS,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 14,
+ .storagebits = 16,
+ }
},
{
.type = IIO_PROXIMITY,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = MAX44000_SCAN_INDEX_PRX,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 8,
+ .storagebits = 8,
+ }
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
{
.type = IIO_CURRENT,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE),
.extend_name = "led",
.output = 1,
+ .scan_index = -1,
},
};
@@ -456,6 +476,42 @@ static int max44000_force_write_defaults(struct max44000_data *data)
return 0;
}
+static irqreturn_t max44000_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct max44000_data *data = iio_priv(indio_dev);
+ u16 buf[indio_dev->scan_bytes / 2];
+ u8 *pos = (u8 *)buf;
+ unsigned int regval;
+ int ret;
+
+ mutex_lock(&data->lock);
+ if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_ALS)) {
+ ret = max44000_read_alsval(data);
+ if (ret < 0)
+ goto out_unlock;
+ *((u16 *)pos) = ret;
+ pos += 2;
+ }
+ if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_PRX)) {
+ ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val);
+ if (ret < 0)
+ goto out_unlock;
+ *pos = regval;
+ }
+ mutex_unlock(&data->lock);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+
+out_unlock:
+ mutex_unlock(&data->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
static int max44000_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -513,6 +569,12 @@ static int max44000_probe(struct i2c_client *client,
return ret;
}
+ ret = iio_triggered_buffer_setup(indio_dev, NULL, max44000_trigger_handler, NULL);
+ if (ret < 0) {
+ dev_err(&client->dev, "iio triggered buffer setup failed\n");
+ return ret;
+ }
+
return iio_device_register(indio_dev);
}
@@ -521,6 +583,7 @@ static int max44000_remove(struct i2c_client *client)
struct iio_dev *indio_dev = i2c_get_clientdata(client);
iio_device_unregister(indio_dev);
+ iio_triggered_buffer_cleanup(indio_dev);
return 0;
}
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Peter Meerwald-Stadler <pmeerw@pmeerw.net> |
|---|---|
| Date | 2016-04-07 22:00 +0200 |
| Subject | Re: [PATCH 5/5] max44000: Initial triggered buffer support |
| Message-ID | <rlofU-rp-9@gated-at.bofh.it> |
| In reply to | #1373523 |
comments below
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/light/max44000.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index e479c53..7b1f8bc 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -19,6 +19,9 @@
> #include <linux/util_macros.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> #include <linux/acpi.h>
>
> #define MAX44000_DRV_NAME "max44000"
> @@ -123,24 +126,41 @@ static const char max44000_scale_avail_str[] =
> "0.5 "
> "4";
>
> +#define MAX44000_SCAN_INDEX_ALS 0
> +#define MAX44000_SCAN_INDEX_PRX 1
> +
> static const struct iio_chan_spec max44000_channels[] = {
> {
> .type = IIO_LIGHT,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> BIT(IIO_CHAN_INFO_INT_TIME),
> + .scan_index = MAX44000_SCAN_INDEX_ALS,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 14,
> + .storagebits = 16,
> + }
> },
> {
> .type = IIO_PROXIMITY,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = MAX44000_SCAN_INDEX_PRX,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 8,
> + .storagebits = 8,
code would get simpler if storagebits = 16
> + }
> },
> + IIO_CHAN_SOFT_TIMESTAMP(2),
> {
> .type = IIO_CURRENT,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> BIT(IIO_CHAN_INFO_SCALE),
> .extend_name = "led",
> .output = 1,
> + .scan_index = -1,
> },
> };
>
> @@ -456,6 +476,42 @@ static int max44000_force_write_defaults(struct max44000_data *data)
> return 0;
> }
>
> +static irqreturn_t max44000_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct max44000_data *data = iio_priv(indio_dev);
> + u16 buf[indio_dev->scan_bytes / 2];
> + u8 *pos = (u8 *)buf;
int i = 0;
> + unsigned int regval;
> + int ret;
> +
> + mutex_lock(&data->lock);
> + if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_ALS)) {
BIT(MAX44000_SCAN_INDEX_ALS)
> + ret = max44000_read_alsval(data);
> + if (ret < 0)
> + goto out_unlock;
> + *((u16 *)pos) = ret;
buf[i++] = ret;
> + pos += 2;
> + }
> + if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_PRX)) {
> + ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val);
> + if (ret < 0)
> + goto out_unlock;
buf[i++] = retval;
> + *pos = regval;
> + }
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
moving iio_push_to_buffers_with_timestamp() before mutex_unlock() would
allow simpler code (not sure if it's worth)
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +
> +out_unlock:
> + mutex_unlock(&data->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +
> static int max44000_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -513,6 +569,12 @@ static int max44000_probe(struct i2c_client *client,
> return ret;
> }
>
> + ret = iio_triggered_buffer_setup(indio_dev, NULL, max44000_trigger_handler, NULL);
> + if (ret < 0) {
> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
> + return ret;
> + }
> +
> return iio_device_register(indio_dev);
no devm_ possible anymore :-)
> }
>
> @@ -521,6 +583,7 @@ static int max44000_remove(struct i2c_client *client)
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> iio_device_unregister(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
> return 0;
> }
>
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-11 18:20 +0200 |
| Subject | Re: [PATCH 5/5] max44000: Initial triggered buffer support |
| Message-ID | <rmMJc-gC-11@gated-at.bofh.it> |
| In reply to | #1373714 |
On 04/07/2016 10:59 PM, Peter Meerwald-Stadler wrote:
>> static int max44000_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> @@ -513,6 +569,12 @@ static int max44000_probe(struct i2c_client *client,
>> return ret;
>> }
>>
>> + ret = iio_triggered_buffer_setup(indio_dev, NULL, max44000_trigger_handler, NULL);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
>> + return ret;
>> + }
>> +
>> return iio_device_register(indio_dev);
>
> no devm_ possible anymore :-)
It's not clear to me why explicit calls to iio_triggered_buffer_cleanup
are done in every driver. Wouldn't it be possible for iio_dev_release to
call iio_triggered_buffer_cleanup? That would require
triggered_buffer_cleanup to explictly NULL the fields it deallocates so
that duplicate cleanups don't crash.
--
Regards,
Leonard
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 15:30 +0200 |
| Subject | Re: [PATCH 5/5] max44000: Initial triggered buffer support |
| Message-ID | <rmnB9-5GJ-11@gated-at.bofh.it> |
| In reply to | #1373523 |
On 07/04/16 17:21, Crestez Dan Leonard wrote:
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/light/max44000.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index e479c53..7b1f8bc 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -19,6 +19,9 @@
> #include <linux/util_macros.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> #include <linux/acpi.h>
>
> #define MAX44000_DRV_NAME "max44000"
> @@ -123,24 +126,41 @@ static const char max44000_scale_avail_str[] =
> "0.5 "
> "4";
>
> +#define MAX44000_SCAN_INDEX_ALS 0
> +#define MAX44000_SCAN_INDEX_PRX 1
> +
> static const struct iio_chan_spec max44000_channels[] = {
> {
> .type = IIO_LIGHT,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
> BIT(IIO_CHAN_INFO_INT_TIME),
> + .scan_index = MAX44000_SCAN_INDEX_ALS,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 14,
> + .storagebits = 16,
> + }
> },
> {
> .type = IIO_PROXIMITY,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = MAX44000_SCAN_INDEX_PRX,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 8,
> + .storagebits = 8,
> + }
> },
> + IIO_CHAN_SOFT_TIMESTAMP(2),
> {
> .type = IIO_CURRENT,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> BIT(IIO_CHAN_INFO_SCALE),
> .extend_name = "led",
> .output = 1,
> + .scan_index = -1,
> },
> };
>
> @@ -456,6 +476,42 @@ static int max44000_force_write_defaults(struct max44000_data *data)
> return 0;
> }
>
> +static irqreturn_t max44000_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct max44000_data *data = iio_priv(indio_dev);
> + u16 buf[indio_dev->scan_bytes / 2];
Autobuilder is moaning about this + you shouldn't directly access scan_bytes
at all from a driver. Here just make sure it's big enough to take anything that might
show up (which isn't terribly big ;)
> + u8 *pos = (u8 *)buf;
> + unsigned int regval;
> + int ret;
> +
> + mutex_lock(&data->lock);
> + if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_ALS)) {
> + ret = max44000_read_alsval(data);
> + if (ret < 0)
> + goto out_unlock;
> + *((u16 *)pos) = ret;
> + pos += 2;
> + }
> + if (*indio_dev->active_scan_mask & (1 << MAX44000_SCAN_INDEX_PRX)) {
> + ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val);
> + if (ret < 0)
> + goto out_unlock;
> + *pos = regval;
> + }
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +
> +out_unlock:
> + mutex_unlock(&data->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +
> static int max44000_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -513,6 +569,12 @@ static int max44000_probe(struct i2c_client *client,
> return ret;
> }
>
> + ret = iio_triggered_buffer_setup(indio_dev, NULL, max44000_trigger_handler, NULL);
> + if (ret < 0) {
> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
> + return ret;
> + }
> +
> return iio_device_register(indio_dev);
> }
>
> @@ -521,6 +583,7 @@ static int max44000_remove(struct i2c_client *client)
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> iio_device_unregister(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
> return 0;
> }
>
>
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 4/5] max44000: Expose ambient sensor scaling |
| Message-ID | <rlkYH-6F9-51@gated-at.bofh.it> |
| In reply to | #1373514 |
This patch exposes ALSTIM as illuminance_integration_time and ALSPGA as
illuminance_scale.
Changing ALSTIM also changes the number of bits available in the data
register. This is handled inside raw value reading because:
* It's very easy to shift a few bits
* It allows SCALE and INT_TIME to be completely independent controls
* Buffer support requires constant scan_type.realbits per-channel
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
drivers/iio/light/max44000.c | 163 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 159 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
index 1dc10b9..e479c53 100644
--- a/drivers/iio/light/max44000.c
+++ b/drivers/iio/light/max44000.c
@@ -53,6 +53,12 @@
#define MAX44000_CFG_MODE_PRX 0x14
#define MAX44000_CFG_TRIM 0x20
+/* REG_RX bits */
+#define MAX44000_CFG_RX_ALSTIM_MASK 0x0c
+#define MAX44000_CFG_RX_ALSTIM_SHIFT 2
+#define MAX44000_CFG_RX_ALSPGA_MASK 0x03
+#define MAX44000_CFG_RX_ALSPGA_SHIFT 0
+
/* REG_TX bits */
#define MAX44000_LED_CURRENT_MASK 0xf
#define MAX44000_LED_CURRENT_MAX 11
@@ -73,6 +79,50 @@ struct max44000_data {
/* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
#define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5
+/* Scale can be multiplied by up to 128x via ALSPGA for measurement gain */
+static const int max44000_alspga_shift[] = {0, 2, 4, 7};
+#define MAX44000_ALSPGA_MAX_SHIFT 7
+
+/* Scale can be multiplied by up to 64x via ALSTIM because of lost resolution
+ *
+ * This scaling factor is hidden from userspace and instead accounted for when
+ * reading raw values from the device.
+ *
+ * This makes it possible to cleanly expose ALSPGA as IIO_CHAN_INFO_SCALE and
+ * ALSTIM as IIO_CHAN_INFO_INT_TIME without the values affecting each other.
+ *
+ * Hanlding this internally is also required for buffer support because the
+ * channel's scan_type can't be modified dynamically.
+ */
+static const int max44000_alstim_shift[] = {0, 2, 4, 6};
+#define MAX44000_ALSTIM_SHIFT(alstim) (2 * (alstim))
+
+/* Available integration times with pretty manual alignment: */
+static const int max44000_int_time_avail_ns_array[] = {
+ 100000000,
+ 25000000,
+ 6250000,
+ 1562500,
+};
+static const char max44000_int_time_avail_str[] =
+ "0.100 "
+ "0.025 "
+ "0.00625 "
+ "0.001625";
+
+/* Available scales (internal to ulux) with pretty manual alignment: */
+static const int max44000_scale_avail_ulux_array[] = {
+ 31250,
+ 125000,
+ 500000,
+ 4000000,
+};
+static const char max44000_scale_avail_str[] =
+ "0.03125 "
+ "0.125 "
+ "0.5 "
+ "4";
+
static const struct iio_chan_spec max44000_channels[] = {
{
.type = IIO_LIGHT,
@@ -94,15 +144,54 @@ static const struct iio_chan_spec max44000_channels[] = {
},
};
+static inline int max44000_read_alstim(struct max44000_data *data)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(data->regmap, MAX44000_REG_CFG_RX, &val);
+ if (ret < 0)
+ return ret;
+ return (val & MAX44000_CFG_RX_ALSTIM_MASK) >> MAX44000_CFG_RX_ALSTIM_SHIFT;
+}
+
+static inline int max44000_write_alstim(struct max44000_data *data, int val)
+{
+ return regmap_write_bits(data->regmap, MAX44000_REG_CFG_RX,
+ MAX44000_CFG_RX_ALSTIM_MASK,
+ val << MAX44000_CFG_RX_ALSTIM_SHIFT);
+}
+
+static inline int max44000_read_alspga(struct max44000_data *data)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(data->regmap, MAX44000_REG_CFG_RX, &val);
+ if (ret < 0)
+ return ret;
+ return (val & MAX44000_CFG_RX_ALSPGA_MASK) >> MAX44000_CFG_RX_ALSPGA_SHIFT;
+}
+
+static inline int max44000_write_alspga(struct max44000_data *data, int val)
+{
+ return regmap_write_bits(data->regmap, MAX44000_REG_CFG_RX,
+ MAX44000_CFG_RX_ALSPGA_MASK,
+ val << MAX44000_CFG_RX_ALSPGA_SHIFT);
+}
+
static inline int max44000_read_alsval(struct max44000_data *data)
{
u16 regval;
- int ret;
+ int alstim, ret;
regval = 0;
ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, ®val, 2);
if (ret < 0)
return ret;
+ alstim = ret = max44000_read_alstim(data);
+ if (ret < 0)
+ return ret;
regval = be16_to_cpu(regval);
@@ -117,7 +206,7 @@ static inline int max44000_read_alsval(struct max44000_data *data)
if (regval & MAX44000_ALSDATA_OVERFLOW)
return 0x3FFF;
- return regval;
+ return regval << MAX44000_ALSTIM_SHIFT(alstim);
}
static inline int max44000_write_led_current_raw(struct max44000_data *data, int val)
@@ -150,6 +239,7 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
int *val, int *val2, long mask)
{
struct max44000_data *data = iio_priv(indio_dev);
+ int alstim, alspga;
unsigned int regval;
int ret;
@@ -195,14 +285,34 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_LIGHT:
- *val = 1;
- *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
+ mutex_lock(&data->lock);
+ alspga = ret = max44000_read_alspga(data);
+ mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+
+ /* Avoid negative shifts */
+ *val = (1 << MAX44000_ALSPGA_MAX_SHIFT);
+ *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2
+ + MAX44000_ALSPGA_MAX_SHIFT
+ - max44000_alspga_shift[alspga];
return IIO_VAL_FRACTIONAL_LOG2;
default:
return -EINVAL;
}
+ case IIO_CHAN_INFO_INT_TIME:
+ mutex_lock(&data->lock);
+ alstim = ret = max44000_read_alstim(data);
+ mutex_unlock(&data->lock);
+
+ if (ret < 0)
+ return ret;
+ *val = 0;
+ *val2 = max44000_int_time_avail_ns_array[alstim];
+ return IIO_VAL_INT_PLUS_NANO;
+
default:
return -EINVAL;
}
@@ -220,15 +330,60 @@ static int max44000_write_raw(struct iio_dev *indio_dev,
ret = max44000_write_led_current_raw(data, val);
mutex_unlock(&data->lock);
return ret;
+ } else if (mask == IIO_CHAN_INFO_INT_TIME && chan->type == IIO_LIGHT) {
+ s64 valns = val * NSEC_PER_SEC + val2;
+ int alstim = find_closest_descending(valns,
+ max44000_int_time_avail_ns_array,
+ ARRAY_SIZE(max44000_int_time_avail_ns_array));
+ mutex_lock(&data->lock);
+ ret = max44000_write_alstim(data, alstim);
+ mutex_unlock(&data->lock);
+ return ret;
+ } else if (mask == IIO_CHAN_INFO_SCALE && chan->type == IIO_LIGHT) {
+ s64 valus = val * USEC_PER_SEC + val2;
+ int alspga = find_closest(valus,
+ max44000_scale_avail_ulux_array,
+ ARRAY_SIZE(max44000_scale_avail_ulux_array));
+ mutex_lock(&data->lock);
+ ret = max44000_write_alspga(data, alspga);
+ mutex_unlock(&data->lock);
+ return ret;
}
return -EINVAL;
}
+static int max44000_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ if (mask == IIO_CHAN_INFO_INT_TIME && chan->type == IIO_LIGHT)
+ return IIO_VAL_INT_PLUS_NANO;
+ else if (mask == IIO_CHAN_INFO_SCALE && chan->type == IIO_LIGHT)
+ return IIO_VAL_INT_PLUS_MICRO;
+ else
+ return IIO_VAL_INT;
+}
+
+static IIO_CONST_ATTR(illuminance_integration_time_available, max44000_int_time_avail_str);
+static IIO_CONST_ATTR(illuminance_scale_available, max44000_scale_avail_str);
+
+static struct attribute *max44000_attributes[] = {
+ &iio_const_attr_illuminance_integration_time_available.dev_attr.attr,
+ &iio_const_attr_illuminance_scale_available.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group max44000_attribute_group = {
+ .attrs = max44000_attributes,
+};
+
static const struct iio_info max44000_info = {
.driver_module = THIS_MODULE,
.read_raw = max44000_read_raw,
.write_raw = max44000_write_raw,
+ .write_raw_get_fmt = max44000_write_raw_get_fmt,
+ .attrs = &max44000_attribute_group,
};
static bool max44000_readable_reg(struct device *dev, unsigned int reg)
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 15:30 +0200 |
| Subject | Re: [PATCH 4/5] max44000: Expose ambient sensor scaling |
| Message-ID | <rmnB8-5GJ-7@gated-at.bofh.it> |
| In reply to | #1373526 |
On 07/04/16 17:21, Crestez Dan Leonard wrote:
> This patch exposes ALSTIM as illuminance_integration_time and ALSPGA as
> illuminance_scale.
>
> Changing ALSTIM also changes the number of bits available in the data
> register.
Hmm.. This usually means we have oversampling going on rather than straight
integration time control - though it could be actually turning on or off
steps in the ADC pipeline...
> This is handled inside raw value reading because:
> * It's very easy to shift a few bits
> * It allows SCALE and INT_TIME to be completely independent controls
> * Buffer support requires constant scan_type.realbits per-channel
Agreed, this is the easiest way to handle this sort of thing.
This looks good to me (few trivial bits I picked up on inline).
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/light/max44000.c | 163 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 159 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index 1dc10b9..e479c53 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -53,6 +53,12 @@
> #define MAX44000_CFG_MODE_PRX 0x14
> #define MAX44000_CFG_TRIM 0x20
>
> +/* REG_RX bits */
> +#define MAX44000_CFG_RX_ALSTIM_MASK 0x0c
> +#define MAX44000_CFG_RX_ALSTIM_SHIFT 2
> +#define MAX44000_CFG_RX_ALSPGA_MASK 0x03
> +#define MAX44000_CFG_RX_ALSPGA_SHIFT 0
> +
> /* REG_TX bits */
> #define MAX44000_LED_CURRENT_MASK 0xf
> #define MAX44000_LED_CURRENT_MAX 11
> @@ -73,6 +79,50 @@ struct max44000_data {
> /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
> #define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5
>
> +/* Scale can be multiplied by up to 128x via ALSPGA for measurement gain */
> +static const int max44000_alspga_shift[] = {0, 2, 4, 7};
> +#define MAX44000_ALSPGA_MAX_SHIFT 7
> +
> +/* Scale can be multiplied by up to 64x via ALSTIM because of lost resolution
Another multiline comment issue. Please check all the patches..
> + *
> + * This scaling factor is hidden from userspace and instead accounted for when
> + * reading raw values from the device.
> + *
> + * This makes it possible to cleanly expose ALSPGA as IIO_CHAN_INFO_SCALE and
> + * ALSTIM as IIO_CHAN_INFO_INT_TIME without the values affecting each other.
> + *
> + * Hanlding this internally is also required for buffer support because the
handling.
> + * channel's scan_type can't be modified dynamically.
> + */
> +static const int max44000_alstim_shift[] = {0, 2, 4, 6};
> +#define MAX44000_ALSTIM_SHIFT(alstim) (2 * (alstim))
> +
> +/* Available integration times with pretty manual alignment: */
> +static const int max44000_int_time_avail_ns_array[] = {
> + 100000000,
> + 25000000,
> + 6250000,
> + 1562500,
> +};
> +static const char max44000_int_time_avail_str[] =
> + "0.100 "
> + "0.025 "
> + "0.00625 "
> + "0.001625";
> +
> +/* Available scales (internal to ulux) with pretty manual alignment: */
> +static const int max44000_scale_avail_ulux_array[] = {
> + 31250,
> + 125000,
> + 500000,
> + 4000000,
> +};
> +static const char max44000_scale_avail_str[] =
> + "0.03125 "
> + "0.125 "
> + "0.5 "
> + "4";
> +
> static const struct iio_chan_spec max44000_channels[] = {
> {
> .type = IIO_LIGHT,
> @@ -94,15 +144,54 @@ static const struct iio_chan_spec max44000_channels[] = {
> },
> };
>
> +static inline int max44000_read_alstim(struct max44000_data *data)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(data->regmap, MAX44000_REG_CFG_RX, &val);
> + if (ret < 0)
> + return ret;
> + return (val & MAX44000_CFG_RX_ALSTIM_MASK) >> MAX44000_CFG_RX_ALSTIM_SHIFT;
> +}
> +
> +static inline int max44000_write_alstim(struct max44000_data *data, int val)
> +{
> + return regmap_write_bits(data->regmap, MAX44000_REG_CFG_RX,
> + MAX44000_CFG_RX_ALSTIM_MASK,
> + val << MAX44000_CFG_RX_ALSTIM_SHIFT);
> +}
> +
> +static inline int max44000_read_alspga(struct max44000_data *data)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(data->regmap, MAX44000_REG_CFG_RX, &val);
> + if (ret < 0)
> + return ret;
> + return (val & MAX44000_CFG_RX_ALSPGA_MASK) >> MAX44000_CFG_RX_ALSPGA_SHIFT;
> +}
> +
> +static inline int max44000_write_alspga(struct max44000_data *data, int val)
> +{
> + return regmap_write_bits(data->regmap, MAX44000_REG_CFG_RX,
> + MAX44000_CFG_RX_ALSPGA_MASK,
> + val << MAX44000_CFG_RX_ALSPGA_SHIFT);
> +}
> +
> static inline int max44000_read_alsval(struct max44000_data *data)
> {
> u16 regval;
> - int ret;
> + int alstim, ret;
>
> regval = 0;
> ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, ®val, 2);
> if (ret < 0)
> return ret;
> + alstim = ret = max44000_read_alstim(data);
> + if (ret < 0)
> + return ret;
>
> regval = be16_to_cpu(regval);
>
> @@ -117,7 +206,7 @@ static inline int max44000_read_alsval(struct max44000_data *data)
> if (regval & MAX44000_ALSDATA_OVERFLOW)
> return 0x3FFF;
>
> - return regval;
> + return regval << MAX44000_ALSTIM_SHIFT(alstim);
> }
>
> static inline int max44000_write_led_current_raw(struct max44000_data *data, int val)
> @@ -150,6 +239,7 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long mask)
> {
> struct max44000_data *data = iio_priv(indio_dev);
> + int alstim, alspga;
> unsigned int regval;
> int ret;
>
> @@ -195,14 +285,34 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
>
> case IIO_LIGHT:
> - *val = 1;
> - *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
> + mutex_lock(&data->lock);
> + alspga = ret = max44000_read_alspga(data);
> + mutex_unlock(&data->lock);
> + if (ret < 0)
> + return ret;
> +
> + /* Avoid negative shifts */
> + *val = (1 << MAX44000_ALSPGA_MAX_SHIFT);
> + *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2
> + + MAX44000_ALSPGA_MAX_SHIFT
> + - max44000_alspga_shift[alspga];
> return IIO_VAL_FRACTIONAL_LOG2;
>
> default:
> return -EINVAL;
> }
>
> + case IIO_CHAN_INFO_INT_TIME:
> + mutex_lock(&data->lock);
> + alstim = ret = max44000_read_alstim(data);
> + mutex_unlock(&data->lock);
> +
> + if (ret < 0)
> + return ret;
> + *val = 0;
> + *val2 = max44000_int_time_avail_ns_array[alstim];
> + return IIO_VAL_INT_PLUS_NANO;
> +
> default:
> return -EINVAL;
> }
> @@ -220,15 +330,60 @@ static int max44000_write_raw(struct iio_dev *indio_dev,
> ret = max44000_write_led_current_raw(data, val);
> mutex_unlock(&data->lock);
> return ret;
> + } else if (mask == IIO_CHAN_INFO_INT_TIME && chan->type == IIO_LIGHT) {
> + s64 valns = val * NSEC_PER_SEC + val2;
> + int alstim = find_closest_descending(valns,
> + max44000_int_time_avail_ns_array,
> + ARRAY_SIZE(max44000_int_time_avail_ns_array));
> + mutex_lock(&data->lock);
> + ret = max44000_write_alstim(data, alstim);
> + mutex_unlock(&data->lock);
> + return ret;
> + } else if (mask == IIO_CHAN_INFO_SCALE && chan->type == IIO_LIGHT) {
> + s64 valus = val * USEC_PER_SEC + val2;
> + int alspga = find_closest(valus,
> + max44000_scale_avail_ulux_array,
> + ARRAY_SIZE(max44000_scale_avail_ulux_array));
> + mutex_lock(&data->lock);
> + ret = max44000_write_alspga(data, alspga);
> + mutex_unlock(&data->lock);
> + return ret;
> }
>
> return -EINVAL;
> }
>
> +static int max44000_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + if (mask == IIO_CHAN_INFO_INT_TIME && chan->type == IIO_LIGHT)
> + return IIO_VAL_INT_PLUS_NANO;
> + else if (mask == IIO_CHAN_INFO_SCALE && chan->type == IIO_LIGHT)
> + return IIO_VAL_INT_PLUS_MICRO;
> + else
> + return IIO_VAL_INT;
> +}
> +
> +static IIO_CONST_ATTR(illuminance_integration_time_available, max44000_int_time_avail_str);
> +static IIO_CONST_ATTR(illuminance_scale_available, max44000_scale_avail_str);
> +
> +static struct attribute *max44000_attributes[] = {
> + &iio_const_attr_illuminance_integration_time_available.dev_attr.attr,
> + &iio_const_attr_illuminance_scale_available.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group max44000_attribute_group = {
> + .attrs = max44000_attributes,
> +};
> +
> static const struct iio_info max44000_info = {
> .driver_module = THIS_MODULE,
> .read_raw = max44000_read_raw,
> .write_raw = max44000_write_raw,
> + .write_raw_get_fmt = max44000_write_raw_get_fmt,
> + .attrs = &max44000_attribute_group,
> };
>
> static bool max44000_readable_reg(struct device *dev, unsigned int reg)
>
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 1/5] max44000: Initial commit |
| Message-ID | <rlkYH-6F9-45@gated-at.bofh.it> |
| In reply to | #1373514 |
This just adds support for reporting illuminance with default settings.
All default registers are written on probe because the device otherwise
lacks a reset function.
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
drivers/iio/light/Kconfig | 11 ++
drivers/iio/light/Makefile | 1 +
drivers/iio/light/max44000.c | 295 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 307 insertions(+)
create mode 100644 drivers/iio/light/max44000.c
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index cfd3df8..42c15c1 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -320,4 +320,15 @@ config VCNL4000
To compile this driver as a module, choose M here: the
module will be called vcnl4000.
+config MAX44000
+ tristate "MAX44000 Ambient and Infrared Proximity Sensor"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y here if you want to build support for Maxim Integrated's
+ MAX44000 ambient and infrared proximity sensor device.
+
+ To compile this driver as a module, choose M here:
+ the module will be called max44000.
+
endmenu
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index b2c3105..14b2f36 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_TCS3472) += tcs3472.o
obj-$(CONFIG_TSL4531) += tsl4531.o
obj-$(CONFIG_US5182D) += us5182d.o
obj-$(CONFIG_VCNL4000) += vcnl4000.o
+obj-$(CONFIG_MAX44000) += max44000.o
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
new file mode 100644
index 0000000..7c12153
--- /dev/null
+++ b/drivers/iio/light/max44000.c
@@ -0,0 +1,295 @@
+/*
+ * MAX44000 Ambient and Infrared Proximity Sensor
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Data sheet: https://datasheets.maximintegrated.com/en/ds/MAX44000.pdf
+ *
+ * 7-bit I2C slave address 0x4a
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/acpi.h>
+
+#define MAX44000_DRV_NAME "max44000"
+
+/* Registers in datasheet order */
+#define MAX44000_REG_STATUS 0x00
+#define MAX44000_REG_CFG_MAIN 0x01
+#define MAX44000_REG_CFG_RX 0x02
+#define MAX44000_REG_CFG_TX 0x03
+#define MAX44000_REG_ALS_DATA_HI 0x04
+#define MAX44000_REG_ALS_DATA_LO 0x05
+#define MAX44000_REG_PRX_DATA 0x16
+#define MAX44000_REG_ALS_UPTHR_HI 0x06
+#define MAX44000_REG_ALS_UPTHR_LO 0x07
+#define MAX44000_REG_ALS_LOTHR_HI 0x08
+#define MAX44000_REG_ALS_LOTHR_LO 0x09
+#define MAX44000_REG_PST 0x0a
+#define MAX44000_REG_PRX_IND 0x0b
+#define MAX44000_REG_PRX_THR 0x0c
+#define MAX44000_REG_TRIM_GAIN_GREEN 0x0f
+#define MAX44000_REG_TRIM_GAIN_IR 0x10
+
+#define MAX44000_ALSDATA_OVERFLOW 0x4000
+
+#define MAX44000_REGMASK_READABLE 0x419fff
+#define MAX44000_REGMASK_WRITEABLE 0x019fce
+#define MAX44000_REGMASK_VOLATILE 0x400031
+
+struct max44000_data {
+ struct mutex lock;
+ struct i2c_client *client;
+ struct regmap *regmap;
+};
+
+/* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
+#define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5
+
+static const struct iio_chan_spec max44000_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static inline int max44000_read_alsval(struct max44000_data *data)
+{
+ u16 regval;
+ int ret;
+
+ regval = 0;
+ ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, ®val, 2);
+ if (ret < 0)
+ return ret;
+
+ regval = be16_to_cpu(regval);
+
+ /* Overflow is explained on datasheet page 17.
+ *
+ * It's a warning that either the G or IR channel has become saturated
+ * and that the value in the register is likely incorrect.
+ *
+ * The recommendation is to change the scale (ALSPGA).
+ * The driver just returns the max representable value.
+ */
+ if (regval & MAX44000_ALSDATA_OVERFLOW)
+ return 0x3FFF;
+
+ return regval;
+}
+
+static int max44000_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct max44000_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_LIGHT:
+ mutex_lock(&data->lock);
+ ret = max44000_read_alsval(data);
+ mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_LIGHT:
+ *val = 1;
+ *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
+ return IIO_VAL_FRACTIONAL_LOG2;
+
+ default:
+ return -EINVAL;
+ }
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info max44000_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = max44000_read_raw,
+};
+
+static bool max44000_readable_reg(struct device *dev, unsigned int reg)
+{
+ return (1 << reg) & MAX44000_REGMASK_READABLE;
+}
+
+static bool max44000_writeable_reg(struct device *dev, unsigned int reg)
+{
+ return (1 << reg) & MAX44000_REGMASK_WRITEABLE;
+}
+
+static bool max44000_volatile_reg(struct device *dev, unsigned int reg)
+{
+ return (1 << reg) & MAX44000_REGMASK_VOLATILE;
+}
+
+static bool max44000_precious_reg(struct device *dev, unsigned int reg)
+{
+ return reg == MAX44000_REG_STATUS;
+}
+
+/* Datasheet pages 9-10: */
+static const struct reg_default max44000_reg_defaults[] = {
+ { MAX44000_REG_CFG_MAIN, 0x24 },
+ /* Upper 4 bits are not documented but start as 1 on powerup
+ * Setting them to 0 causes proximity to misbehave so set them to 1
+ */
+ { MAX44000_REG_CFG_RX, 0xf0 },
+ { MAX44000_REG_CFG_TX, 0x00 },
+ { MAX44000_REG_ALS_UPTHR_HI, 0x00 },
+ { MAX44000_REG_ALS_UPTHR_LO, 0x00 },
+ { MAX44000_REG_ALS_LOTHR_HI, 0x00 },
+ { MAX44000_REG_ALS_LOTHR_LO, 0x00 },
+ { MAX44000_REG_PST, 0x00 },
+ { MAX44000_REG_PRX_IND, 0x00 },
+ { MAX44000_REG_PRX_THR, 0x00 },
+ { MAX44000_REG_TRIM_GAIN_GREEN, 0x80 },
+ { MAX44000_REG_TRIM_GAIN_IR, 0x80 },
+};
+
+static const struct regmap_config max44000_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = MAX44000_REG_PRX_DATA,
+ .readable_reg = max44000_readable_reg,
+ .writeable_reg = max44000_writeable_reg,
+ .volatile_reg = max44000_volatile_reg,
+ .precious_reg = max44000_precious_reg,
+
+ .use_single_rw = 1,
+ .cache_type = REGCACHE_FLAT,
+
+ .reg_defaults = max44000_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(max44000_reg_defaults),
+};
+
+static int max44000_force_write_defaults(struct max44000_data *data)
+{
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(max44000_reg_defaults); ++i) {
+ ret = regmap_write(data->regmap,
+ max44000_reg_defaults[i].reg,
+ max44000_reg_defaults[i].def);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int max44000_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct max44000_data *data;
+ struct iio_dev *indio_dev;
+ int ret, reg;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+ data = iio_priv(indio_dev);
+ data->regmap = devm_regmap_init_i2c(client, &max44000_regmap_config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(&client->dev, "regmap_init failed!\n");
+ return PTR_ERR(data->regmap);
+ }
+
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+ mutex_init(&data->lock);
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->info = &max44000_info;
+ indio_dev->name = MAX44000_DRV_NAME;
+ indio_dev->channels = max44000_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max44000_channels);
+
+ /* Read status at least once to clear the power-on-reset bit. */
+ ret = regmap_read(data->regmap, MAX44000_REG_STATUS, ®);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to read init status: %d\n", ret);
+ return ret;
+ }
+
+ /* The device has no reset command, write defaults explicitly. */
+ ret = max44000_force_write_defaults(data);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to write defaults: %d\n", ret);
+ return ret;
+ }
+
+ return iio_device_register(indio_dev);
+}
+
+static int max44000_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+ iio_device_unregister(indio_dev);
+ return 0;
+}
+
+static const struct i2c_device_id max44000_id[] = {
+ {"max44000", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max44000_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id max44000_of_match[] = {
+ { .compatible = "maxim,max44000" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max44000_of_match);
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id max44000_acpi_match[] = {
+ {"MAX44000", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, max44000_acpi_match);
+#endif
+
+static struct i2c_driver max44000_driver = {
+ .driver = {
+ .name = MAX44000_DRV_NAME,
+ .of_match_table = of_match_ptr(max44000_of_match),
+ .acpi_match_table = ACPI_PTR(max44000_acpi_match),
+ },
+ .probe = max44000_probe,
+ .remove = max44000_remove,
+ .id_table = max44000_id,
+};
+
+module_i2c_driver(max44000_driver);
+
+MODULE_AUTHOR("Crestez Dan Leonard <leonard.crestez@intel.com>");
+MODULE_DESCRIPTION("MAX44000 Ambient and Infrared Proximity Sensor");
+MODULE_LICENSE("GPL v2");
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Peter Meerwald-Stadler <pmeerw@pmeerw.net> |
|---|---|
| Date | 2016-04-07 21:50 +0200 |
| Subject | Re: [PATCH 1/5] max44000: Initial commit |
| Message-ID | <rlo6e-o4-9@gated-at.bofh.it> |
| In reply to | #1373527 |
> This just adds support for reporting illuminance with default settings.
>
> All default registers are written on probe because the device otherwise
> lacks a reset function.
comments below
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/light/Kconfig | 11 ++
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/max44000.c | 295 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 307 insertions(+)
> create mode 100644 drivers/iio/light/max44000.c
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index cfd3df8..42c15c1 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -320,4 +320,15 @@ config VCNL4000
> To compile this driver as a module, choose M here: the
> module will be called vcnl4000.
>
> +config MAX44000
> + tristate "MAX44000 Ambient and Infrared Proximity Sensor"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Say Y here if you want to build support for Maxim Integrated's
> + MAX44000 ambient and infrared proximity sensor device.
> +
> + To compile this driver as a module, choose M here:
> + the module will be called max44000.
> +
> endmenu
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index b2c3105..14b2f36 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_TCS3472) += tcs3472.o
> obj-$(CONFIG_TSL4531) += tsl4531.o
> obj-$(CONFIG_US5182D) += us5182d.o
> obj-$(CONFIG_VCNL4000) += vcnl4000.o
> +obj-$(CONFIG_MAX44000) += max44000.o
alphabetical order please
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> new file mode 100644
> index 0000000..7c12153
> --- /dev/null
> +++ b/drivers/iio/light/max44000.c
> @@ -0,0 +1,295 @@
> +/*
> + * MAX44000 Ambient and Infrared Proximity Sensor
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Data sheet: https://datasheets.maximintegrated.com/en/ds/MAX44000.pdf
> + *
> + * 7-bit I2C slave address 0x4a
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/util_macros.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/acpi.h>
> +
> +#define MAX44000_DRV_NAME "max44000"
> +
> +/* Registers in datasheet order */
> +#define MAX44000_REG_STATUS 0x00
> +#define MAX44000_REG_CFG_MAIN 0x01
> +#define MAX44000_REG_CFG_RX 0x02
> +#define MAX44000_REG_CFG_TX 0x03
> +#define MAX44000_REG_ALS_DATA_HI 0x04
> +#define MAX44000_REG_ALS_DATA_LO 0x05
> +#define MAX44000_REG_PRX_DATA 0x16
> +#define MAX44000_REG_ALS_UPTHR_HI 0x06
> +#define MAX44000_REG_ALS_UPTHR_LO 0x07
> +#define MAX44000_REG_ALS_LOTHR_HI 0x08
> +#define MAX44000_REG_ALS_LOTHR_LO 0x09
> +#define MAX44000_REG_PST 0x0a
> +#define MAX44000_REG_PRX_IND 0x0b
> +#define MAX44000_REG_PRX_THR 0x0c
> +#define MAX44000_REG_TRIM_GAIN_GREEN 0x0f
> +#define MAX44000_REG_TRIM_GAIN_IR 0x10
> +
> +#define MAX44000_ALSDATA_OVERFLOW 0x4000
> +
> +#define MAX44000_REGMASK_READABLE 0x419fff
> +#define MAX44000_REGMASK_WRITEABLE 0x019fce
> +#define MAX44000_REGMASK_VOLATILE 0x400031
> +
> +struct max44000_data {
> + struct mutex lock;
> + struct i2c_client *client;
> + struct regmap *regmap;
> +};
> +
> +/* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
> +#define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5
> +
> +static const struct iio_chan_spec max44000_channels[] = {
> + {
> + .type = IIO_LIGHT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + },
> +};
> +
> +static inline int max44000_read_alsval(struct max44000_data *data)
drop inline, let the compiler figure out if inlining is beneficial
> +{
> + u16 regval;
> + int ret;
> +
> + regval = 0;
needed?
> + ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, ®val, 2);
sizeof(regval)
> + if (ret < 0)
> + return ret;
> +
> + regval = be16_to_cpu(regval);
> +
> + /* Overflow is explained on datasheet page 17.
> + *
> + * It's a warning that either the G or IR channel has become saturated
> + * and that the value in the register is likely incorrect.
> + *
> + * The recommendation is to change the scale (ALSPGA).
> + * The driver just returns the max representable value.
> + */
> + if (regval & MAX44000_ALSDATA_OVERFLOW)
> + return 0x3FFF;
> +
> + return regval;
> +}
> +
> +static int max44000_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max44000_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_LIGHT:
> + mutex_lock(&data->lock);
> + ret = max44000_read_alsval(data);
> + mutex_unlock(&data->lock);
> + if (ret < 0)
> + return ret;
> + *val = ret;
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_LIGHT:
> + *val = 1;
> + *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info max44000_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = max44000_read_raw,
> +};
> +
> +static bool max44000_readable_reg(struct device *dev, unsigned int reg)
> +{
> + return (1 << reg) & MAX44000_REGMASK_READABLE;
> +}
> +
> +static bool max44000_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + return (1 << reg) & MAX44000_REGMASK_WRITEABLE;
> +}
> +
> +static bool max44000_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + return (1 << reg) & MAX44000_REGMASK_VOLATILE;
> +}
> +
> +static bool max44000_precious_reg(struct device *dev, unsigned int reg)
> +{
> + return reg == MAX44000_REG_STATUS;
> +}
> +
> +/* Datasheet pages 9-10: */
> +static const struct reg_default max44000_reg_defaults[] = {
> + { MAX44000_REG_CFG_MAIN, 0x24 },
> + /* Upper 4 bits are not documented but start as 1 on powerup
> + * Setting them to 0 causes proximity to misbehave so set them to 1
> + */
> + { MAX44000_REG_CFG_RX, 0xf0 },
> + { MAX44000_REG_CFG_TX, 0x00 },
> + { MAX44000_REG_ALS_UPTHR_HI, 0x00 },
> + { MAX44000_REG_ALS_UPTHR_LO, 0x00 },
> + { MAX44000_REG_ALS_LOTHR_HI, 0x00 },
> + { MAX44000_REG_ALS_LOTHR_LO, 0x00 },
> + { MAX44000_REG_PST, 0x00 },
> + { MAX44000_REG_PRX_IND, 0x00 },
> + { MAX44000_REG_PRX_THR, 0x00 },
> + { MAX44000_REG_TRIM_GAIN_GREEN, 0x80 },
> + { MAX44000_REG_TRIM_GAIN_IR, 0x80 },
> +};
> +
> +static const struct regmap_config max44000_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = MAX44000_REG_PRX_DATA,
> + .readable_reg = max44000_readable_reg,
> + .writeable_reg = max44000_writeable_reg,
> + .volatile_reg = max44000_volatile_reg,
> + .precious_reg = max44000_precious_reg,
> +
> + .use_single_rw = 1,
> + .cache_type = REGCACHE_FLAT,
> +
> + .reg_defaults = max44000_reg_defaults,
> + .num_reg_defaults = ARRAY_SIZE(max44000_reg_defaults),
> +};
> +
> +static int max44000_force_write_defaults(struct max44000_data *data)
> +{
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(max44000_reg_defaults); ++i) {
> + ret = regmap_write(data->regmap,
> + max44000_reg_defaults[i].reg,
> + max44000_reg_defaults[i].def);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int max44000_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct max44000_data *data;
> + struct iio_dev *indio_dev;
> + int ret, reg;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> + data = iio_priv(indio_dev);
> + data->regmap = devm_regmap_init_i2c(client, &max44000_regmap_config);
> + if (IS_ERR(data->regmap)) {
> + dev_err(&client->dev, "regmap_init failed!\n");
> + return PTR_ERR(data->regmap);
> + }
> +
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + mutex_init(&data->lock);
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &max44000_info;
> + indio_dev->name = MAX44000_DRV_NAME;
> + indio_dev->channels = max44000_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max44000_channels);
> +
> + /* Read status at least once to clear the power-on-reset bit. */
> + ret = regmap_read(data->regmap, MAX44000_REG_STATUS, ®);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "failed to read init status: %d\n", ret);
> + return ret;
> + }
> +
> + /* The device has no reset command, write defaults explicitly. */
> + ret = max44000_force_write_defaults(data);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "failed to write defaults: %d\n", ret);
> + return ret;
> + }
> +
> + return iio_device_register(indio_dev);
devm_ would work here to make _remove obsolete
> +}
> +
> +static int max44000_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> + iio_device_unregister(indio_dev);
> + return 0;
> +}
> +
> +static const struct i2c_device_id max44000_id[] = {
> + {"max44000", 0},
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max44000_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id max44000_of_match[] = {
> + { .compatible = "maxim,max44000" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max44000_of_match);
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id max44000_acpi_match[] = {
> + {"MAX44000", 0},
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, max44000_acpi_match);
> +#endif
> +
> +static struct i2c_driver max44000_driver = {
> + .driver = {
> + .name = MAX44000_DRV_NAME,
> + .of_match_table = of_match_ptr(max44000_of_match),
> + .acpi_match_table = ACPI_PTR(max44000_acpi_match),
> + },
> + .probe = max44000_probe,
> + .remove = max44000_remove,
> + .id_table = max44000_id,
> +};
> +
> +module_i2c_driver(max44000_driver);
> +
> +MODULE_AUTHOR("Crestez Dan Leonard <leonard.crestez@intel.com>");
> +MODULE_DESCRIPTION("MAX44000 Ambient and Infrared Proximity Sensor");
> +MODULE_LICENSE("GPL v2");
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 15:20 +0200 |
| Subject | Re: [PATCH 1/5] max44000: Initial commit |
| Message-ID | <rmnrt-5Be-41@gated-at.bofh.it> |
| In reply to | #1373709 |
On 07/04/16 20:48, Peter Meerwald-Stadler wrote:
>
>> This just adds support for reporting illuminance with default settings.
>>
>> All default registers are written on probe because the device otherwise
>> lacks a reset function.
>
> comments below
Mostly fine, but a few corners need cleaning up.
Also, I'm not keep on the brute force write everything. The driver should
cope with any values in those registers and deal with refreshing the
cache etc so that it can do so. Writing a bunch of defaults is rather a
brittle approach.
Jonathan
>
>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
>> ---
>> drivers/iio/light/Kconfig | 11 ++
>> drivers/iio/light/Makefile | 1 +
>> drivers/iio/light/max44000.c | 295 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 307 insertions(+)
>> create mode 100644 drivers/iio/light/max44000.c
>>
>> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
>> index cfd3df8..42c15c1 100644
>> --- a/drivers/iio/light/Kconfig
>> +++ b/drivers/iio/light/Kconfig
>> @@ -320,4 +320,15 @@ config VCNL4000
>> To compile this driver as a module, choose M here: the
>> module will be called vcnl4000.
>>
>> +config MAX44000
>> + tristate "MAX44000 Ambient and Infrared Proximity Sensor"
>> + depends on I2C
>> + select REGMAP_I2C
>> + help
>> + Say Y here if you want to build support for Maxim Integrated's
>> + MAX44000 ambient and infrared proximity sensor device.
>> +
>> + To compile this driver as a module, choose M here:
>> + the module will be called max44000.
>> +
>> endmenu
>> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
>> index b2c3105..14b2f36 100644
>> --- a/drivers/iio/light/Makefile
>> +++ b/drivers/iio/light/Makefile
>> @@ -30,3 +30,4 @@ obj-$(CONFIG_TCS3472) += tcs3472.o
>> obj-$(CONFIG_TSL4531) += tsl4531.o
>> obj-$(CONFIG_US5182D) += us5182d.o
>> obj-$(CONFIG_VCNL4000) += vcnl4000.o
>> +obj-$(CONFIG_MAX44000) += max44000.o
>
> alphabetical order please
>
>> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
>> new file mode 100644
>> index 0000000..7c12153
>> --- /dev/null
>> +++ b/drivers/iio/light/max44000.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * MAX44000 Ambient and Infrared Proximity Sensor
>> + *
>> + * Copyright (c) 2016, Intel Corporation.
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License. See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * Data sheet: https://datasheets.maximintegrated.com/en/ds/MAX44000.pdf
>> + *
>> + * 7-bit I2C slave address 0x4a
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/util_macros.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/acpi.h>
>> +
>> +#define MAX44000_DRV_NAME "max44000"
>> +
>> +/* Registers in datasheet order */
>> +#define MAX44000_REG_STATUS 0x00
>> +#define MAX44000_REG_CFG_MAIN 0x01
>> +#define MAX44000_REG_CFG_RX 0x02
>> +#define MAX44000_REG_CFG_TX 0x03
>> +#define MAX44000_REG_ALS_DATA_HI 0x04
>> +#define MAX44000_REG_ALS_DATA_LO 0x05
>> +#define MAX44000_REG_PRX_DATA 0x16
>> +#define MAX44000_REG_ALS_UPTHR_HI 0x06
>> +#define MAX44000_REG_ALS_UPTHR_LO 0x07
>> +#define MAX44000_REG_ALS_LOTHR_HI 0x08
>> +#define MAX44000_REG_ALS_LOTHR_LO 0x09
>> +#define MAX44000_REG_PST 0x0a
>> +#define MAX44000_REG_PRX_IND 0x0b
>> +#define MAX44000_REG_PRX_THR 0x0c
>> +#define MAX44000_REG_TRIM_GAIN_GREEN 0x0f
>> +#define MAX44000_REG_TRIM_GAIN_IR 0x10
>> +
>> +#define MAX44000_ALSDATA_OVERFLOW 0x4000
>> +
>> +#define MAX44000_REGMASK_READABLE 0x419fff
>> +#define MAX44000_REGMASK_WRITEABLE 0x019fce
>> +#define MAX44000_REGMASK_VOLATILE 0x400031
These are horrible! Do it as a switch over the relevant
registers in the functions below... Much easier to read / debug.
>> +
>> +struct max44000_data {
>> + struct mutex lock;
>> + struct i2c_client *client;
This client pointer isn't used outside probe and remove where it is easily
available anyway. Hence don't keep a copy in here.
>> + struct regmap *regmap;
>> +};
>> +
>> +/* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */
>> +#define MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2 5
>> +
>> +static const struct iio_chan_spec max44000_channels[] = {
>> + {
>> + .type = IIO_LIGHT,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>> + },
>> +};
>> +
>> +static inline int max44000_read_alsval(struct max44000_data *data)
>
> drop inline, let the compiler figure out if inlining is beneficial
>
>> +{
>> + u16 regval;
>> + int ret;
>> +
>> + regval = 0;
>
> needed?
>
>> + ret = regmap_bulk_read(data->regmap, MAX44000_REG_ALS_DATA_HI, ®val, 2);
>
> sizeof(regval)
>
>> + if (ret < 0)
>> + return ret;
>> +
>> + regval = be16_to_cpu(regval);
>> +
>> + /* Overflow is explained on datasheet page 17.
>> + *
>> + * It's a warning that either the G or IR channel has become saturated
>> + * and that the value in the register is likely incorrect.
>> + *
>> + * The recommendation is to change the scale (ALSPGA).
>> + * The driver just returns the max representable value.
>> + */
>> + if (regval & MAX44000_ALSDATA_OVERFLOW)
>> + return 0x3FFF;
>> +
>> + return regval;
>> +}
>> +
>> +static int max44000_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val, int *val2, long mask)
>> +{
>> + struct max44000_data *data = iio_priv(indio_dev);
>> + int ret;
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + switch (chan->type) {
>> + case IIO_LIGHT:
>> + mutex_lock(&data->lock);
>> + ret = max44000_read_alsval(data);
>> + mutex_unlock(&data->lock);
>> + if (ret < 0)
>> + return ret;
>> + *val = ret;
>> + return IIO_VAL_INT;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + case IIO_CHAN_INFO_SCALE:
>> + switch (chan->type) {
>> + case IIO_LIGHT:
>> + *val = 1;
>> + *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
>> + return IIO_VAL_FRACTIONAL_LOG2;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static const struct iio_info max44000_info = {
>> + .driver_module = THIS_MODULE,
>> + .read_raw = max44000_read_raw,
>> +};
>> +
>> +static bool max44000_readable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return (1 << reg) & MAX44000_REGMASK_READABLE;
See above. This is a really nasty and hard to review way of doing this.
switch (reg) {
REG1:
REG2:
REG3:
return true;
default:
return false;
may be more code, but it's easy to tell if it is right.
>> +}
>> +
>> +static bool max44000_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return (1 << reg) & MAX44000_REGMASK_WRITEABLE;
>> +}
>> +
>> +static bool max44000_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> + return (1 << reg) & MAX44000_REGMASK_VOLATILE;
>> +}
>> +
>> +static bool max44000_precious_reg(struct device *dev, unsigned int reg)
>> +{
>> + return reg == MAX44000_REG_STATUS;
>> +}
>> +
>> +/* Datasheet pages 9-10: */
>> +static const struct reg_default max44000_reg_defaults[] = {
>> + { MAX44000_REG_CFG_MAIN, 0x24 },
>> + /* Upper 4 bits are not documented but start as 1 on powerup
Multiline comment syntax please.
>> + * Setting them to 0 causes proximity to misbehave so set them to 1
>> + */
>> + { MAX44000_REG_CFG_RX, 0xf0 },
>> + { MAX44000_REG_CFG_TX, 0x00 },
>> + { MAX44000_REG_ALS_UPTHR_HI, 0x00 },
>> + { MAX44000_REG_ALS_UPTHR_LO, 0x00 },
>> + { MAX44000_REG_ALS_LOTHR_HI, 0x00 },
>> + { MAX44000_REG_ALS_LOTHR_LO, 0x00 },
>> + { MAX44000_REG_PST, 0x00 },
>> + { MAX44000_REG_PRX_IND, 0x00 },
>> + { MAX44000_REG_PRX_THR, 0x00 },
>> + { MAX44000_REG_TRIM_GAIN_GREEN, 0x80 },
>> + { MAX44000_REG_TRIM_GAIN_IR, 0x80 },
>> +};
>> +
>> +static const struct regmap_config max44000_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> +
>> + .max_register = MAX44000_REG_PRX_DATA,
>> + .readable_reg = max44000_readable_reg,
>> + .writeable_reg = max44000_writeable_reg,
>> + .volatile_reg = max44000_volatile_reg,
>> + .precious_reg = max44000_precious_reg,
>> +
>> + .use_single_rw = 1,
>> + .cache_type = REGCACHE_FLAT,
This always seems like a good idea, but tends to cause issues.
FLAT is really only meant for very high performance devices, you
are probably better with something else here. If you are doing this
deliberately to make the below writes actually occur, then please
add a comment here.
>> +
>> + .reg_defaults = max44000_reg_defaults,
>> + .num_reg_defaults = ARRAY_SIZE(max44000_reg_defaults),
>> +};
>> +
>> +static int max44000_force_write_defaults(struct max44000_data *data)
>> +{
>> + int i, ret;
>> +
>> + for (i = 0; i < ARRAY_SIZE(max44000_reg_defaults); ++i) {
>> + ret = regmap_write(data->regmap,
>> + max44000_reg_defaults[i].reg,
>> + max44000_reg_defaults[i].def);
Silly question, but if the cached value matches the values you are trying
to write here will this work?
There is a regcache_mark_dirty call that will ensure all registers in the
cache are read..
If you then need any particular values they should be explicitly written
on the assumption you have no idea what the state is. Brute force writing
all the defaults is nasty and doesn't give any information as to what
is happening.
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> +static int max44000_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct max44000_data *data;
>> + struct iio_dev *indio_dev;
>> + int ret, reg;
>> +
>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> + data = iio_priv(indio_dev);
>> + data->regmap = devm_regmap_init_i2c(client, &max44000_regmap_config);
>> + if (IS_ERR(data->regmap)) {
>> + dev_err(&client->dev, "regmap_init failed!\n");
>> + return PTR_ERR(data->regmap);
>> + }
>> +
>> + i2c_set_clientdata(client, indio_dev);
>> + data->client = client;
>> + mutex_init(&data->lock);
>> + indio_dev->dev.parent = &client->dev;
>> + indio_dev->info = &max44000_info;
>> + indio_dev->name = MAX44000_DRV_NAME;
>> + indio_dev->channels = max44000_channels;
>> + indio_dev->num_channels = ARRAY_SIZE(max44000_channels);
>> +
>> + /* Read status at least once to clear the power-on-reset bit. */
>> + ret = regmap_read(data->regmap, MAX44000_REG_STATUS, ®);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "failed to read init status: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* The device has no reset command, write defaults explicitly. */
>> + ret = max44000_force_write_defaults(data);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "failed to write defaults: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + return iio_device_register(indio_dev);
>
> devm_ would work here to make _remove obsolete
>
>> +}
>> +
>> +static int max44000_remove(struct i2c_client *client)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +
>> + iio_device_unregister(indio_dev);
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id max44000_id[] = {
>> + {"max44000", 0},
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, max44000_id);
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id max44000_of_match[] = {
>> + { .compatible = "maxim,max44000" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, max44000_of_match);
>> +#endif
>> +
>> +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id max44000_acpi_match[] = {
>> + {"MAX44000", 0},
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, max44000_acpi_match);
>> +#endif
>> +
>> +static struct i2c_driver max44000_driver = {
>> + .driver = {
>> + .name = MAX44000_DRV_NAME,
>> + .of_match_table = of_match_ptr(max44000_of_match),
>> + .acpi_match_table = ACPI_PTR(max44000_acpi_match),
>> + },
>> + .probe = max44000_probe,
>> + .remove = max44000_remove,
>> + .id_table = max44000_id,
>> +};
>> +
>> +module_i2c_driver(max44000_driver);
>> +
>> +MODULE_AUTHOR("Crestez Dan Leonard <leonard.crestez@intel.com>");
>> +MODULE_DESCRIPTION("MAX44000 Ambient and Infrared Proximity Sensor");
>> +MODULE_LICENSE("GPL v2");
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-11 17:20 +0200 |
| Subject | Re: [PATCH 1/5] max44000: Initial commit |
| Message-ID | <rmLN7-7Xx-1@gated-at.bofh.it> |
| In reply to | #1374767 |
On 04/10/2016 04:12 PM, Jonathan Cameron wrote:
> On 07/04/16 20:48, Peter Meerwald-Stadler wrote:
>>
>>> This just adds support for reporting illuminance with default settings.
>>>
>>> All default registers are written on probe because the device otherwise
>>> lacks a reset function.
>>
>> comments below
> Mostly fine, but a few corners need cleaning up.
>
> Also, I'm not keep on the brute force write everything. The driver should
> cope with any values in those registers and deal with refreshing the
> cache etc so that it can do so. Writing a bunch of defaults is rather a
> brittle approach.
But if the hardware is not in the expected state the driver won't report
correct values, at least not without reading scaling factors.
It's not clear what you mean by brittle?
>>> +struct max44000_data {
>>> + struct mutex lock;
>>> + struct i2c_client *client;
> This client pointer isn't used outside probe and remove where it is easily
> available anyway. Hence don't keep a copy in here.
Ok, will remove
>>> +static bool max44000_readable_reg(struct device *dev, unsigned int reg)
>>> +{
>>> + return (1 << reg) & MAX44000_REGMASK_READABLE;
> See above. This is a really nasty and hard to review way of doing this.
> switch (reg) {
> REG1:
> REG2:
> REG3:
> return true;
> default:
> return false;
>
> may be more code, but it's easy to tell if it is right.
Won't a switch result in larger executable code? Would it be acceptable
to just expand the REGMASK_* into a large or-ing of (1 <<
MAX44000_REG_*)? Then it would be clear in the source what's going on
but binary will be the same.
>>> +static const struct reg_default max44000_reg_defaults[] = {
>>> + { MAX44000_REG_CFG_MAIN, 0x24 },
>>> + /* Upper 4 bits are not documented but start as 1 on powerup
> Multiline comment syntax please.
Ok, I will fix this in all the patches.
>>> + .use_single_rw = 1,
>>> + .cache_type = REGCACHE_FLAT,
> This always seems like a good idea, but tends to cause issues.
> FLAT is really only meant for very high performance devices, you
> are probably better with something else here. If you are doing this
> deliberately to make the below writes actually occur, then please
> add a comment here.
I used REGCACHE_FLAT because my device has a very small number of
registers and I assume it uses less memory. Honestly it would make sense
for regmap to include a REGCACHE_AUTO cache_type and pick the cache
implementation automatically based on number of registers.
>>> +static int max44000_force_write_defaults(struct max44000_data *data)
>>> +{
>>> + int i, ret;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(max44000_reg_defaults); ++i) {
>>> + ret = regmap_write(data->regmap,
>>> + max44000_reg_defaults[i].reg,
>>> + max44000_reg_defaults[i].def);
> Silly question, but if the cached value matches the values you are trying
> to write here will this work?
Yes. It would not work otherwise since the regmap cache is explicitly
initialized with my listed defaults.
As far as I can tell regmap_write will always write to the hardware.
> There is a regcache_mark_dirty call that will ensure all registers in the
> cache are read..
The regcache_mark_dirty function is used to notify the cache that the
device has been reset to the known default values. I attempted to do
something like:
regcache_mark_dirty()
regcache_sync()
This doesn't work because this explicitly avoid overwriting known defaults.
> If you then need any particular values they should be explicitly written
> on the assumption you have no idea what the state is. Brute force writing
> all the defaults is nasty and doesn't give any information as to what
> is happening.
If the device had a reset command I should have used that, right? What
is happening is that I am implementing a reset command in software.
I can skip writing values like REG_TRIM_* which are used for calibration
and are not exposed by the driver. This would allow these values to be
configured externally using something like i2cset at boot time. Is that
what you mean?
I could also skip initializing scaling parameters but then stuff like
in_illuminance_scale would persist after rmmod/insmod. This seems
undesirable.
--
Regards,
Leonard
[toc] | [prev] | [next] | [standalone]
| From | Crestez Dan Leonard <leonard.crestez@intel.com> |
|---|---|
| Date | 2016-04-07 18:30 +0200 |
| Subject | [PATCH 3/5] max44000: Support controlling LED current output |
| Message-ID | <rlkYI-6F9-63@gated-at.bofh.it> |
| In reply to | #1373514 |
This is exposed as an output channel with "led" as an extend_name.
Other sensors also have support for controlling an external LED. It's
not clear that simply exposing an undecorated output channel is the
correct approach.
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
drivers/iio/light/max44000.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
index 10a0fe8..1dc10b9 100644
--- a/drivers/iio/light/max44000.c
+++ b/drivers/iio/light/max44000.c
@@ -85,6 +85,13 @@ static const struct iio_chan_spec max44000_channels[] = {
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
},
+ {
+ .type = IIO_CURRENT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .extend_name = "led",
+ .output = 1,
+ },
};
static inline int max44000_read_alsval(struct max44000_data *data)
@@ -124,6 +131,20 @@ static inline int max44000_write_led_current_raw(struct max44000_data *data, int
MAX44000_LED_CURRENT_MASK, val);
}
+static inline int max44000_read_led_current_raw(struct max44000_data *data)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, MAX44000_REG_CFG_TX, ®val);
+ if (ret < 0)
+ return ret;
+ regval &= MAX44000_LED_CURRENT_MASK;
+ if (regval >= 8)
+ regval -= 4;
+ return regval;
+}
+
static int max44000_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
@@ -153,12 +174,26 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
*val = regval;
return IIO_VAL_INT;
+ case IIO_CURRENT:
+ mutex_lock(&data->lock);
+ ret = max44000_read_led_current_raw(data);
+ mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ return IIO_VAL_INT;
+
default:
return -EINVAL;
}
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
+ case IIO_CURRENT:
+ /* Output register is in 10s of miliamps */
+ *val = 10;
+ return IIO_VAL_INT;
+
case IIO_LIGHT:
*val = 1;
*val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
@@ -173,9 +208,27 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
}
}
+static int max44000_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct max44000_data *data = iio_priv(indio_dev);
+ int ret;
+
+ if (mask == IIO_CHAN_INFO_RAW && chan->type == IIO_CURRENT) {
+ mutex_lock(&data->lock);
+ ret = max44000_write_led_current_raw(data, val);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ return -EINVAL;
+}
+
static const struct iio_info max44000_info = {
.driver_module = THIS_MODULE,
.read_raw = max44000_read_raw,
+ .write_raw = max44000_write_raw,
};
static bool max44000_readable_reg(struct device *dev, unsigned int reg)
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 15:20 +0200 |
| Subject | Re: [PATCH 3/5] max44000: Support controlling LED current output |
| Message-ID | <rmnrt-5Be-21@gated-at.bofh.it> |
| In reply to | #1373531 |
On 07/04/16 17:21, Crestez Dan Leonard wrote:
> This is exposed as an output channel with "led" as an extend_name.
>
> Other sensors also have support for controlling an external LED. It's
> not clear that simply exposing an undecorated output channel is the
> correct approach.
Agreed that this is less than ideal. Other suggestions welcome.
It gets complicated as some devices have multiple LEDs and they aren't
necessarily directly associated with a particular sensor.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/light/max44000.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c
> index 10a0fe8..1dc10b9 100644
> --- a/drivers/iio/light/max44000.c
> +++ b/drivers/iio/light/max44000.c
> @@ -85,6 +85,13 @@ static const struct iio_chan_spec max44000_channels[] = {
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> },
> + {
> + .type = IIO_CURRENT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .extend_name = "led",
> + .output = 1,
> + },
> };
>
> static inline int max44000_read_alsval(struct max44000_data *data)
> @@ -124,6 +131,20 @@ static inline int max44000_write_led_current_raw(struct max44000_data *data, int
> MAX44000_LED_CURRENT_MASK, val);
> }
>
> +static inline int max44000_read_led_current_raw(struct max44000_data *data)
> +{
> + unsigned int regval;
> + int ret;
> +
> + ret = regmap_read(data->regmap, MAX44000_REG_CFG_TX, ®val);
> + if (ret < 0)
> + return ret;
> + regval &= MAX44000_LED_CURRENT_MASK;
> + if (regval >= 8)
> + regval -= 4;
> + return regval;
> +}
> +
> static int max44000_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -153,12 +174,26 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
> *val = regval;
> return IIO_VAL_INT;
>
> + case IIO_CURRENT:
> + mutex_lock(&data->lock);
> + ret = max44000_read_led_current_raw(data);
> + mutex_unlock(&data->lock);
> + if (ret < 0)
> + return ret;
> + *val = ret;
> + return IIO_VAL_INT;
> +
> default:
> return -EINVAL;
> }
>
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
> + case IIO_CURRENT:
> + /* Output register is in 10s of miliamps */
> + *val = 10;
> + return IIO_VAL_INT;
> +
> case IIO_LIGHT:
> *val = 1;
> *val2 = MAX44000_ALS_TO_LUX_DEFAULT_FRACTION_LOG2;
> @@ -173,9 +208,27 @@ static int max44000_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int max44000_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct max44000_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + if (mask == IIO_CHAN_INFO_RAW && chan->type == IIO_CURRENT) {
> + mutex_lock(&data->lock);
> + ret = max44000_write_led_current_raw(data, val);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + return -EINVAL;
> +}
> +
> static const struct iio_info max44000_info = {
> .driver_module = THIS_MODULE,
> .read_raw = max44000_read_raw,
> + .write_raw = max44000_write_raw,
> };
>
> static bool max44000_readable_reg(struct device *dev, unsigned int reg)
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web