All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] iio: adc: meson: a few improvements
@ 2024-03-23 23:13 ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

This series contains three improvements to the meson SAR ADC driver.
None of them are meant to change the existing behavior. The goal is
to make the driver code easier to read and understand.


Martin Blumenstingl (3):
  iio: adc: meson: fix voltage reference selection field name typo
  iio: adc: meson: consistently use bool/enum in struct
    meson_sar_adc_param
  iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access

 drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 27+ messages in thread
* [PATCH v1 0/3] iio: adc: meson: a few improvements
@ 2024-03-23 23:13 ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

This series contains three improvements to the meson SAR ADC driver.
None of them are meant to change the existing behavior. The goal is
to make the driver code easier to read and understand.


Martin Blumenstingl (3):
  iio: adc: meson: fix voltage reference selection field name typo
  iio: adc: meson: consistently use bool/enum in struct
    meson_sar_adc_param
  iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access

 drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* [PATCH v1 0/3] iio: adc: meson: a few improvements
@ 2024-03-23 23:13 ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

This series contains three improvements to the meson SAR ADC driver.
None of them are meant to change the existing behavior. The goal is
to make the driver code easier to read and understand.


Martin Blumenstingl (3):
  iio: adc: meson: fix voltage reference selection field name typo
  iio: adc: meson: consistently use bool/enum in struct
    meson_sar_adc_param
  iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access

 drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

-- 
2.44.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
* [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
  2024-03-23 23:13 ` Martin Blumenstingl
  (?)
@ 2024-03-23 23:13   ` Martin Blumenstingl
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

The field should be called "vref_voltage", without a typo in the word
voltage. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 13b473d8c6c7..2615d74534df 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -327,7 +327,7 @@ struct meson_sar_adc_param {
 	u8					vref_select;
 	u8					cmv_select;
 	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_volatge;
+	enum meson_sar_adc_vref_sel		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		}
 
 		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
-				    priv->param->vref_volatge);
+				    priv->param->vref_voltage);
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
@@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 	.cmv_select = 1,
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

The field should be called "vref_voltage", without a typo in the word
voltage. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 13b473d8c6c7..2615d74534df 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -327,7 +327,7 @@ struct meson_sar_adc_param {
 	u8					vref_select;
 	u8					cmv_select;
 	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_volatge;
+	enum meson_sar_adc_vref_sel		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		}
 
 		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
-				    priv->param->vref_volatge);
+				    priv->param->vref_voltage);
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
@@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 	.cmv_select = 1,
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

The field should be called "vref_voltage", without a typo in the word
voltage. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 13b473d8c6c7..2615d74534df 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -327,7 +327,7 @@ struct meson_sar_adc_param {
 	u8					vref_select;
 	u8					cmv_select;
 	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_volatge;
+	enum meson_sar_adc_vref_sel		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		}
 
 		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
-				    priv->param->vref_volatge);
+				    priv->param->vref_voltage);
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
@@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.cmv_select = 1,
 };
 
@@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_volatge = 1,
+	.vref_voltage = 1,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 	.cmv_select = 1,
-- 
2.44.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
  2024-03-23 23:13 ` Martin Blumenstingl
  (?)
@ 2024-03-23 23:13   ` Martin Blumenstingl
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Consistently use bool for any register bit that enables/disables
functionality and enum for register values where there's a choice
between different settings. The aim is to make the code easier to read
and understand by being more consistent. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 2615d74534df..6b2af0c2bbc7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -156,9 +156,9 @@
 #define MESON_SAR_ADC_REG11					0x2c
 	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
 	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
-	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
-	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
-	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
+	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
+	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
+	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
 
 #define MESON_SAR_ADC_REG13					0x34
 	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
@@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
 	VREF_VDDA = 1,
 };
 
+enum meson_sar_adc_vref_voltage {
+	VREF_VOLTAGE_0V9 = 0,
+	VREF_VOLTAGE_1V8 = 1,
+};
+
 enum meson_sar_adc_avg_mode {
 	NO_AVERAGING = 0x0,
 	MEAN_AVERAGING = 0x1,
@@ -321,13 +326,13 @@ struct meson_sar_adc_param {
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
-	u8					disable_ring_counter;
+	bool					disable_ring_counter;
 	bool					has_reg11;
 	bool					has_vref_select;
-	u8					vref_select;
-	u8					cmv_select;
-	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_voltage;
+	bool					cmv_select;
+	bool					adc_eoc;
+	enum meson_sar_adc_vref_sel		vref_select;
+	enum meson_sar_adc_vref_voltage		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
 	}
 
-	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
-			    priv->param->disable_ring_counter);
+	if (priv->param->disable_ring_counter)
+		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
+	else
+		regval = 0;
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
 	if (priv->param->has_reg11) {
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
+		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
 
@@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
-				    priv->param->cmv_select);
+		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
 	}
