[PATCH v3 0/8] Add settings for precharge and internal resistor

[PATCH v3 0/8] Add settings for precharge and internal resistor

[Gwendal Grignou]

For Semtech sensors SX9324 and SX9360, allow confugration of the
pre-charge resistor (9324/9360) and internal resistor (9324).

Fix register name spelling mistakes first and set default value properly
for sx9324 internal resistor register.

The 9360 changes are independent from the 9324 changes, but they are
very similar.

Changes since v2:
- use -ohms instead of -kohms, as ohms is a defined unit in
  dt-schema.git dtschema/schemas/property-units.yaml

Changes sinve v1:
- Add kOhm Unit, add a new patch to fix invalid register setting.

Gwendal Grignou (8):
  iio: sx9324: Fix default precharge internal resistance register
  iio: sx9324: Fix register field spelling
  dt-bindings: iio: sx9324: Add precharge resistor setting
  iio: sx9324: Add precharge internal resistance setting
  dt-bindings: iio: sx9324: Add internal compensation resistor setting
  iio: sx9324: Add Setting for internal compensation resistor
  dt-bindings: iio: sx9360: Add precharge resistor setting
  iio: sx9360: Add pre-charge resistor setting

 .../iio/proximity/semtech,sx9324.yaml         | 19 ++++++++
 .../iio/proximity/semtech,sx9360.yaml         |  9 ++++
 drivers/iio/proximity/sx9324.c                | 44 ++++++++++++++++---
 drivers/iio/proximity/sx9360.c                | 12 ++++-
 4 files changed, 78 insertions(+), 6 deletions(-)

-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 1/8] iio: sx9324: Fix default precharge internal resistance register

[Gwendal Grignou]

Fix the default value for the register that set the resistance:
it has to be 0x10.

Fixes: 4c18a890dff8d ("iio:proximity:sx9324: Add SX9324 support")
Cc: stable@vger.kernel.org
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- no changes

Changes since v1:
- new patch.

 drivers/iio/proximity/sx9324.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index 0d9bbbb50cb45..d50ce67aafdf2 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -70,7 +70,8 @@
 #define SX9324_REG_AFE_PH2		0x2a
 #define SX9324_REG_AFE_PH3		0x2b
 #define SX9324_REG_AFE_CTRL8		0x2c
-#define SX9324_REG_AFE_CTRL8_RESFILTN_4KOHM 0x02
+#define SX9324_REG_AFE_CTRL8_RSVD	0x10
+#define SX9324_REG_AFE_CTRL8_RESFILTIN_4KOHM 0x02
 #define SX9324_REG_AFE_CTRL9		0x2d
 #define SX9324_REG_AFE_CTRL9_AGAIN_1	0x08
 
