* [PATCH 0/6] fixes, cleanup and improvements for m62332
@ 2015-08-28 21:59 Hartmut Knaack
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
` (6 more replies)
0 siblings, 7 replies; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
A set of patches to address some issues I spotted during review.
Hartmut Knaack (6):
iio:dac:m62332: share scale and offset
iio:dac:m62332: shutdown on remove
iio:dac:m62332: use ARRAY_SIZE
iio:dac:m62332: drop unrequired variable
iio:dac:m62332: address some style issues
iio:dac:m62332: use dynamic scale
drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
1 file changed, 33 insertions(+), 30 deletions(-)
--
2.4.6
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-08-31 14:53 ` Jonathan Cameron
2015-10-11 14:40 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 2/6] iio:dac:m62332: shutdown on remove Hartmut Knaack
` (5 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
This device simply uses its Vcc as reference voltage, so the same scale
applies for all channels. Also offset doesn't appear to be different for
any channel. Represent this by switching these two attributes to
info_mask_shared_by_type.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
Hope you don't mind too much, that I inserted an extra tab in the whole
block rather than messing up style and cleaning it up later.
drivers/iio/dac/m62332.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index c23d7fa889ee..cffc0630ed32 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
.driver_module = THIS_MODULE,
};
-#define M62332_CHANNEL(chan) { \
- .type = IIO_VOLTAGE, \
- .indexed = 1, \
- .output = 1, \
- .channel = (chan), \
- .datasheet_name = "CH" #chan, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
- BIT(IIO_CHAN_INFO_SCALE) | \
- BIT(IIO_CHAN_INFO_OFFSET), \
+#define M62332_CHANNEL(chan) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .output = 1, \
+ .channel = (chan), \
+ .datasheet_name = "CH" #chan, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
}
static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/6] iio:dac:m62332: shutdown on remove
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-08-31 12:56 ` Daniel Baluta
2015-08-28 21:59 ` [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE Hartmut Knaack
` (4 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
The regulator framework requests to balance regulator_enable() calls with
regulator_disable() calls. To meet this requirement, set channels to 0 on
remove, which implies a regulator_disable() call in case that channel was
enabled.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
drivers/iio/dac/m62332.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index cffc0630ed32..c61720de8606 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -243,6 +243,8 @@ static int m62332_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
iio_map_array_unregister(indio_dev);
+ m62332_set_value(indio_dev, 0, 0);
+ m62332_set_value(indio_dev, 0, 1);
return 0;
}
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
2015-08-28 21:59 ` [PATCH 2/6] iio:dac:m62332: shutdown on remove Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-08-31 12:58 ` Daniel Baluta
2015-08-28 21:59 ` [PATCH 4/6] iio:dac:m62332: drop unrequired variable Hartmut Knaack
` (3 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
Make use of ARRAY_SIZE to prevent buffer issues.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
drivers/iio/dac/m62332.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index c61720de8606..fe750982b502 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -62,8 +62,8 @@ static int m62332_set_value(struct iio_dev *indio_dev,
goto out;
}
- res = i2c_master_send(client, outbuf, 2);
- if (res >= 0 && res != 2)
+ res = i2c_master_send(client, outbuf, ARRAY_SIZE(outbuf));
+ if (res >= 0 && res != ARRAY_SIZE(outbuf))
res = -EIO;
if (res < 0)
goto out;
@@ -212,7 +212,7 @@ static int m62332_probe(struct i2c_client *client,
/* establish that the iio_dev is a child of the i2c device */
indio_dev->dev.parent = &client->dev;
- indio_dev->num_channels = M62332_CHANNELS;
+ indio_dev->num_channels = ARRAY_SIZE(m62332_channels);
indio_dev->channels = m62332_channels;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &m62332_info;
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/6] iio:dac:m62332: drop unrequired variable
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
` (2 preceding siblings ...)
2015-08-28 21:59 ` [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-08-31 19:46 ` Daniel Baluta
2015-08-28 21:59 ` [PATCH 5/6] iio:dac:m62332: address some style issues Hartmut Knaack
` (2 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
A return variable is not required in _write_raw(), and dropping it reduces
complexity, as well.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
drivers/iio/dac/m62332.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index fe750982b502..1b65fc007bce 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -112,21 +112,17 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
static int m62332_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val, int val2, long mask)
{
- int ret;
-
switch (mask) {
case IIO_CHAN_INFO_RAW:
if (val < 0 || val > 255)
return -EINVAL;
- ret = m62332_set_value(indio_dev, val, chan->channel);
- break;
+ return m62332_set_value(indio_dev, val, chan->channel);
default:
- ret = -EINVAL;
break;
}
- return ret;
+ return -EINVAL;
}
#ifdef CONFIG_PM_SLEEP
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 5/6] iio:dac:m62332: address some style issues
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
` (3 preceding siblings ...)
2015-08-28 21:59 ` [PATCH 4/6] iio:dac:m62332: drop unrequired variable Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-10-11 14:45 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 6/6] iio:dac:m62332: use dynamic scale Hartmut Knaack
2015-08-31 15:00 ` [PATCH 0/6] fixes, cleanup and improvements for m62332 Jonathan Cameron
6 siblings, 1 reply; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
Fix some indentation issues and separate returns by empty lines (IIO
style). Also rename the channel mask in _read_raw() to mask.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
drivers/iio/dac/m62332.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index 1b65fc007bce..fdb3e042c14d 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -40,8 +40,7 @@ struct m62332_data {
#endif
};
-static int m62332_set_value(struct iio_dev *indio_dev,
- u8 val, int channel)
+static int m62332_set_value(struct iio_dev *indio_dev, u8 val, int channel)
{
struct m62332_data *data = iio_priv(indio_dev);
struct i2c_client *client = data->client;
@@ -87,30 +86,35 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val,
int *val2,
- long m)
+ long mask)
{
struct m62332_data *data = iio_priv(indio_dev);
- switch (m) {
+ switch (mask) {
case IIO_CHAN_INFO_SCALE:
/* Corresponds to Vref / 2^(bits) */
*val = data->vref_mv;
*val2 = 8;
+
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_RAW:
*val = data->raw[chan->channel];
+
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
*val = 1;
+
return IIO_VAL_INT;
default:
break;
}
+
return -EINVAL;
}
static int m62332_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan, int val, int val2, long mask)
+ struct iio_chan_spec const *chan, int val, int val2,
+ long mask)
{
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -195,6 +199,7 @@ static int m62332_probe(struct i2c_client *client,
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
return -ENOMEM;
+
data = iio_priv(indio_dev);
i2c_set_clientdata(client, indio_dev);
data->client = client;
@@ -230,6 +235,7 @@ static int m62332_probe(struct i2c_client *client,
err:
iio_map_array_unregister(indio_dev);
+
return ret;
}
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 6/6] iio:dac:m62332: use dynamic scale
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
` (4 preceding siblings ...)
2015-08-28 21:59 ` [PATCH 5/6] iio:dac:m62332: address some style issues Hartmut Knaack
@ 2015-08-28 21:59 ` Hartmut Knaack
2015-10-11 14:46 ` Jonathan Cameron
2015-08-31 15:00 ` [PATCH 0/6] fixes, cleanup and improvements for m62332 Jonathan Cameron
6 siblings, 1 reply; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-28 21:59 UTC (permalink / raw
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
Some regulators can supply multiple voltages. To take changing voltages
into account, the scale needs to be calculated on every read access.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
drivers/iio/dac/m62332.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
index fdb3e042c14d..76e8b044b979 100644
--- a/drivers/iio/dac/m62332.c
+++ b/drivers/iio/dac/m62332.c
@@ -31,7 +31,6 @@
struct m62332_data {
struct i2c_client *client;
- u16 vref_mv;
struct regulator *vcc;
struct mutex mutex;
u8 raw[M62332_CHANNELS];
@@ -89,11 +88,16 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
long mask)
{
struct m62332_data *data = iio_priv(indio_dev);
+ int ret;
switch (mask) {
case IIO_CHAN_INFO_SCALE:
/* Corresponds to Vref / 2^(bits) */
- *val = data->vref_mv;
+ ret = regulator_get_voltage(data->vcc);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000; /* mV */
*val2 = 8;
return IIO_VAL_FRACTIONAL_LOG2;
@@ -218,11 +222,6 @@ static int m62332_probe(struct i2c_client *client,
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &m62332_info;
- ret = regulator_get_voltage(data->vcc);
- if (ret < 0)
- return ret;
- data->vref_mv = ret / 1000; /* mV */
-
ret = iio_map_array_register(indio_dev, client->dev.platform_data);
if (ret < 0)
return ret;
--
2.4.6
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] iio:dac:m62332: shutdown on remove
2015-08-28 21:59 ` [PATCH 2/6] iio:dac:m62332: shutdown on remove Hartmut Knaack
@ 2015-08-31 12:56 ` Daniel Baluta
2015-08-31 19:26 ` Hartmut Knaack
2015-10-11 14:42 ` Jonathan Cameron
0 siblings, 2 replies; 26+ messages in thread
From: Daniel Baluta @ 2015-08-31 12:56 UTC (permalink / raw
To: Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
Peter Meerwald, Dmitry Eremin-Solenikov
On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> The regulator framework requests to balance regulator_enable() calls with
> regulator_disable() calls. To meet this requirement, set channels to 0 on
> remove, which implies a regulator_disable() call in case that channel was
> enabled.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/dac/m62332.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index cffc0630ed32..c61720de8606 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -243,6 +243,8 @@ static int m62332_remove(struct i2c_client *client)
>
> iio_device_unregister(indio_dev);
> iio_map_array_unregister(indio_dev);
> + m62332_set_value(indio_dev, 0, 0);
> + m62332_set_value(indio_dev, 0, 1);
>
> return 0;
> }
Wouldn't be nice to factor this two calls in a separate function?
thanks,
Daniel.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE
2015-08-28 21:59 ` [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE Hartmut Knaack
@ 2015-08-31 12:58 ` Daniel Baluta
2015-10-11 14:43 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Daniel Baluta @ 2015-08-31 12:58 UTC (permalink / raw
To: Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
Peter Meerwald, Dmitry Eremin-Solenikov
On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Make use of ARRAY_SIZE to prevent buffer issues.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/dac/m62332.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index c61720de8606..fe750982b502 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -62,8 +62,8 @@ static int m62332_set_value(struct iio_dev *indio_dev,
> goto out;
> }
>
> - res = i2c_master_send(client, outbuf, 2);
> - if (res >= 0 && res != 2)
> + res = i2c_master_send(client, outbuf, ARRAY_SIZE(outbuf));
> + if (res >= 0 && res != ARRAY_SIZE(outbuf))
> res = -EIO;
> if (res < 0)
> goto out;
> @@ -212,7 +212,7 @@ static int m62332_probe(struct i2c_client *client,
> /* establish that the iio_dev is a child of the i2c device */
> indio_dev->dev.parent = &client->dev;
>
> - indio_dev->num_channels = M62332_CHANNELS;
> + indio_dev->num_channels = ARRAY_SIZE(m62332_channels);
> indio_dev->channels = m62332_channels;
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->info = &m62332_info;
> --
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
@ 2015-08-31 14:53 ` Jonathan Cameron
2015-09-12 10:11 ` Jonathan Cameron
2015-10-11 14:40 ` Jonathan Cameron
1 sibling, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-08-31 14:53 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 28/08/15 22:59, Hartmut Knaack wrote:
> This device simply uses its Vcc as reference voltage, so the same scale
> applies for all channels. Also offset doesn't appear to be different for
> any channel. Represent this by switching these two attributes to
> info_mask_shared_by_type.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
This sort of tidy up is always a little interesting. Technically it
is an ABI change (be it one that changes from one possible representation
to a better one). Unfortunately technically we can't rely on users
using a library or similar that would hide this detail for them.
Still we can make the change if no one notices. Perhaps
Dmitry will want to comment on this however, so I'll let it sit for a while
longer.
Jonathan
> ---
> Hope you don't mind too much, that I inserted an extra tab in the whole
> block rather than messing up style and cleaning it up later.
>
> drivers/iio/dac/m62332.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index c23d7fa889ee..cffc0630ed32 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
> .driver_module = THIS_MODULE,
> };
>
> -#define M62332_CHANNEL(chan) { \
> - .type = IIO_VOLTAGE, \
> - .indexed = 1, \
> - .output = 1, \
> - .channel = (chan), \
> - .datasheet_name = "CH" #chan, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> - BIT(IIO_CHAN_INFO_SCALE) | \
> - BIT(IIO_CHAN_INFO_OFFSET), \
> +#define M62332_CHANNEL(chan) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = (chan), \
> + .datasheet_name = "CH" #chan, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> }
>
> static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] fixes, cleanup and improvements for m62332
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
` (5 preceding siblings ...)
2015-08-28 21:59 ` [PATCH 6/6] iio:dac:m62332: use dynamic scale Hartmut Knaack
@ 2015-08-31 15:00 ` Jonathan Cameron
2015-08-31 16:09 ` Dmitry Eremin-Solenikov
6 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-08-31 15:00 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 28/08/15 22:59, Hartmut Knaack wrote:
> A set of patches to address some issues I spotted during review.
>
> Hartmut Knaack (6):
> iio:dac:m62332: share scale and offset
> iio:dac:m62332: shutdown on remove
> iio:dac:m62332: use ARRAY_SIZE
> iio:dac:m62332: drop unrequired variable
> iio:dac:m62332: address some style issues
> iio:dac:m62332: use dynamic scale
>
> drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
> 1 file changed, 33 insertions(+), 30 deletions(-)
>
Other than letting this series sit on the list a few days longer,
for Dmitry or others to take a look at it, I'm happy with this series.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] fixes, cleanup and improvements for m62332
2015-08-31 15:00 ` [PATCH 0/6] fixes, cleanup and improvements for m62332 Jonathan Cameron
@ 2015-08-31 16:09 ` Dmitry Eremin-Solenikov
2015-09-12 10:13 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Dmitry Eremin-Solenikov @ 2015-08-31 16:09 UTC (permalink / raw
To: Jonathan Cameron
Cc: Hartmut Knaack, linux-iio, Lars-Peter Clausen, Peter Meerwald
Hello,
2015-08-31 18:00 GMT+03:00 Jonathan Cameron <jic23@kernel.org>:
> On 28/08/15 22:59, Hartmut Knaack wrote:
>> A set of patches to address some issues I spotted during review.
>>
>> Hartmut Knaack (6):
>> iio:dac:m62332: share scale and offset
>> iio:dac:m62332: shutdown on remove
>> iio:dac:m62332: use ARRAY_SIZE
>> iio:dac:m62332: drop unrequired variable
>> iio:dac:m62332: address some style issues
>> iio:dac:m62332: use dynamic scale
>>
>> drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
>> 1 file changed, 33 insertions(+), 30 deletions(-)
>>
> Other than letting this series sit on the list a few days longer,
> for Dmitry or others to take a look at it, I'm happy with this series.
The changes look good, however I'd like to test them on my hardware first.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] iio:dac:m62332: shutdown on remove
2015-08-31 12:56 ` Daniel Baluta
@ 2015-08-31 19:26 ` Hartmut Knaack
2015-10-11 14:42 ` Jonathan Cameron
1 sibling, 0 replies; 26+ messages in thread
From: Hartmut Knaack @ 2015-08-31 19:26 UTC (permalink / raw
To: Daniel Baluta
Cc: linux-iio@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
Peter Meerwald, Dmitry Eremin-Solenikov
Daniel Baluta schrieb am 31.08.2015 um 14:56:
> On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
>> The regulator framework requests to balance regulator_enable() calls with
>> regulator_disable() calls. To meet this requirement, set channels to 0 on
>> remove, which implies a regulator_disable() call in case that channel was
>> enabled.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>
> Acked-by: Daniel Baluta <daniel.baluta@intel.com>
>
>> ---
>> drivers/iio/dac/m62332.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>> index cffc0630ed32..c61720de8606 100644
>> --- a/drivers/iio/dac/m62332.c
>> +++ b/drivers/iio/dac/m62332.c
>> @@ -243,6 +243,8 @@ static int m62332_remove(struct i2c_client *client)
>>
>> iio_device_unregister(indio_dev);
>> iio_map_array_unregister(indio_dev);
>> + m62332_set_value(indio_dev, 0, 0);
>> + m62332_set_value(indio_dev, 0, 1);
>>
>> return 0;
>> }
>
> Wouldn't be nice to factor this two calls in a separate function?
>
I thought about factoring out the same two calls in _susped() and reuse
it here, as well (discarding error codes). But in contrast to the suspend
call, which gets aborted on an error, both regulators have to be disabled
in _remove() unconditionally.
And to just have a call here like _powerdown(), I'm not really convinced
the overhead/effort is worth the slight benefit. But I don't really mind,
either.
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] iio:dac:m62332: drop unrequired variable
2015-08-28 21:59 ` [PATCH 4/6] iio:dac:m62332: drop unrequired variable Hartmut Knaack
@ 2015-08-31 19:46 ` Daniel Baluta
2015-10-11 14:44 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Daniel Baluta @ 2015-08-31 19:46 UTC (permalink / raw
To: Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Jonathan Cameron, Lars-Peter Clausen,
Peter Meerwald, Dmitry Eremin-Solenikov
On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> A return variable is not required in _write_raw(), and dropping it reduces
> complexity, as well.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Initially forgot to CC linux-iio.
> ---
> drivers/iio/dac/m62332.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index fe750982b502..1b65fc007bce 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -112,21 +112,17 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
> static int m62332_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int val, int val2, long mask)
> {
> - int ret;
> -
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> if (val < 0 || val > 255)
> return -EINVAL;
>
> - ret = m62332_set_value(indio_dev, val, chan->channel);
> - break;
> + return m62332_set_value(indio_dev, val, chan->channel);
> default:
> - ret = -EINVAL;
> break;
> }
>
> - return ret;
> + return -EINVAL;
> }
>
> #ifdef CONFIG_PM_SLEEP
> --
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-08-31 14:53 ` Jonathan Cameron
@ 2015-09-12 10:11 ` Jonathan Cameron
2015-09-12 10:14 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-09-12 10:11 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 31/08/15 15:53, Jonathan Cameron wrote:
> On 28/08/15 22:59, Hartmut Knaack wrote:
>> This device simply uses its Vcc as reference voltage, so the same scale
>> applies for all channels. Also offset doesn't appear to be different for
>> any channel. Represent this by switching these two attributes to
>> info_mask_shared_by_type.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> This sort of tidy up is always a little interesting. Technically it
> is an ABI change (be it one that changes from one possible representation
> to a better one). Unfortunately technically we can't rely on users
> using a library or similar that would hide this detail for them.
>
> Still we can make the change if no one notices. Perhaps
> Dmitry will want to comment on this however, so I'll let it sit for a while
> longer.
>
> Jonathan
Applied to the togreg branch of iio.git. As ever, pushed out first as
testing for the autobuilders to play with it.
Thanks,
Jonathan
>> ---
>> Hope you don't mind too much, that I inserted an extra tab in the whole
>> block rather than messing up style and cleaning it up later.
>>
>> drivers/iio/dac/m62332.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>> index c23d7fa889ee..cffc0630ed32 100644
>> --- a/drivers/iio/dac/m62332.c
>> +++ b/drivers/iio/dac/m62332.c
>> @@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> -#define M62332_CHANNEL(chan) { \
>> - .type = IIO_VOLTAGE, \
>> - .indexed = 1, \
>> - .output = 1, \
>> - .channel = (chan), \
>> - .datasheet_name = "CH" #chan, \
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> - BIT(IIO_CHAN_INFO_SCALE) | \
>> - BIT(IIO_CHAN_INFO_OFFSET), \
>> +#define M62332_CHANNEL(chan) { \
>> + .type = IIO_VOLTAGE, \
>> + .indexed = 1, \
>> + .output = 1, \
>> + .channel = (chan), \
>> + .datasheet_name = "CH" #chan, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
>> + BIT(IIO_CHAN_INFO_OFFSET), \
>> }
>>
>> static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] fixes, cleanup and improvements for m62332
2015-08-31 16:09 ` Dmitry Eremin-Solenikov
@ 2015-09-12 10:13 ` Jonathan Cameron
2015-10-03 11:16 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-09-12 10:13 UTC (permalink / raw
To: Dmitry Eremin-Solenikov
Cc: Hartmut Knaack, linux-iio, Lars-Peter Clausen, Peter Meerwald
On 31/08/15 17:09, Dmitry Eremin-Solenikov wrote:
> Hello,
>
> 2015-08-31 18:00 GMT+03:00 Jonathan Cameron <jic23@kernel.org>:
>> On 28/08/15 22:59, Hartmut Knaack wrote:
>>> A set of patches to address some issues I spotted during review.
>>>
>>> Hartmut Knaack (6):
>>> iio:dac:m62332: share scale and offset
>>> iio:dac:m62332: shutdown on remove
>>> iio:dac:m62332: use ARRAY_SIZE
>>> iio:dac:m62332: drop unrequired variable
>>> iio:dac:m62332: address some style issues
>>> iio:dac:m62332: use dynamic scale
>>>
>>> drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
>>> 1 file changed, 33 insertions(+), 30 deletions(-)
>>>
>> Other than letting this series sit on the list a few days longer,
>> for Dmitry or others to take a look at it, I'm happy with this series.
>
> The changes look good, however I'd like to test them on my hardware first.
>
Hi Dmitry,
Sorry I'd forgotten you sent this so have just backed out the first couple
for now. Any idea when you might get a chance to test these out? We
aren't in a rush given timing relative to the cycle, but always
good to have some idea of how long things are likely to take!
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-09-12 10:11 ` Jonathan Cameron
@ 2015-09-12 10:14 ` Jonathan Cameron
2015-09-27 15:56 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-09-12 10:14 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 12/09/15 11:11, Jonathan Cameron wrote:
> On 31/08/15 15:53, Jonathan Cameron wrote:
>> On 28/08/15 22:59, Hartmut Knaack wrote:
>>> This device simply uses its Vcc as reference voltage, so the same scale
>>> applies for all channels. Also offset doesn't appear to be different for
>>> any channel. Represent this by switching these two attributes to
>>> info_mask_shared_by_type.
>>>
>>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>> This sort of tidy up is always a little interesting. Technically it
>> is an ABI change (be it one that changes from one possible representation
>> to a better one). Unfortunately technically we can't rely on users
>> using a library or similar that would hide this detail for them.
>>
>> Still we can make the change if no one notices. Perhaps
>> Dmitry will want to comment on this however, so I'll let it sit for a while
>> longer.
>>
>> Jonathan
> Applied to the togreg branch of iio.git. As ever, pushed out first as
> testing for the autobuilders to play with it.
Oops, had forgotten these were waiting on a tested-by from Dmitry.
Backed out again for now.
Sorry about that.
Jonathan
>
> Thanks,
>
> Jonathan
>>> ---
>>> Hope you don't mind too much, that I inserted an extra tab in the whole
>>> block rather than messing up style and cleaning it up later.
>>>
>>> drivers/iio/dac/m62332.c | 18 +++++++++---------
>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>>> index c23d7fa889ee..cffc0630ed32 100644
>>> --- a/drivers/iio/dac/m62332.c
>>> +++ b/drivers/iio/dac/m62332.c
>>> @@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
>>> .driver_module = THIS_MODULE,
>>> };
>>>
>>> -#define M62332_CHANNEL(chan) { \
>>> - .type = IIO_VOLTAGE, \
>>> - .indexed = 1, \
>>> - .output = 1, \
>>> - .channel = (chan), \
>>> - .datasheet_name = "CH" #chan, \
>>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> - BIT(IIO_CHAN_INFO_SCALE) | \
>>> - BIT(IIO_CHAN_INFO_OFFSET), \
>>> +#define M62332_CHANNEL(chan) { \
>>> + .type = IIO_VOLTAGE, \
>>> + .indexed = 1, \
>>> + .output = 1, \
>>> + .channel = (chan), \
>>> + .datasheet_name = "CH" #chan, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
>>> + BIT(IIO_CHAN_INFO_OFFSET), \
>>> }
>>>
>>> static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-09-12 10:14 ` Jonathan Cameron
@ 2015-09-27 15:56 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-09-27 15:56 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 12/09/15 11:14, Jonathan Cameron wrote:
> On 12/09/15 11:11, Jonathan Cameron wrote:
>> On 31/08/15 15:53, Jonathan Cameron wrote:
>>> On 28/08/15 22:59, Hartmut Knaack wrote:
>>>> This device simply uses its Vcc as reference voltage, so the same scale
>>>> applies for all channels. Also offset doesn't appear to be different for
>>>> any channel. Represent this by switching these two attributes to
>>>> info_mask_shared_by_type.
>>>>
>>>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>>> This sort of tidy up is always a little interesting. Technically it
>>> is an ABI change (be it one that changes from one possible representation
>>> to a better one). Unfortunately technically we can't rely on users
>>> using a library or similar that would hide this detail for them.
>>>
>>> Still we can make the change if no one notices. Perhaps
>>> Dmitry will want to comment on this however, so I'll let it sit for a while
>>> longer.
>>>
>>> Jonathan
>> Applied to the togreg branch of iio.git. As ever, pushed out first as
>> testing for the autobuilders to play with it.
> Oops, had forgotten these were waiting on a tested-by from Dmitry.
> Backed out again for now.
>
> Sorry about that.
>
Dmitry, any timescale for when you might get a chance to test this series?
Jonathan
> Jonathan
>>
>> Thanks,
>>
>> Jonathan
>>>> ---
>>>> Hope you don't mind too much, that I inserted an extra tab in the whole
>>>> block rather than messing up style and cleaning it up later.
>>>>
>>>> drivers/iio/dac/m62332.c | 18 +++++++++---------
>>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>>>> index c23d7fa889ee..cffc0630ed32 100644
>>>> --- a/drivers/iio/dac/m62332.c
>>>> +++ b/drivers/iio/dac/m62332.c
>>>> @@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
>>>> .driver_module = THIS_MODULE,
>>>> };
>>>>
>>>> -#define M62332_CHANNEL(chan) { \
>>>> - .type = IIO_VOLTAGE, \
>>>> - .indexed = 1, \
>>>> - .output = 1, \
>>>> - .channel = (chan), \
>>>> - .datasheet_name = "CH" #chan, \
>>>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>>> - BIT(IIO_CHAN_INFO_SCALE) | \
>>>> - BIT(IIO_CHAN_INFO_OFFSET), \
>>>> +#define M62332_CHANNEL(chan) { \
>>>> + .type = IIO_VOLTAGE, \
>>>> + .indexed = 1, \
>>>> + .output = 1, \
>>>> + .channel = (chan), \
>>>> + .datasheet_name = "CH" #chan, \
>>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
>>>> + BIT(IIO_CHAN_INFO_OFFSET), \
>>>> }
>>>>
>>>> static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] fixes, cleanup and improvements for m62332
2015-09-12 10:13 ` Jonathan Cameron
@ 2015-10-03 11:16 ` Jonathan Cameron
2015-10-11 14:38 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-03 11:16 UTC (permalink / raw
To: Dmitry Eremin-Solenikov
Cc: Hartmut Knaack, linux-iio, Lars-Peter Clausen, Peter Meerwald
On 12/09/15 11:13, Jonathan Cameron wrote:
> On 31/08/15 17:09, Dmitry Eremin-Solenikov wrote:
>> Hello,
>>
>> 2015-08-31 18:00 GMT+03:00 Jonathan Cameron <jic23@kernel.org>:
>>> On 28/08/15 22:59, Hartmut Knaack wrote:
>>>> A set of patches to address some issues I spotted during review.
>>>>
>>>> Hartmut Knaack (6):
>>>> iio:dac:m62332: share scale and offset
>>>> iio:dac:m62332: shutdown on remove
>>>> iio:dac:m62332: use ARRAY_SIZE
>>>> iio:dac:m62332: drop unrequired variable
>>>> iio:dac:m62332: address some style issues
>>>> iio:dac:m62332: use dynamic scale
>>>>
>>>> drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
>>>> 1 file changed, 33 insertions(+), 30 deletions(-)
>>>>
>>> Other than letting this series sit on the list a few days longer,
>>> for Dmitry or others to take a look at it, I'm happy with this series.
>>
>> The changes look good, however I'd like to test them on my hardware first.
>>
> Hi Dmitry,
>
> Sorry I'd forgotten you sent this so have just backed out the first couple
> for now. Any idea when you might get a chance to test these out? We
> aren't in a rush given timing relative to the cycle, but always
> good to have some idea of how long things are likely to take!
>
Hi Dmitry,
Any timescale on running the tests you wanted to on these?
If I don't hear from you before next weekend, I'll probably pick them up
and we can deal with any issues they are in.
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] fixes, cleanup and improvements for m62332
2015-10-03 11:16 ` Jonathan Cameron
@ 2015-10-11 14:38 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:38 UTC (permalink / raw
To: Dmitry Eremin-Solenikov
Cc: Hartmut Knaack, linux-iio, Lars-Peter Clausen, Peter Meerwald
On 03/10/15 12:16, Jonathan Cameron wrote:
> On 12/09/15 11:13, Jonathan Cameron wrote:
>> On 31/08/15 17:09, Dmitry Eremin-Solenikov wrote:
>>> Hello,
>>>
>>> 2015-08-31 18:00 GMT+03:00 Jonathan Cameron <jic23@kernel.org>:
>>>> On 28/08/15 22:59, Hartmut Knaack wrote:
>>>>> A set of patches to address some issues I spotted during review.
>>>>>
>>>>> Hartmut Knaack (6):
>>>>> iio:dac:m62332: share scale and offset
>>>>> iio:dac:m62332: shutdown on remove
>>>>> iio:dac:m62332: use ARRAY_SIZE
>>>>> iio:dac:m62332: drop unrequired variable
>>>>> iio:dac:m62332: address some style issues
>>>>> iio:dac:m62332: use dynamic scale
>>>>>
>>>>> drivers/iio/dac/m62332.c | 63 +++++++++++++++++++++++++-----------------------
>>>>> 1 file changed, 33 insertions(+), 30 deletions(-)
>>>>>
>>>> Other than letting this series sit on the list a few days longer,
>>>> for Dmitry or others to take a look at it, I'm happy with this series.
>>>
>>> The changes look good, however I'd like to test them on my hardware first.
>>>
>> Hi Dmitry,
>>
>> Sorry I'd forgotten you sent this so have just backed out the first couple
>> for now. Any idea when you might get a chance to test these out? We
>> aren't in a rush given timing relative to the cycle, but always
>> good to have some idea of how long things are likely to take!
>>
> Hi Dmitry,
>
> Any timescale on running the tests you wanted to on these?
> If I don't hear from you before next weekend, I'll probably pick them up
> and we can deal with any issues they are in.
>
> Jonathan
>
Ah well. Dmitry has it seems unfortunately dropped out of sight.
As I'm fairly happy these are straight forward I'm going to apply
them now.
Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] iio:dac:m62332: share scale and offset
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
2015-08-31 14:53 ` Jonathan Cameron
@ 2015-10-11 14:40 ` Jonathan Cameron
1 sibling, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:40 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 28/08/15 22:59, Hartmut Knaack wrote:
> This device simply uses its Vcc as reference voltage, so the same scale
> applies for all channels. Also offset doesn't appear to be different for
> any channel. Represent this by switching these two attributes to
> info_mask_shared_by_type.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git.
Thanks,
Jonathan
> ---
> Hope you don't mind too much, that I inserted an extra tab in the whole
> block rather than messing up style and cleaning it up later.
>
> drivers/iio/dac/m62332.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index c23d7fa889ee..cffc0630ed32 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -173,15 +173,15 @@ static const struct iio_info m62332_info = {
> .driver_module = THIS_MODULE,
> };
>
> -#define M62332_CHANNEL(chan) { \
> - .type = IIO_VOLTAGE, \
> - .indexed = 1, \
> - .output = 1, \
> - .channel = (chan), \
> - .datasheet_name = "CH" #chan, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> - BIT(IIO_CHAN_INFO_SCALE) | \
> - BIT(IIO_CHAN_INFO_OFFSET), \
> +#define M62332_CHANNEL(chan) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = (chan), \
> + .datasheet_name = "CH" #chan, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> }
>
> static const struct iio_chan_spec m62332_channels[M62332_CHANNELS] = {
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] iio:dac:m62332: shutdown on remove
2015-08-31 12:56 ` Daniel Baluta
2015-08-31 19:26 ` Hartmut Knaack
@ 2015-10-11 14:42 ` Jonathan Cameron
1 sibling, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:42 UTC (permalink / raw
To: Daniel Baluta, Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
On 31/08/15 13:56, Daniel Baluta wrote:
> On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
>> The regulator framework requests to balance regulator_enable() calls with
>> regulator_disable() calls. To meet this requirement, set channels to 0 on
>> remove, which implies a regulator_disable() call in case that channel was
>> enabled.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>
> Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git - as ever initially
pushed out as testing for the autobuilders to mess around with it.
Jonathan
>
>> ---
>> drivers/iio/dac/m62332.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>> index cffc0630ed32..c61720de8606 100644
>> --- a/drivers/iio/dac/m62332.c
>> +++ b/drivers/iio/dac/m62332.c
>> @@ -243,6 +243,8 @@ static int m62332_remove(struct i2c_client *client)
>>
>> iio_device_unregister(indio_dev);
>> iio_map_array_unregister(indio_dev);
>> + m62332_set_value(indio_dev, 0, 0);
>> + m62332_set_value(indio_dev, 0, 1);
>>
>> return 0;
>> }
>
> Wouldn't be nice to factor this two calls in a separate function?
>
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE
2015-08-31 12:58 ` Daniel Baluta
@ 2015-10-11 14:43 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:43 UTC (permalink / raw
To: Daniel Baluta, Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
On 31/08/15 13:58, Daniel Baluta wrote:
> On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
>> Make use of ARRAY_SIZE to prevent buffer issues.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>
> Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git - pushed out as testing
for now.
J
>
>> ---
>> drivers/iio/dac/m62332.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>> index c61720de8606..fe750982b502 100644
>> --- a/drivers/iio/dac/m62332.c
>> +++ b/drivers/iio/dac/m62332.c
>> @@ -62,8 +62,8 @@ static int m62332_set_value(struct iio_dev *indio_dev,
>> goto out;
>> }
>>
>> - res = i2c_master_send(client, outbuf, 2);
>> - if (res >= 0 && res != 2)
>> + res = i2c_master_send(client, outbuf, ARRAY_SIZE(outbuf));
>> + if (res >= 0 && res != ARRAY_SIZE(outbuf))
>> res = -EIO;
>> if (res < 0)
>> goto out;
>> @@ -212,7 +212,7 @@ static int m62332_probe(struct i2c_client *client,
>> /* establish that the iio_dev is a child of the i2c device */
>> indio_dev->dev.parent = &client->dev;
>>
>> - indio_dev->num_channels = M62332_CHANNELS;
>> + indio_dev->num_channels = ARRAY_SIZE(m62332_channels);
>> indio_dev->channels = m62332_channels;
>> indio_dev->modes = INDIO_DIRECT_MODE;
>> indio_dev->info = &m62332_info;
>> --
>> 2.4.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] iio:dac:m62332: drop unrequired variable
2015-08-31 19:46 ` Daniel Baluta
@ 2015-10-11 14:44 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:44 UTC (permalink / raw
To: Daniel Baluta, Hartmut Knaack
Cc: linux-iio@vger.kernel.org, Lars-Peter Clausen, Peter Meerwald,
Dmitry Eremin-Solenikov
On 31/08/15 20:46, Daniel Baluta wrote:
> On Sat, Aug 29, 2015 at 12:59 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
>> A return variable is not required in _write_raw(), and dropping it reduces
>> complexity, as well.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>
> Acked-by: Daniel Baluta <daniel.baluta@intel.com>
>
> Initially forgot to CC linux-iio.
Applied.
>
>> ---
>> drivers/iio/dac/m62332.c | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
>> index fe750982b502..1b65fc007bce 100644
>> --- a/drivers/iio/dac/m62332.c
>> +++ b/drivers/iio/dac/m62332.c
>> @@ -112,21 +112,17 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
>> static int m62332_write_raw(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan, int val, int val2, long mask)
>> {
>> - int ret;
>> -
>> switch (mask) {
>> case IIO_CHAN_INFO_RAW:
>> if (val < 0 || val > 255)
>> return -EINVAL;
>>
>> - ret = m62332_set_value(indio_dev, val, chan->channel);
>> - break;
>> + return m62332_set_value(indio_dev, val, chan->channel);
>> default:
>> - ret = -EINVAL;
>> break;
>> }
>>
>> - return ret;
>> + return -EINVAL;
>> }
>>
>> #ifdef CONFIG_PM_SLEEP
>> --
>> 2.4.6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] iio:dac:m62332: address some style issues
2015-08-28 21:59 ` [PATCH 5/6] iio:dac:m62332: address some style issues Hartmut Knaack
@ 2015-10-11 14:45 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:45 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 28/08/15 22:59, Hartmut Knaack wrote:
> Fix some indentation issues and separate returns by empty lines (IIO
> style). Also rename the channel mask in _read_raw() to mask.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/dac/m62332.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index 1b65fc007bce..fdb3e042c14d 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -40,8 +40,7 @@ struct m62332_data {
> #endif
> };
>
> -static int m62332_set_value(struct iio_dev *indio_dev,
> - u8 val, int channel)
> +static int m62332_set_value(struct iio_dev *indio_dev, u8 val, int channel)
> {
> struct m62332_data *data = iio_priv(indio_dev);
> struct i2c_client *client = data->client;
> @@ -87,30 +86,35 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val,
> int *val2,
> - long m)
> + long mask)
> {
> struct m62332_data *data = iio_priv(indio_dev);
>
> - switch (m) {
> + switch (mask) {
> case IIO_CHAN_INFO_SCALE:
> /* Corresponds to Vref / 2^(bits) */
> *val = data->vref_mv;
> *val2 = 8;
> +
> return IIO_VAL_FRACTIONAL_LOG2;
> case IIO_CHAN_INFO_RAW:
> *val = data->raw[chan->channel];
> +
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_OFFSET:
> *val = 1;
> +
> return IIO_VAL_INT;
> default:
> break;
> }
> +
> return -EINVAL;
> }
>
> static int m62332_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan, int val, int val2, long mask)
> + struct iio_chan_spec const *chan, int val, int val2,
> + long mask)
> {
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -195,6 +199,7 @@ static int m62332_probe(struct i2c_client *client,
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> if (!indio_dev)
> return -ENOMEM;
> +
> data = iio_priv(indio_dev);
> i2c_set_clientdata(client, indio_dev);
> data->client = client;
> @@ -230,6 +235,7 @@ static int m62332_probe(struct i2c_client *client,
>
> err:
> iio_map_array_unregister(indio_dev);
> +
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 6/6] iio:dac:m62332: use dynamic scale
2015-08-28 21:59 ` [PATCH 6/6] iio:dac:m62332: use dynamic scale Hartmut Knaack
@ 2015-10-11 14:46 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2015-10-11 14:46 UTC (permalink / raw
To: Hartmut Knaack, linux-iio
Cc: Lars-Peter Clausen, Peter Meerwald, Dmitry Eremin-Solenikov
On 28/08/15 22:59, Hartmut Knaack wrote:
> Some regulators can supply multiple voltages. To take changing voltages
> into account, the scale needs to be calculated on every read access.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
This is a pretty common situation, but just because it is done
less than ideally in a lot of places, doesn't mean we shouldn't
fix this one!
Applied.
Jonathan
> ---
> drivers/iio/dac/m62332.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/dac/m62332.c b/drivers/iio/dac/m62332.c
> index fdb3e042c14d..76e8b044b979 100644
> --- a/drivers/iio/dac/m62332.c
> +++ b/drivers/iio/dac/m62332.c
> @@ -31,7 +31,6 @@
>
> struct m62332_data {
> struct i2c_client *client;
> - u16 vref_mv;
> struct regulator *vcc;
> struct mutex mutex;
> u8 raw[M62332_CHANNELS];
> @@ -89,11 +88,16 @@ static int m62332_read_raw(struct iio_dev *indio_dev,
> long mask)
> {
> struct m62332_data *data = iio_priv(indio_dev);
> + int ret;
>
> switch (mask) {
> case IIO_CHAN_INFO_SCALE:
> /* Corresponds to Vref / 2^(bits) */
> - *val = data->vref_mv;
> + ret = regulator_get_voltage(data->vcc);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000; /* mV */
> *val2 = 8;
>
> return IIO_VAL_FRACTIONAL_LOG2;
> @@ -218,11 +222,6 @@ static int m62332_probe(struct i2c_client *client,
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->info = &m62332_info;
>
> - ret = regulator_get_voltage(data->vcc);
> - if (ret < 0)
> - return ret;
> - data->vref_mv = ret / 1000; /* mV */
> -
> ret = iio_map_array_register(indio_dev, client->dev.platform_data);
> if (ret < 0)
> return ret;
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-10-11 14:46 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-28 21:59 [PATCH 0/6] fixes, cleanup and improvements for m62332 Hartmut Knaack
2015-08-28 21:59 ` [PATCH 1/6] iio:dac:m62332: share scale and offset Hartmut Knaack
2015-08-31 14:53 ` Jonathan Cameron
2015-09-12 10:11 ` Jonathan Cameron
2015-09-12 10:14 ` Jonathan Cameron
2015-09-27 15:56 ` Jonathan Cameron
2015-10-11 14:40 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 2/6] iio:dac:m62332: shutdown on remove Hartmut Knaack
2015-08-31 12:56 ` Daniel Baluta
2015-08-31 19:26 ` Hartmut Knaack
2015-10-11 14:42 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 3/6] iio:dac:m62332: use ARRAY_SIZE Hartmut Knaack
2015-08-31 12:58 ` Daniel Baluta
2015-10-11 14:43 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 4/6] iio:dac:m62332: drop unrequired variable Hartmut Knaack
2015-08-31 19:46 ` Daniel Baluta
2015-10-11 14:44 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 5/6] iio:dac:m62332: address some style issues Hartmut Knaack
2015-10-11 14:45 ` Jonathan Cameron
2015-08-28 21:59 ` [PATCH 6/6] iio:dac:m62332: use dynamic scale Hartmut Knaack
2015-10-11 14:46 ` Jonathan Cameron
2015-08-31 15:00 ` [PATCH 0/6] fixes, cleanup and improvements for m62332 Jonathan Cameron
2015-08-31 16:09 ` Dmitry Eremin-Solenikov
2015-09-12 10:13 ` Jonathan Cameron
2015-10-03 11:16 ` Jonathan Cameron
2015-10-11 14:38 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.