@@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
@@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
@@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
-	.cmv_select = 1,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
@@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.adc_eoc = 1,
+	.vref_voltage = VREF_VOLTAGE_0V9,
+	.adc_eoc = true,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 };
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Consistently use bool for any register bit that enables/disables
functionality and enum for register values where there's a choice
between different settings. The aim is to make the code easier to read
and understand by being more consistent. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 2615d74534df..6b2af0c2bbc7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -156,9 +156,9 @@
 #define MESON_SAR_ADC_REG11					0x2c
 	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
 	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
-	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
-	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
-	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
+	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
+	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
+	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
 
 #define MESON_SAR_ADC_REG13					0x34
 	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
@@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
 	VREF_VDDA = 1,
 };
 
+enum meson_sar_adc_vref_voltage {
+	VREF_VOLTAGE_0V9 = 0,
+	VREF_VOLTAGE_1V8 = 1,
+};
+
 enum meson_sar_adc_avg_mode {
 	NO_AVERAGING = 0x0,
 	MEAN_AVERAGING = 0x1,
@@ -321,13 +326,13 @@ struct meson_sar_adc_param {
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
-	u8					disable_ring_counter;
+	bool					disable_ring_counter;
 	bool					has_reg11;
 	bool					has_vref_select;
-	u8					vref_select;
-	u8					cmv_select;
-	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_voltage;
+	bool					cmv_select;
+	bool					adc_eoc;
+	enum meson_sar_adc_vref_sel		vref_select;
+	enum meson_sar_adc_vref_voltage		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
 	}
 
-	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
-			    priv->param->disable_ring_counter);
+	if (priv->param->disable_ring_counter)
+		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
+	else
+		regval = 0;
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
 	if (priv->param->has_reg11) {
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
+		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
 
@@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
-				    priv->param->cmv_select);
+		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
 	}
@@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
@@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
@@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
-	.cmv_select = 1,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
@@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.adc_eoc = 1,
+	.vref_voltage = VREF_VOLTAGE_0V9,
+	.adc_eoc = true,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 };
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Consistently use bool for any register bit that enables/disables
functionality and enum for register values where there's a choice
between different settings. The aim is to make the code easier to read
and understand by being more consistent. No functional changes intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 2615d74534df..6b2af0c2bbc7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -156,9 +156,9 @@
 #define MESON_SAR_ADC_REG11					0x2c
 	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
 	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
-	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
-	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
-	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
+	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
+	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
+	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
 
 #define MESON_SAR_ADC_REG13					0x34
 	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
@@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
 	VREF_VDDA = 1,
 };
 
+enum meson_sar_adc_vref_voltage {
+	VREF_VOLTAGE_0V9 = 0,
+	VREF_VOLTAGE_1V8 = 1,
+};
+
 enum meson_sar_adc_avg_mode {
 	NO_AVERAGING = 0x0,
 	MEAN_AVERAGING = 0x1,
@@ -321,13 +326,13 @@ struct meson_sar_adc_param {
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
-	u8					disable_ring_counter;
+	bool					disable_ring_counter;
 	bool					has_reg11;
 	bool					has_vref_select;
-	u8					vref_select;
-	u8					cmv_select;
-	u8					adc_eoc;
-	enum meson_sar_adc_vref_sel		vref_voltage;
+	bool					cmv_select;
+	bool					adc_eoc;
+	enum meson_sar_adc_vref_sel		vref_select;
+	enum meson_sar_adc_vref_voltage		vref_voltage;
 };
 
 struct meson_sar_adc_data {
@@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
 	}
 
-	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
-			    priv->param->disable_ring_counter);
+	if (priv->param->disable_ring_counter)
+		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
+	else
+		regval = 0;
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
 	if (priv->param->has_reg11) {
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
+		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
 
@@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
 
-		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
-				    priv->param->cmv_select);
+		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
 	}
@@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
@@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
-	.cmv_select = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
@@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.vref_voltage = 1,
+	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
-	.cmv_select = 1,
+	.cmv_select = true,
 };
 
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
@@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.resolution = 12,
 	.disable_ring_counter = 1,
 	.has_reg11 = true,