@@ -781,7 +782,8 @@ static const struct sx_common_reg_default sx9324_default_regs[] = {
 	{ SX9324_REG_AFE_PH2, 0x1a },
 	{ SX9324_REG_AFE_PH3, 0x16 },
 
-	{ SX9324_REG_AFE_CTRL8, SX9324_REG_AFE_CTRL8_RESFILTN_4KOHM },
+	{ SX9324_REG_AFE_CTRL8, SX9324_REG_AFE_CTRL8_RSVD |
+		SX9324_REG_AFE_CTRL8_RESFILTIN_4KOHM },
 	{ SX9324_REG_AFE_CTRL9, SX9324_REG_AFE_CTRL9_AGAIN_1 },
 
 	{ SX9324_REG_PROX_CTRL0, SX9324_REG_PROX_CTRL0_GAIN_1 |
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 2/8] iio: sx9324: Fix register field spelling

[Gwendal Grignou]

Field for PROX_CTRL4 should contain PROX_CTRL4.

Fixes: 4c18a890dff8d ("iio:proximity:sx9324: Add SX9324 support")
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- no changes

Changes since v1:
- Add Fixes keyword in commit message.

 drivers/iio/proximity/sx9324.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index d50ce67aafdf2..1dfa19426c210 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -90,7 +90,7 @@
 #define SX9324_REG_PROX_CTRL4_AVGNEGFILT_MASK	GENMASK(5, 3)
 #define SX9324_REG_PROX_CTRL4_AVGNEG_FILT_2 0x08
 #define SX9324_REG_PROX_CTRL4_AVGPOSFILT_MASK	GENMASK(2, 0)
-#define SX9324_REG_PROX_CTRL3_AVGPOS_FILT_256 0x04
+#define SX9324_REG_PROX_CTRL4_AVGPOS_FILT_256 0x04
 #define SX9324_REG_PROX_CTRL5		0x35
 #define SX9324_REG_PROX_CTRL5_HYST_MASK			GENMASK(5, 4)
 #define SX9324_REG_PROX_CTRL5_CLOSE_DEBOUNCE_MASK	GENMASK(3, 2)
@@ -794,7 +794,7 @@ static const struct sx_common_reg_default sx9324_default_regs[] = {
 	{ SX9324_REG_PROX_CTRL3, SX9324_REG_PROX_CTRL3_AVGDEB_2SAMPLES |
 		SX9324_REG_PROX_CTRL3_AVGPOS_THRESH_16K },
 	{ SX9324_REG_PROX_CTRL4, SX9324_REG_PROX_CTRL4_AVGNEG_FILT_2 |
-		SX9324_REG_PROX_CTRL3_AVGPOS_FILT_256 },
+		SX9324_REG_PROX_CTRL4_AVGPOS_FILT_256 },
 	{ SX9324_REG_PROX_CTRL5, 0x00 },
 	{ SX9324_REG_PROX_CTRL6, SX9324_REG_PROX_CTRL6_PROXTHRESH_32 },
 	{ SX9324_REG_PROX_CTRL7, SX9324_REG_PROX_CTRL6_PROXTHRESH_32 },
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 3/8] dt-bindings: iio: sx9324: Add precharge resistor setting

[Gwendal Grignou]

Allow configure the resistance used during precharge.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Change kOhms into ohms.

Changes since v1:
- Suffix field with kOhms unit.

 .../bindings/iio/proximity/semtech,sx9324.yaml           | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
index b8a6ee16854ff..a22cad1507b6b 100644
--- a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
+++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
@@ -126,6 +126,14 @@ properties:
       UINT_MAX (4294967295) represents infinite. Other values
       represent 1-1/N.
 
+  semtech,input-precharge-resistor-ohms:
+    default: 4000
+    description:
+      Pre-charge input resistance in Ohm.
+      Rounded down to a 2000 Ohm multiple.
+    minimum: 0
+    maximum: 30000
+
 required:
   - compatible
   - reg
@@ -157,5 +165,6 @@ examples:
         semtech,ph01-proxraw-strength = <2>;
         semtech,ph23-proxraw-strength = <2>;
         semtech,avg-pos-strength = <64>;
+        semtech,input-precharge-resistor-ohms = <2000>;
       };
     };
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 4/8] iio: sx9324: Add precharge internal resistance setting

[Gwendal Grignou]

Add ability to set the precharge internal resistance from the device
tree.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Change kOhms into ohms.

Changes since v1:
- Suffix field with kOhms unit.
- Split patch in 2.

 drivers/iio/proximity/sx9324.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index 1dfa19426c210..3f229dffd5380 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -72,6 +72,7 @@
 #define SX9324_REG_AFE_CTRL8		0x2c
 #define SX9324_REG_AFE_CTRL8_RSVD	0x10
 #define SX9324_REG_AFE_CTRL8_RESFILTIN_4KOHM 0x02
+#define SX9324_REG_AFE_CTRL8_RESFILTIN_MASK GENMASK(3, 0)
 #define SX9324_REG_AFE_CTRL9		0x2d
 #define SX9324_REG_AFE_CTRL9_AGAIN_1	0x08
 
