All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iio: sx9310: Support ACPI property
@ 2021-02-05 21:58 Gwendal Grignou
  2021-02-06 14:58 ` Jonathan Cameron
  0 siblings, 1 reply; 2+ messages in thread
From: Gwendal Grignou @ 2021-02-05 21:58 UTC (permalink / raw)
  To: jic23, lars, swboyd, campello; +Cc: linux-iio, Gwendal Grignou

Use device_property_read_... to support both device tree and ACPI
bindings.

Add support for variable array per documentation
("iio/proximity/semtech,sx9310.yaml").

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 Changes since v1:
   Use device_property_count_u32(...) instead of device_property_read_u32_array(..., NULL, 0)

 drivers/iio/proximity/sx9310.c | 35 +++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index 6a04959df35e5..bba9dc6ac844d 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -20,6 +20,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
@@ -1215,31 +1216,35 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev)
 }
 
 static const struct sx9310_reg_default *
-sx9310_get_default_reg(struct sx9310_data *data, int i,
+sx9310_get_default_reg(struct device *dev, int i,
 		       struct sx9310_reg_default *reg_def)
 {
-	int ret;
-	const struct device_node *np = data->client->dev.of_node;
+	int ret, count;
 	u32 combined[SX9310_NUM_CHANNELS] = { 4, 4, 4, 4 };
 	unsigned long comb_mask = 0;
 	const char *res;
 	u32 start = 0, raw = 0, pos = 0;
 
 	memcpy(reg_def, &sx9310_default_regs[i], sizeof(*reg_def));
-	if (!np)
-		return reg_def;
-
 	switch (reg_def->reg) {
 	case SX9310_REG_PROX_CTRL2:
-		if (of_property_read_bool(np, "semtech,cs0-ground")) {
+		if (device_property_read_bool(dev, "semtech,cs0-ground")) {
 			reg_def->def &= ~SX9310_REG_PROX_CTRL2_SHIELDEN_MASK;
 			reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND;
 		}
 
 		reg_def->def &= ~SX9310_REG_PROX_CTRL2_COMBMODE_MASK;
-		of_property_read_u32_array(np, "semtech,combined-sensors",
-					   combined, ARRAY_SIZE(combined));
-		for (i = 0; i < ARRAY_SIZE(combined); i++) {
+		count = device_property_count_u32(dev, "semtech,combined-sensors");
+		if (count > 0 && count <= ARRAY_SIZE(combined))
+			ret = device_property_read_u32_array(dev,
+					"semtech,combined-sensors", combined,
+					count);
+		else
+			ret = -EINVAL;
+		if (ret)
+			count = ARRAY_SIZE(combined);
+
+		for (i = 0; i < count; i++) {
 			if (combined[i] <= SX9310_NUM_CHANNELS)
 				comb_mask |= BIT(combined[i]);
 		}
@@ -1256,7 +1261,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
 
 		break;
 	case SX9310_REG_PROX_CTRL4:
-		ret = of_property_read_string(np, "semtech,resolution", &res);
+		ret = device_property_read_string(dev, "semtech,resolution", &res);
 		if (ret)
 			break;
 
@@ -1280,7 +1285,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
 
 		break;
 	case SX9310_REG_PROX_CTRL5:
-		ret = of_property_read_u32(np, "semtech,startup-sensor", &start);
+		ret = device_property_read_u32(dev, "semtech,startup-sensor", &start);
 		if (ret) {
 			start = FIELD_GET(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK,
 					  reg_def->def);
@@ -1290,7 +1295,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
 		reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK,
 					   start);
 
-		ret = of_property_read_u32(np, "semtech,proxraw-strength", &raw);
+		ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw);
 		if (ret) {
 			raw = FIELD_GET(SX9310_REG_PROX_CTRL5_RAWFILT_MASK,
 					reg_def->def);
@@ -1303,7 +1308,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
 					   raw);
 		break;
 	case SX9310_REG_PROX_CTRL7:
-		ret = of_property_read_u32(np, "semtech,avg-pos-strength", &pos);
+		ret = device_property_read_u32(dev, "semtech,avg-pos-strength", &pos);
 		if (ret)
 			break;
 
@@ -1339,7 +1344,7 @@ static int sx9310_init_device(struct iio_dev *indio_dev)
 
 	/* Program some sane defaults. */
 	for (i = 0; i < ARRAY_SIZE(sx9310_default_regs); i++) {
-		initval = sx9310_get_default_reg(data, i, &tmp);
+		initval = sx9310_get_default_reg(&indio_dev->dev, i, &tmp);
 		ret = regmap_write(data->regmap, initval->reg, initval->def);
 		if (ret)
 			return ret;
-- 
2.30.0.478.g8a0d178c01-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH v2] iio: sx9310: Support ACPI property
  2021-02-05 21:58 [PATCH v2] iio: sx9310: Support ACPI property Gwendal Grignou
@ 2021-02-06 14:58 ` Jonathan Cameron
  0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2021-02-06 14:58 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: lars, swboyd, campello, linux-iio

On Fri,  5 Feb 2021 13:58:11 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:

> Use device_property_read_... to support both device tree and ACPI
> bindings.
> 
> Add support for variable array per documentation
> ("iio/proximity/semtech,sx9310.yaml").
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  Changes since v1:
>    Use device_property_count_u32(...) instead of device_property_read_u32_array(..., NULL, 0)
> 
>  drivers/iio/proximity/sx9310.c | 35 +++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index 6a04959df35e5..bba9dc6ac844d 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -20,6 +20,7 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/pm.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> @@ -1215,31 +1216,35 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev)
>  }
>  
>  static const struct sx9310_reg_default *
> -sx9310_get_default_reg(struct sx9310_data *data, int i,
> +sx9310_get_default_reg(struct device *dev, int i,
>  		       struct sx9310_reg_default *reg_def)
>  {
> -	int ret;
> -	const struct device_node *np = data->client->dev.of_node;
> +	int ret, count;
>  	u32 combined[SX9310_NUM_CHANNELS] = { 4, 4, 4, 4 };
>  	unsigned long comb_mask = 0;
>  	const char *res;
>  	u32 start = 0, raw = 0, pos = 0;
>  
>  	memcpy(reg_def, &sx9310_default_regs[i], sizeof(*reg_def));
> -	if (!np)
> -		return reg_def;
> -
>  	switch (reg_def->reg) {
>  	case SX9310_REG_PROX_CTRL2:
> -		if (of_property_read_bool(np, "semtech,cs0-ground")) {
> +		if (device_property_read_bool(dev, "semtech,cs0-ground")) {
>  			reg_def->def &= ~SX9310_REG_PROX_CTRL2_SHIELDEN_MASK;
>  			reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND;
>  		}
>  
>  		reg_def->def &= ~SX9310_REG_PROX_CTRL2_COMBMODE_MASK;
> -		of_property_read_u32_array(np, "semtech,combined-sensors",
> -					   combined, ARRAY_SIZE(combined));
> -		for (i = 0; i < ARRAY_SIZE(combined); i++) {
> +		count = device_property_count_u32(dev, "semtech,combined-sensors");
> +		if (count > 0 && count <= ARRAY_SIZE(combined))
> +			ret = device_property_read_u32_array(dev,
> +					"semtech,combined-sensors", combined,
> +					count);
> +		else
> +			ret = -EINVAL;
> +		if (ret)
> +			count = ARRAY_SIZE(combined);

This is odd enough please give a comment on why we would set count to
the array size on error in reading the array.
(obviously we do that before by ignoring the error, but still good
to improve clarity on what is going on here - I assume we are just
falling back to defaults...)

Also, this crossed with Stephen continuing the thread on v1 and suggesting
an alternative form so I'll wait for that discussion to be resolved.

Thanks,

Jonathan


> +
> +		for (i = 0; i < count; i++) {
>  			if (combined[i] <= SX9310_NUM_CHANNELS)
>  				comb_mask |= BIT(combined[i]);
>  		}
> @@ -1256,7 +1261,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
>  
>  		break;
>  	case SX9310_REG_PROX_CTRL4:
> -		ret = of_property_read_string(np, "semtech,resolution", &res);
> +		ret = device_property_read_string(dev, "semtech,resolution", &res);
>  		if (ret)
>  			break;
>  
> @@ -1280,7 +1285,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
>  
>  		break;
>  	case SX9310_REG_PROX_CTRL5:
> -		ret = of_property_read_u32(np, "semtech,startup-sensor", &start);
> +		ret = device_property_read_u32(dev, "semtech,startup-sensor", &start);
>  		if (ret) {
>  			start = FIELD_GET(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK,
>  					  reg_def->def);
> @@ -1290,7 +1295,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
>  		reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK,
>  					   start);
>  
> -		ret = of_property_read_u32(np, "semtech,proxraw-strength", &raw);
> +		ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw);
>  		if (ret) {
>  			raw = FIELD_GET(SX9310_REG_PROX_CTRL5_RAWFILT_MASK,
>  					reg_def->def);
> @@ -1303,7 +1308,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
>  					   raw);
>  		break;
>  	case SX9310_REG_PROX_CTRL7:
> -		ret = of_property_read_u32(np, "semtech,avg-pos-strength", &pos);
> +		ret = device_property_read_u32(dev, "semtech,avg-pos-strength", &pos);
>  		if (ret)
>  			break;
>  
> @@ -1339,7 +1344,7 @@ static int sx9310_init_device(struct iio_dev *indio_dev)
>  
>  	/* Program some sane defaults. */
>  	for (i = 0; i < ARRAY_SIZE(sx9310_default_regs); i++) {
> -		initval = sx9310_get_default_reg(data, i, &tmp);
> +		initval = sx9310_get_default_reg(&indio_dev->dev, i, &tmp);
>  		ret = regmap_write(data->regmap, initval->reg, initval->def);
>  		if (ret)
>  			return ret;


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-06 14:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 21:58 [PATCH v2] iio: sx9310: Support ACPI property Gwendal Grignou
2021-02-06 14:58 ` 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.