-	.adc_eoc = 1,
+	.vref_voltage = VREF_VOLTAGE_0V9,
+	.adc_eoc = true,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
 };
-- 
2.44.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 3/3] iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
  2024-03-23 23:13 ` Martin Blumenstingl
  (?)
@ 2024-03-23 23:13   ` Martin Blumenstingl
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Simply check the max_register value to decide whether
MESON_SAR_ADC_REG11 is present on the current IP revision. This allows
dropping two additional bool fields from struct meson_sar_adc_param
which previously had to be manually kept in sync. No functional changes
intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 6b2af0c2bbc7..8c1e542c0ab7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -320,14 +320,12 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
 struct meson_sar_adc_param {
 	bool					has_bl30_integration;
 	unsigned long				clock_rate;
-	u32					bandgap_reg;
 	unsigned int				resolution;
 	const struct regmap_config		*regmap_config;
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
 	bool					disable_ring_counter;
-	bool					has_reg11;
 	bool					has_vref_select;
 	bool					cmv_select;
 	bool					adc_eoc;
@@ -995,7 +993,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
-	if (priv->param->has_reg11) {
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11) {
 		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
@@ -1031,16 +1029,15 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	const struct meson_sar_adc_param *param = priv->param;
-	u32 enable_mask;
 
-	if (param->bandgap_reg == MESON_SAR_ADC_REG11)
-		enable_mask = MESON_SAR_ADC_REG11_BANDGAP_EN;
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11)
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
+				   MESON_SAR_ADC_REG11_BANDGAP_EN,
+				   on_off ? MESON_SAR_ADC_REG11_BANDGAP_EN : 0);
 	else
-		enable_mask = MESON_SAR_ADC_DELTA_10_TS_VBG_EN;
-
-	regmap_update_bits(priv->regmap, param->bandgap_reg, enable_mask,
-			   on_off ? enable_mask : 0);
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELTA_10,
+				   MESON_SAR_ADC_DELTA_10_TS_VBG_EN,
+				   on_off ? MESON_SAR_ADC_DELTA_10_TS_VBG_EN : 0);
 }
 
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
@@ -1205,7 +1202,6 @@ static const struct iio_info meson_sar_adc_iio_info = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 4,
@@ -1216,7 +1212,6 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 5,
@@ -1227,10 +1222,8 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1238,11 +1231,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1250,11 +1241,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
@@ -1264,11 +1253,9 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_0V9,
 	.adc_eoc = true,
 	.has_vref_select = true,
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 3/3] iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Simply check the max_register value to decide whether
MESON_SAR_ADC_REG11 is present on the current IP revision. This allows
dropping two additional bool fields from struct meson_sar_adc_param
which previously had to be manually kept in sync. No functional changes
intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 6b2af0c2bbc7..8c1e542c0ab7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -320,14 +320,12 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
 struct meson_sar_adc_param {
 	bool					has_bl30_integration;
 	unsigned long				clock_rate;
-	u32					bandgap_reg;
 	unsigned int				resolution;
 	const struct regmap_config		*regmap_config;
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
 	bool					disable_ring_counter;
-	bool					has_reg11;
 	bool					has_vref_select;
 	bool					cmv_select;
 	bool					adc_eoc;
@@ -995,7 +993,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
-	if (priv->param->has_reg11) {
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11) {
 		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
@@ -1031,16 +1029,15 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	const struct meson_sar_adc_param *param = priv->param;
-	u32 enable_mask;
 
-	if (param->bandgap_reg == MESON_SAR_ADC_REG11)
-		enable_mask = MESON_SAR_ADC_REG11_BANDGAP_EN;
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11)
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
+				   MESON_SAR_ADC_REG11_BANDGAP_EN,
+				   on_off ? MESON_SAR_ADC_REG11_BANDGAP_EN : 0);
 	else
-		enable_mask = MESON_SAR_ADC_DELTA_10_TS_VBG_EN;
-
-	regmap_update_bits(priv->regmap, param->bandgap_reg, enable_mask,
-			   on_off ? enable_mask : 0);
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELTA_10,
+				   MESON_SAR_ADC_DELTA_10_TS_VBG_EN,
+				   on_off ? MESON_SAR_ADC_DELTA_10_TS_VBG_EN : 0);
 }
 
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
@@ -1205,7 +1202,6 @@ static const struct iio_info meson_sar_adc_iio_info = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 4,
@@ -1216,7 +1212,6 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 5,
@@ -1227,10 +1222,8 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1238,11 +1231,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1250,11 +1241,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
@@ -1264,11 +1253,9 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_0V9,
 	.adc_eoc = true,
 	.has_vref_select = true,