@@ -893,6 +894,15 @@ sx9324_get_default_reg(struct device *dev, int idx,
 		reg_def->def |= FIELD_PREP(SX9324_REG_AFE_CTRL4_RESOLUTION_MASK,
 					   raw);
 		break;
+	case SX9324_REG_AFE_CTRL8:
+		ret = device_property_read_u32(dev,
+				"semtech,input-precharge-resistor-ohms",
+				&raw);
+		reg_def->def &= ~SX9324_REG_AFE_CTRL8_RESFILTIN_MASK;
+		reg_def->def |= FIELD_PREP(SX9324_REG_AFE_CTRL8_RESFILTIN_MASK,
+					   raw / 2000);
+		break;
+
 	case SX9324_REG_ADV_CTRL5:
 		ret = device_property_read_u32(dev, "semtech,startup-sensor",
 					       &start);
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 5/8] dt-bindings: iio: sx9324: Add internal compensation resistor setting

[Gwendal Grignou]

Allow setting the internal resistor used for compensation.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- no changes

Changes since v1:
- no changes

 .../bindings/iio/proximity/semtech,sx9324.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
index a22cad1507b6b..f99b2c1cd3ac2 100644
--- a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
+++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9324.yaml
@@ -126,6 +126,15 @@ properties:
       UINT_MAX (4294967295) represents infinite. Other values
       represent 1-1/N.
 
+  semtech,int-comp-resistor:
+    description:
+      Internal resistor setting for compensation.
+    enum:
+      - lowest
+      - low
+      - high
+      - highest
+
   semtech,input-precharge-resistor-ohms:
     default: 4000
     description:
@@ -165,6 +174,7 @@ examples:
         semtech,ph01-proxraw-strength = <2>;
         semtech,ph23-proxraw-strength = <2>;
         semtech,avg-pos-strength = <64>;
+        semtech,int-comp-resistor = "lowest";
         semtech,input-precharge-resistor-ohms = <2000>;
       };
     };
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 6/8] iio: sx9324: Add Setting for internal compensation resistor

[Gwendal Grignou]

Based on device tree setting, set the internal compensation resistor.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- No changes.

Changes since v1:
- No changes.

 drivers/iio/proximity/sx9324.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index 3f229dffd5380..38dfdc6dc86ad 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -52,6 +52,11 @@
 #define SX9324_REG_CLK_SPRD		0x15
 
 #define SX9324_REG_AFE_CTRL0		0x20
+#define SX9324_REG_AFE_CTRL0_RINT_MASK GENMASK(7, 6)
+#define SX9324_REG_AFE_CTRL0_RINT_LOWEST	0x00
+#define SX9324_REG_AFE_CTRL0_RINT_LOW		0x40
+#define SX9324_REG_AFE_CTRL0_RINT_HIGH		0x80
+#define SX9324_REG_AFE_CTRL0_RINT_HIGHEST	0xc0
 #define SX9324_REG_AFE_CTRL1		0x21
 #define SX9324_REG_AFE_CTRL2		0x22
 #define SX9324_REG_AFE_CTRL3		0x23