-- 
2.44.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* [PATCH v1 3/3] iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
@ 2024-03-23 23:13   ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-23 23:13 UTC (permalink / raw)
  To: linux-amlogic
  Cc: linux-arm-kernel, linux-kernel, gnstark, neil.armstrong, lars,
	jic23, Martin Blumenstingl

Simply check the max_register value to decide whether
MESON_SAR_ADC_REG11 is present on the current IP revision. This allows
dropping two additional bool fields from struct meson_sar_adc_param
which previously had to be manually kept in sync. No functional changes
intended.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/iio/adc/meson_saradc.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 6b2af0c2bbc7..8c1e542c0ab7 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -320,14 +320,12 @@ static const struct iio_chan_spec meson_sar_adc_and_temp_iio_channels[] = {
 struct meson_sar_adc_param {
 	bool					has_bl30_integration;
 	unsigned long				clock_rate;
-	u32					bandgap_reg;
 	unsigned int				resolution;
 	const struct regmap_config		*regmap_config;
 	u8					temperature_trimming_bits;
 	unsigned int				temperature_multiplier;
 	unsigned int				temperature_divider;
 	bool					disable_ring_counter;
-	bool					has_reg11;
 	bool					has_vref_select;
 	bool					cmv_select;
 	bool					adc_eoc;
@@ -995,7 +993,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
 			   regval);
 
-	if (priv->param->has_reg11) {
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11) {
 		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
 		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 				   MESON_SAR_ADC_REG11_EOC, regval);
@@ -1031,16 +1029,15 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	const struct meson_sar_adc_param *param = priv->param;
-	u32 enable_mask;
 
-	if (param->bandgap_reg == MESON_SAR_ADC_REG11)
-		enable_mask = MESON_SAR_ADC_REG11_BANDGAP_EN;
+	if (priv->param->regmap_config->max_register >= MESON_SAR_ADC_REG11)
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
+				   MESON_SAR_ADC_REG11_BANDGAP_EN,
+				   on_off ? MESON_SAR_ADC_REG11_BANDGAP_EN : 0);
 	else
-		enable_mask = MESON_SAR_ADC_DELTA_10_TS_VBG_EN;
-
-	regmap_update_bits(priv->regmap, param->bandgap_reg, enable_mask,
-			   on_off ? enable_mask : 0);
+		regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELTA_10,
+				   MESON_SAR_ADC_DELTA_10_TS_VBG_EN,
+				   on_off ? MESON_SAR_ADC_DELTA_10_TS_VBG_EN : 0);
 }
 
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
@@ -1205,7 +1202,6 @@ static const struct iio_info meson_sar_adc_iio_info = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 4,
@@ -1216,7 +1212,6 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8_param = {
 static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1150000,
-	.bandgap_reg = MESON_SAR_ADC_DELTA_10,
 	.regmap_config = &meson_sar_adc_regmap_config_meson8,
 	.resolution = 10,
 	.temperature_trimming_bits = 5,
@@ -1227,10 +1222,8 @@ static const struct meson_sar_adc_param meson_sar_adc_meson8b_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 10,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1238,11 +1231,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
 static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.cmv_select = true,
 };
@@ -1250,11 +1241,9 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
 static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 	.has_bl30_integration = true,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_1V8,
 	.has_vref_select = true,
 	.vref_select = VREF_VDDA,
@@ -1264,11 +1253,9 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
 static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
 	.has_bl30_integration = false,
 	.clock_rate = 1200000,
-	.bandgap_reg = MESON_SAR_ADC_REG11,
 	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
 	.resolution = 12,
 	.disable_ring_counter = 1,
-	.has_reg11 = true,
 	.vref_voltage = VREF_VOLTAGE_0V9,
 	.adc_eoc = true,
 	.has_vref_select = true,
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 0/3] iio: adc: meson: a few improvements
  2024-03-23 23:13 ` Martin Blumenstingl
  (?)
@ 2024-03-24 14:04   ` Jonathan Cameron
  -1 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:04 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:06 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> This series contains three improvements to the meson SAR ADC driver.