@@ -769,7 +774,7 @@ static const struct sx_common_reg_default sx9324_default_regs[] = {
 	 */
 	{ SX9324_REG_GNRL_CTRL1, SX9324_REG_GNRL_CTRL1_PAUSECTRL },
 
-	{ SX9324_REG_AFE_CTRL0, 0x00 },
+	{ SX9324_REG_AFE_CTRL0, SX9324_REG_AFE_CTRL0_RINT_LOWEST },
 	{ SX9324_REG_AFE_CTRL3, 0x00 },
 	{ SX9324_REG_AFE_CTRL4, SX9324_REG_AFE_CTRL4_FREQ_83_33HZ |
 		SX9324_REG_AFE_CTRL4_RES_100 },
@@ -855,6 +860,7 @@ sx9324_get_default_reg(struct device *dev, int idx,
 	char prop[] = SX9324_PROXRAW_DEF;
 	u32 start = 0, raw = 0, pos = 0;
 	int ret, count, ph, pin;
+	const char *res;
 
 	memcpy(reg_def, &sx9324_default_regs[idx], sizeof(*reg_def));
 	switch (reg_def->reg) {
@@ -875,6 +881,22 @@ sx9324_get_default_reg(struct device *dev, int idx,
 			       SX9324_REG_AFE_PH0_PIN_MASK(pin);
 		reg_def->def = raw;
 		break;
+	case SX9324_REG_AFE_CTRL0:
+		ret = device_property_read_string(dev,
+				"semtech,int-comp-resistor", &res);
+		if (ret)
+			break;
+		reg_def->def &= ~SX9324_REG_AFE_CTRL0_RINT_MASK;
+		if (!strcmp(res, "lowest"))
+			reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOWEST;
+		else if (!strcmp(res, "low"))
+			reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOW;
+		else if (!strcmp(res, "high"))
+			reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGH;
+		else if (!strcmp(res, "highest"))
+			reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGHEST;
+
+		break;
 	case SX9324_REG_AFE_CTRL4:
 	case SX9324_REG_AFE_CTRL7:
 		if (reg_def->reg == SX9324_REG_AFE_CTRL4)
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 7/8] dt-bindings: iio: sx9360: Add precharge resistor setting

[Gwendal Grignou]

Allow configure the resistance used during precharge.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Change kOhms into ohms.

Changes since v1:
- Suffix property with kOhms.

 .../bindings/iio/proximity/semtech,sx9360.yaml           | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
index 63e1a1fd00d4c..453d2f02abbb1 100644
--- a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
+++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
@@ -61,6 +61,14 @@ properties:
       UINT_MAX (4294967295) represents infinite. Other values
       represent 1-1/N.
 
+  semtech,input-precharge-resistor-ohms:
+    default: 0
+    description:
+      Pre-charge input resistance in Ohm.
+      Rounded down to a 2000 Ohm multiple.
+    minimum: 0
+    maximum: 30
+
 required:
   - compatible
   - reg
@@ -85,5 +93,6 @@ examples:
         semtech,resolution = <256>;
         semtech,proxraw-strength = <2>;
         semtech,avg-pos-strength = <64>;
+        semtech,input-precharge-resistor-ohms = <4000>;
       };
     };
-- 
2.35.1.1021.g381101b075-goog

[PATCH v3 8/8] iio: sx9360: Add pre-charge resistor setting

[Gwendal Grignou]

Add ability to set the precharge internal resistance from the device
tree.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Change kOhms into ohms.

Changes since v1:
- Suffix field with kOhms unit.

 drivers/iio/proximity/sx9360.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index 3ebb30c8a4f61..21cade4358aeb 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -51,6 +51,8 @@
 #define SX9360_REG_GNRL_REG_2_FREQ(_r)  (SX9360_FOSC_HZ / ((_r) * 8192))
 
 #define SX9360_REG_AFE_CTRL1		0x21
+#define SX9360_REG_AFE_CTRL1_RESFILTIN_MASK GENMASK(3, 0)
+#define SX9360_REG_AFE_CTRL1_RESFILTIN_0OHMS 0
 #define SX9360_REG_AFE_PARAM0_PHR	0x22
 #define SX9360_REG_AFE_PARAM1_PHR	0x23
 #define SX9360_REG_AFE_PARAM0_PHM	0x24
@@ -671,7 +673,7 @@ static const struct sx_common_reg_default sx9360_default_regs[] = {
 	{ SX9360_REG_GNRL_CTRL1, 0x00 },
 	{ SX9360_REG_GNRL_CTRL2, SX9360_REG_GNRL_CTRL2_PERIOD_102MS },
 
-	{ SX9360_REG_AFE_CTRL1, 0x00 },
+	{ SX9360_REG_AFE_CTRL1, SX9360_REG_AFE_CTRL1_RESFILTIN_0OHMS },
 	{ SX9360_REG_AFE_PARAM0_PHR, SX9360_REG_AFE_PARAM0_RSVD |
 		SX9360_REG_AFE_PARAM0_RESOLUTION_128 },
 	{ SX9360_REG_AFE_PARAM1_PHR, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
@@ -722,6 +724,14 @@ sx9360_get_default_reg(struct device *dev, int idx,
 
 	memcpy(reg_def, &sx9360_default_regs[idx], sizeof(*reg_def));
 	switch (reg_def->reg) {
+	case SX9360_REG_AFE_CTRL1:
+		ret = device_property_read_u32(dev,
+				"semtech,input-precharge-resistor-ohms",
+				&raw);
+		reg_def->def &= ~SX9360_REG_AFE_CTRL1_RESFILTIN_MASK;
+		reg_def->def |= FIELD_PREP(SX9360_REG_AFE_CTRL1_RESFILTIN_MASK,
+					   raw / 2000);
+		break;
 	case SX9360_REG_AFE_PARAM0_PHR:
 	case SX9360_REG_AFE_PARAM0_PHM:
 		ret = device_property_read_u32(dev, "semtech,resolution", &raw);
-- 
2.35.1.1021.g381101b075-goog

Re: [PATCH v3 1/8] iio: sx9324: Fix default precharge internal resistance register

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:20)
> Fix the default value for the register that set the resistance:
> it has to be 0x10.
>
> Fixes: 4c18a890dff8d ("iio:proximity:sx9324: Add SX9324 support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

I'd appreciate if you can carry forward tags next time. Then I know
where to focus on things that have changed.

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 2/8] iio: sx9324: Fix register field spelling

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:21)
> Field for PROX_CTRL4 should contain PROX_CTRL4.
>
> Fixes: 4c18a890dff8d ("iio:proximity:sx9324: Add SX9324 support")
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 3/8] dt-bindings: iio: sx9324: Add precharge resistor setting

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:22)
> Allow configure the resistance used during precharge.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 4/8] iio: sx9324: Add precharge internal resistance setting

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:23)
> Add ability to set the precharge internal resistance from the device
> tree.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 5/8] dt-bindings: iio: sx9324: Add internal compensation resistor setting

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:24)
> Allow setting the internal resistor used for compensation.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 6/8] iio: sx9324: Add Setting for internal compensation resistor

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:25)
> diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
> index 3f229dffd5380..38dfdc6dc86ad 100644
> --- a/drivers/iio/proximity/sx9324.c
> +++ b/drivers/iio/proximity/sx9324.c
> @@ -875,6 +881,22 @@ sx9324_get_default_reg(struct device *dev, int idx,
>                                SX9324_REG_AFE_PH0_PIN_MASK(pin);
>                 reg_def->def = raw;
>                 break;
> +       case SX9324_REG_AFE_CTRL0:
> +               ret = device_property_read_string(dev,
> +                               "semtech,int-comp-resistor", &res);
> +               if (ret)
> +                       break;
> +               reg_def->def &= ~SX9324_REG_AFE_CTRL0_RINT_MASK;
> +               if (!strcmp(res, "lowest"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOWEST;
> +               else if (!strcmp(res, "low"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOW;
> +               else if (!strcmp(res, "high"))

Should the order of arguments be swapped? I'm wondering if I can have
'hig' in the DT property and then it will match 'high' here? I suppose
the schema validator will find that and complain first.

> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGH;
> +               else if (!strcmp(res, "highest"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGHEST;
> +
> +               break;

Re: [PATCH v3 7/8] dt-bindings: iio: sx9360: Add precharge resistor setting

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:26)
> Allow configure the resistance used during precharge.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 8/8] iio: sx9360: Add pre-charge resistor setting

[Stephen Boyd]

Quoting Gwendal Grignou (2022-03-25 15:08:27)
> Add ability to set the precharge internal resistance from the device
> tree.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v3 6/8] iio: sx9324: Add Setting for internal compensation resistor

[Jonathan Cameron]

On Fri, 25 Mar 2022 19:04:32 -0500
Stephen Boyd <swboyd@chromium.org> wrote:

> Quoting Gwendal Grignou (2022-03-25 15:08:25)
> > diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
> > index 3f229dffd5380..38dfdc6dc86ad 100644
> > --- a/drivers/iio/proximity/sx9324.c
> > +++ b/drivers/iio/proximity/sx9324.c
> > @@ -875,6 +881,22 @@ sx9324_get_default_reg(struct device *dev, int idx,
> >                                SX9324_REG_AFE_PH0_PIN_MASK(pin);
> >                 reg_def->def = raw;
> >                 break;
> > +       case SX9324_REG_AFE_CTRL0:
> > +               ret = device_property_read_string(dev,
> > +                               "semtech,int-comp-resistor", &res);
> > +               if (ret)
> > +                       break;
> > +               reg_def->def &= ~SX9324_REG_AFE_CTRL0_RINT_MASK;
> > +               if (!strcmp(res, "lowest"))
> > +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOWEST;
> > +               else if (!strcmp(res, "low"))
> > +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOW;
> > +               else if (!strcmp(res, "high"))  
> 
> Should the order of arguments be swapped? I'm wondering if I can have
> 'hig' in the DT property and then it will match 'high' here? I suppose
> the schema validator will find that and complain first.

Wouldn't that result in a failed match between the null terminator of the first
string and the 'h'? Hence I don't think it matters but maybe I'm missing something..

> 
> > +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGH;
> > +               else if (!strcmp(res, "highest"))
> > +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGHEST;
> > +
> > +               break;  

Re: [PATCH v3 0/8] Add settings for precharge and internal resistor

[Jonathan Cameron]

On Fri, 25 Mar 2022 15:08:19 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:

> For Semtech sensors SX9324 and SX9360, allow confugration of the
> pre-charge resistor (9324/9360) and internal resistor (9324).
> 
> Fix register name spelling mistakes first and set default value properly
> for sx9324 internal resistor register.
> 
> The 9360 changes are independent from the 9324 changes, but they are
> very similar.
> 
> Changes since v2:
> - use -ohms instead of -kohms, as ohms is a defined unit in
>   dt-schema.git dtschema/schemas/property-units.yaml
> 
> Changes sinve v1:
> - Add kOhm Unit, add a new patch to fix invalid register setting.

This version looks good to me, but will leave time for dt review.
We aren't in a rush anyway at the moment so I'll probably not pick
it up until after rc1 is available.

Thanks,

Jonathan

> 
> Gwendal Grignou (8):
>   iio: sx9324: Fix default precharge internal resistance register
>   iio: sx9324: Fix register field spelling
>   dt-bindings: iio: sx9324: Add precharge resistor setting
>   iio: sx9324: Add precharge internal resistance setting
>   dt-bindings: iio: sx9324: Add internal compensation resistor setting
>   iio: sx9324: Add Setting for internal compensation resistor
>   dt-bindings: iio: sx9360: Add precharge resistor setting
>   iio: sx9360: Add pre-charge resistor setting
> 
>  .../iio/proximity/semtech,sx9324.yaml         | 19 ++++++++
>  .../iio/proximity/semtech,sx9360.yaml         |  9 ++++
>  drivers/iio/proximity/sx9324.c                | 44 ++++++++++++++++---
>  drivers/iio/proximity/sx9360.c                | 12 ++++-
>  4 files changed, 78 insertions(+), 6 deletions(-)
> 

Re: [PATCH v3 1/8] iio: sx9324: Fix default precharge internal resistance register

[Andy Shevchenko]

On Sat, Mar 26, 2022 at 12:34 AM Gwendal Grignou <gwendal@chromium.org> wrote:
>
> Fix the default value for the register that set the resistance:
> it has to be 0x10.

Has to be according to what? Datasheet? Reverse engineering?

...

> +#define SX9324_REG_AFE_CTRL8_RSVD      0x10

Seems like a BIT() mask here. Also I don't think it's a good idea to
put RSVD. Does it mean "reserved"? Is it really how it's written in
the datasheet?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/8] iio: sx9324: Add Setting for internal compensation resistor

[Andy Shevchenko]

On Sat, Mar 26, 2022 at 12:34 AM Gwendal Grignou <gwendal@chromium.org> wrote:
>
> Based on device tree setting, set the internal compensation resistor.

...

> +               if (!strcmp(res, "lowest"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOWEST;
> +               else if (!strcmp(res, "low"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_LOW;
> +               else if (!strcmp(res, "high"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGH;
> +               else if (!strcmp(res, "highest"))
> +                       reg_def->def |= SX9324_REG_AFE_CTRL0_RINT_HIGHEST;

As far as I can see the values, you can simply put the string literals
into the array:

static const char * const sx9324_rints = { "lowest", "low", "high", "highest" };

Then

  ret = match_string(sx9324_rints, ARRAY_SIZE(sx9324_rints), res);
  if (ret < 0)
    return ret;
  reg_def->def |= ret << SX9324_REG_AFE_CTRL0_RINT_SHIFT;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/8] dt-bindings: iio: sx9324: Add precharge resistor setting

[Rob Herring]

On Fri, 25 Mar 2022 15:08:22 -0700, Gwendal Grignou wrote:
> Allow configure the resistance used during precharge.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v2:
> - Change kOhms into ohms.
> 
> Changes since v1:
> - Suffix field with kOhms unit.
> 
>  .../bindings/iio/proximity/semtech,sx9324.yaml           | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 5/8] dt-bindings: iio: sx9324: Add internal compensation resistor setting

[Rob Herring]

On Fri, 25 Mar 2022 15:08:24 -0700, Gwendal Grignou wrote:
> Allow setting the internal resistor used for compensation.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v2:
> - no changes
> 
> Changes since v1:
> - no changes
> 
>  .../bindings/iio/proximity/semtech,sx9324.yaml         | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 7/8] dt-bindings: iio: sx9360: Add precharge resistor setting

[Rob Herring]

On Fri, Mar 25, 2022 at 03:08:26PM -0700, Gwendal Grignou wrote:
> Allow configure the resistance used during precharge.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v2:
> - Change kOhms into ohms.
> 
> Changes since v1:
> - Suffix property with kOhms.
> 
>  .../bindings/iio/proximity/semtech,sx9360.yaml           | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
> index 63e1a1fd00d4c..453d2f02abbb1 100644
> --- a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
> +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
> @@ -61,6 +61,14 @@ properties:
>        UINT_MAX (4294967295) represents infinite. Other values
>        represent 1-1/N.
>  
> +  semtech,input-precharge-resistor-ohms:
> +    default: 0
> +    description:
> +      Pre-charge input resistance in Ohm.
> +      Rounded down to a 2000 Ohm multiple.
> +    minimum: 0
> +    maximum: 30

This should give you a failure.

> +
>  required:
>    - compatible
>    - reg
> @@ -85,5 +93,6 @@ examples:
>          semtech,resolution = <256>;
>          semtech,proxraw-strength = <2>;
>          semtech,avg-pos-strength = <64>;
> +        semtech,input-precharge-resistor-ohms = <4000>;
>        };
>      };
> -- 
> 2.35.1.1021.g381101b075-goog
> 
>