> None of them are meant to change the existing behavior. The goal is
> to make the driver code easier to read and understand.
> 
> 
> Martin Blumenstingl (3):
>   iio: adc: meson: fix voltage reference selection field name typo
>   iio: adc: meson: consistently use bool/enum in struct
>     meson_sar_adc_param
>   iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
> 
>  drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 42 deletions(-)
> 

Hi Martin,

Please resend +CC linux-iio@vger.kernel.org

I'll take a quick look but I won't pick up anything that hasn't been on that
list (as I use patchwork to track status etc).

Jonathan

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 0/3] iio: adc: meson: a few improvements
@ 2024-03-24 14:04   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:04 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:06 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> This series contains three improvements to the meson SAR ADC driver.
> None of them are meant to change the existing behavior. The goal is
> to make the driver code easier to read and understand.
> 
> 
> Martin Blumenstingl (3):
>   iio: adc: meson: fix voltage reference selection field name typo
>   iio: adc: meson: consistently use bool/enum in struct
>     meson_sar_adc_param
>   iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
> 
>  drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 42 deletions(-)
> 

Hi Martin,

Please resend +CC linux-iio@vger.kernel.org

I'll take a quick look but I won't pick up anything that hasn't been on that
list (as I use patchwork to track status etc).

Jonathan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 0/3] iio: adc: meson: a few improvements
@ 2024-03-24 14:04   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:04 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:06 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> This series contains three improvements to the meson SAR ADC driver.
> None of them are meant to change the existing behavior. The goal is
> to make the driver code easier to read and understand.
> 
> 
> Martin Blumenstingl (3):
>   iio: adc: meson: fix voltage reference selection field name typo
>   iio: adc: meson: consistently use bool/enum in struct
>     meson_sar_adc_param
>   iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access
> 
>  drivers/iio/adc/meson_saradc.c | 78 ++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 42 deletions(-)
> 

Hi Martin,

Please resend +CC linux-iio@vger.kernel.org

I'll take a quick look but I won't pick up anything that hasn't been on that
list (as I use patchwork to track status etc).

Jonathan

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
  2024-03-23 23:13   ` Martin Blumenstingl
  (?)
@ 2024-03-24 14:07     ` Jonathan Cameron
  -1 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:07 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:08 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>  #define MESON_SAR_ADC_REG11					0x2c
>  	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>  	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)

Looks like a spaces to tab conversion.  Not related to rest of patch so
put them back as spaces.  If you really want to tidy that up with tabs
later, separate patch please.

Otherwise LGTM

>  
>  #define MESON_SAR_ADC_REG13					0x34
>  	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>  	VREF_VDDA = 1,
>  };
>  
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>  enum meson_sar_adc_avg_mode {
>  	NO_AVERAGING = 0x0,
>  	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>  	u8					temperature_trimming_bits;
>  	unsigned int				temperature_multiplier;
>  	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>  	bool					has_reg11;
>  	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>  };
>  
>  struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>  	}
>  
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>  	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>  			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>  			   regval);
>  
>  	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_EOC, regval);
>  
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>  
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>  	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>  	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>  	.resolution = 10,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
>  };


^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-24 14:07     ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:07 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:08 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>  #define MESON_SAR_ADC_REG11					0x2c
>  	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>  	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)

Looks like a spaces to tab conversion.  Not related to rest of patch so
put them back as spaces.  If you really want to tidy that up with tabs
later, separate patch please.

Otherwise LGTM

>  
>  #define MESON_SAR_ADC_REG13					0x34
>  	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>  	VREF_VDDA = 1,
>  };
>  
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>  enum meson_sar_adc_avg_mode {
>  	NO_AVERAGING = 0x0,
>  	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>  	u8					temperature_trimming_bits;
>  	unsigned int				temperature_multiplier;
>  	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>  	bool					has_reg11;
>  	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>  };
>  
>  struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>  	}
>  
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>  	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>  			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>  			   regval);
>  
>  	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_EOC, regval);
>  
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>  
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>  	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>  	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>  	.resolution = 10,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-24 14:07     ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2024-03-24 14:07 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-arm-kernel, linux-kernel, gnstark,
	neil.armstrong, lars

On Sun, 24 Mar 2024 00:13:08 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>  #define MESON_SAR_ADC_REG11					0x2c
>  	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>  	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)

Looks like a spaces to tab conversion.  Not related to rest of patch so
put them back as spaces.  If you really want to tidy that up with tabs
later, separate patch please.

Otherwise LGTM

>  
>  #define MESON_SAR_ADC_REG13					0x34
>  	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>  	VREF_VDDA = 1,
>  };
>  
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>  enum meson_sar_adc_avg_mode {
>  	NO_AVERAGING = 0x0,
>  	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>  	u8					temperature_trimming_bits;
>  	unsigned int				temperature_multiplier;
>  	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>  	bool					has_reg11;
>  	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>  };
>  
>  struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>  	}
>  
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>  	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>  			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>  			   regval);
>  
>  	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_EOC, regval);
>  
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>  
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>  		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>  				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>  	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>  	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>  	.resolution = 10,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>  };
>  
>  static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>  	.resolution = 12,
>  	.disable_ring_counter = 1,
>  	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>  	.has_vref_select = true,
>  	.vref_select = VREF_VDDA,
>  };


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
  2024-03-23 23:13   ` Martin Blumenstingl
  (?)
@ 2024-03-25 23:45     ` George Stark
  -1 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-25 23:45 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

Thanks for the patch

Should the tag
Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
be added?

On 3/24/24 02:13, Martin Blumenstingl wrote:
> The field should be called "vref_voltage", without a typo in the word
> voltage. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 13b473d8c6c7..2615d74534df 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -327,7 +327,7 @@ struct meson_sar_adc_param {
>   	u8					vref_select;
>   	u8					cmv_select;
>   	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_volatge;
> +	enum meson_sar_adc_vref_sel		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		}
>   
>   		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
> -				    priv->param->vref_volatge);
> +				    priv->param->vref_voltage);
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> @@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   	.cmv_select = 1,

-- 
Best regards
George

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-25 23:45     ` George Stark
  0 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-25 23:45 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

Thanks for the patch

Should the tag
Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
be added?

On 3/24/24 02:13, Martin Blumenstingl wrote:
> The field should be called "vref_voltage", without a typo in the word
> voltage. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 13b473d8c6c7..2615d74534df 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -327,7 +327,7 @@ struct meson_sar_adc_param {
>   	u8					vref_select;
>   	u8					cmv_select;
>   	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_volatge;
> +	enum meson_sar_adc_vref_sel		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		}
>   
>   		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
> -				    priv->param->vref_volatge);
> +				    priv->param->vref_voltage);
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> @@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   	.cmv_select = 1,

-- 
Best regards
George

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-25 23:45     ` George Stark
  0 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-25 23:45 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

Thanks for the patch

Should the tag
Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
be added?

On 3/24/24 02:13, Martin Blumenstingl wrote:
> The field should be called "vref_voltage", without a typo in the word
> voltage. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 13b473d8c6c7..2615d74534df 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -327,7 +327,7 @@ struct meson_sar_adc_param {
>   	u8					vref_select;
>   	u8					cmv_select;
>   	u8					adc_eoc;
> -	enum meson_sar_adc_vref_sel		vref_volatge;
> +	enum meson_sar_adc_vref_sel		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		}
>   
>   		regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
> -				    priv->param->vref_volatge);
> +				    priv->param->vref_voltage);
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> @@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.cmv_select = 1,
>   };
>   
> @@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_volatge = 1,
> +	.vref_voltage = 1,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   	.cmv_select = 1,

-- 
Best regards
George

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
  2024-03-23 23:13   ` Martin Blumenstingl
  (?)
@ 2024-03-26  0:41     ` George Stark
  -1 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-26  0:41 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

On 3/24/24 02:13, Martin Blumenstingl wrote:
> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>   1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>   #define MESON_SAR_ADC_REG11					0x2c
>   	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>   	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
>   
>   #define MESON_SAR_ADC_REG13					0x34
>   	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>   	VREF_VDDA = 1,
>   };
>   
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>   enum meson_sar_adc_avg_mode {
>   	NO_AVERAGING = 0x0,
>   	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>   	u8					temperature_trimming_bits;
>   	unsigned int				temperature_multiplier;
>   	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>   	bool					has_reg11;
>   	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;


The reason to choose u8 type over bool was that those are not actually
bool values but direct values of hw register bits. We have little 
information about real meaning of these bits so it won't help much to
add bool layer and keep real values in the init code instead of param 
section (adc_eoc, cmv_select).
bool disable_ring_counter will look deceptive too because it doesn't
say whether disable ring_counter or not (we always disable it) but
how to disable it (write 0 or 1)

I think the poor choice was not the type of variables but their names:
u8 adc_eoc_bit;
u8 cmv_select_bit;
u8 disable_ring_counter_bit;

would be clearer.


> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>   	}
>   
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>   	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>   			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>   			   regval);
>   
>   	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_EOC, regval);
>   
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>   	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   };

-- 
Best regards
George

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-26  0:41     ` George Stark
  0 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-26  0:41 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

On 3/24/24 02:13, Martin Blumenstingl wrote:
> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>   1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>   #define MESON_SAR_ADC_REG11					0x2c
>   	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>   	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
>   
>   #define MESON_SAR_ADC_REG13					0x34
>   	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>   	VREF_VDDA = 1,
>   };
>   
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>   enum meson_sar_adc_avg_mode {
>   	NO_AVERAGING = 0x0,
>   	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>   	u8					temperature_trimming_bits;
>   	unsigned int				temperature_multiplier;
>   	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>   	bool					has_reg11;
>   	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;


The reason to choose u8 type over bool was that those are not actually
bool values but direct values of hw register bits. We have little 
information about real meaning of these bits so it won't help much to
add bool layer and keep real values in the init code instead of param 
section (adc_eoc, cmv_select).
bool disable_ring_counter will look deceptive too because it doesn't
say whether disable ring_counter or not (we always disable it) but
how to disable it (write 0 or 1)

I think the poor choice was not the type of variables but their names:
u8 adc_eoc_bit;
u8 cmv_select_bit;
u8 disable_ring_counter_bit;

would be clearer.


> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>   	}
>   
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>   	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>   			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>   			   regval);
>   
>   	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_EOC, regval);
>   
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>   	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   };

-- 
Best regards
George

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
@ 2024-03-26  0:41     ` George Stark
  0 siblings, 0 replies; 27+ messages in thread
From: George Stark @ 2024-03-26  0:41 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hello Martin

On 3/24/24 02:13, Martin Blumenstingl wrote:
> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
>   1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
>   #define MESON_SAR_ADC_REG11					0x2c
>   	#define MESON_SAR_ADC_REG11_BANDGAP_EN			BIT(13)
>   	#define MESON_SAR_ADC_REG11_CMV_SEL                     BIT(6)
> -	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE                BIT(5)
> -	#define MESON_SAR_ADC_REG11_EOC                         BIT(1)
> -	#define MESON_SAR_ADC_REG11_VREF_SEL                    BIT(0)
> +	#define MESON_SAR_ADC_REG11_VREF_VOLTAGE		BIT(5)
> +	#define MESON_SAR_ADC_REG11_EOC				BIT(1)
> +	#define MESON_SAR_ADC_REG11_VREF_SEL			BIT(0)
>   
>   #define MESON_SAR_ADC_REG13					0x34
>   	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
>   	VREF_VDDA = 1,
>   };
>   
> +enum meson_sar_adc_vref_voltage {
> +	VREF_VOLTAGE_0V9 = 0,
> +	VREF_VOLTAGE_1V8 = 1,
> +};
> +
>   enum meson_sar_adc_avg_mode {
>   	NO_AVERAGING = 0x0,
>   	MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
>   	u8					temperature_trimming_bits;
>   	unsigned int				temperature_multiplier;
>   	unsigned int				temperature_divider;
> -	u8					disable_ring_counter;
> +	bool					disable_ring_counter;
>   	bool					has_reg11;
>   	bool					has_vref_select;
> -	u8					vref_select;
> -	u8					cmv_select;
> -	u8					adc_eoc;


The reason to choose u8 type over bool was that those are not actually
bool values but direct values of hw register bits. We have little 
information about real meaning of these bits so it won't help much to
add bool layer and keep real values in the init code instead of param 
section (adc_eoc, cmv_select).
bool disable_ring_counter will look deceptive too because it doesn't
say whether disable ring_counter or not (we always disable it) but
how to disable it (write 0 or 1)

I think the poor choice was not the type of variables but their names:
u8 adc_eoc_bit;
u8 cmv_select_bit;
u8 disable_ring_counter_bit;

would be clearer.


> -	enum meson_sar_adc_vref_sel		vref_voltage;
> +	bool					cmv_select;
> +	bool					adc_eoc;
> +	enum meson_sar_adc_vref_sel		vref_select;
> +	enum meson_sar_adc_vref_voltage		vref_voltage;
>   };
>   
>   struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   				   MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
>   	}
>   
> -	regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> -			    priv->param->disable_ring_counter);
> +	if (priv->param->disable_ring_counter)
> +		regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> +	else
> +		regval = 0;
>   	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
>   			   MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
>   			   regval);
>   
>   	if (priv->param->has_reg11) {
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> +		regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_EOC, regval);
>   
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>   
> -		regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> -				    priv->param->cmv_select);
> +		regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
>   		regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
>   				   MESON_SAR_ADC_REG11_CMV_SEL, regval);
>   	}
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
>   	.regmap_config = &meson_sar_adc_regmap_config_gxbb,
>   	.resolution = 10,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> -	.cmv_select = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.vref_voltage = 1,
> +	.vref_voltage = VREF_VOLTAGE_1V8,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
> -	.cmv_select = 1,
> +	.cmv_select = true,
>   };
>   
>   static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
>   	.resolution = 12,
>   	.disable_ring_counter = 1,
>   	.has_reg11 = true,
> -	.adc_eoc = 1,
> +	.vref_voltage = VREF_VOLTAGE_0V9,
> +	.adc_eoc = true,
>   	.has_vref_select = true,
>   	.vref_select = VREF_VDDA,
>   };

-- 
Best regards
George

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
  2024-03-25 23:45     ` George Stark
  (?)
@ 2024-03-31 21:30       ` Martin Blumenstingl
  -1 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-31 21:30 UTC (permalink / raw)
  To: George Stark
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hi George,

On Tue, Mar 26, 2024 at 12:45 AM George Stark <gnstark@salutedevices.com> wrote:
>
> Hello Martin
>
> Thanks for the patch
you're welcome

> Should the tag
> Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
> be added?
My understanding is that "Fixes" is for actual bugfixes. There's no
harm done by a type that's only visible in the driver source (it's not
exposed to DT, userspace or elsewhere).


Best regards,
Martin

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-31 21:30       ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-31 21:30 UTC (permalink / raw)
  To: George Stark
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hi George,

On Tue, Mar 26, 2024 at 12:45 AM George Stark <gnstark@salutedevices.com> wrote:
>
> Hello Martin
>
> Thanks for the patch
you're welcome

> Should the tag
> Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
> be added?
My understanding is that "Fixes" is for actual bugfixes. There's no
harm done by a type that's only visible in the driver source (it's not
exposed to DT, userspace or elsewhere).


Best regards,
Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
@ 2024-03-31 21:30       ` Martin Blumenstingl
  0 siblings, 0 replies; 27+ messages in thread
From: Martin Blumenstingl @ 2024-03-31 21:30 UTC (permalink / raw)
  To: George Stark
  Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
	linux-amlogic, kernel

Hi George,

On Tue, Mar 26, 2024 at 12:45 AM George Stark <gnstark@salutedevices.com> wrote:
>
> Hello Martin
>
> Thanks for the patch
you're welcome

> Should the tag
> Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
> be added?
My understanding is that "Fixes" is for actual bugfixes. There's no
harm done by a type that's only visible in the driver source (it's not
exposed to DT, userspace or elsewhere).


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 27+ messages in thread
end of thread, other threads:[~2024-03-31 21:31 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-23 23:13 [PATCH v1 0/3] iio: adc: meson: a few improvements Martin Blumenstingl
2024-03-23 23:13 ` Martin Blumenstingl
2024-03-23 23:13 ` Martin Blumenstingl
2024-03-23 23:13 ` [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-25 23:45   ` George Stark
2024-03-25 23:45     ` George Stark
2024-03-25 23:45     ` George Stark
2024-03-31 21:30     ` Martin Blumenstingl
2024-03-31 21:30       ` Martin Blumenstingl
2024-03-31 21:30       ` Martin Blumenstingl
2024-03-23 23:13 ` [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-24 14:07   ` Jonathan Cameron
2024-03-24 14:07     ` Jonathan Cameron
2024-03-24 14:07     ` Jonathan Cameron
2024-03-26  0:41   ` George Stark
2024-03-26  0:41     ` George Stark
2024-03-26  0:41     ` George Stark
2024-03-23 23:13 ` [PATCH v1 3/3] iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-23 23:13   ` Martin Blumenstingl
2024-03-24 14:04 ` [PATCH v1 0/3] iio: adc: meson: a few improvements Jonathan Cameron
2024-03-24 14:04   ` Jonathan Cameron
2024-03-24 14:04   ` 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.