[PATCH V3 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support

[PATCH V3 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

this patchset add a sc27xx_adc_variant_data structure
and add sc272*,sc273* and ump9620 PMIC support.

Cixi Geng (7):
  dt-bindings:iio:adc: add sprd,ump9620-adc dt-binding
  iio: adc: sc27xx: fix read big scale voltage not right
  iio: adc: sc27xx: structure adjuststment and optimization
  iio: adc: refactor some functions for support more PMiCs
  iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
  iio: adc: sc27xx: add support for PMIC sc2730
  iio: adc: sc27xx: add support for PMIC ump9620

v2 changes:
  fix dt_binding_check error
  adjust some code-style issue
  optimize the copy-paste functions
  the smatch warnings found by lkp
  and  ohter comments by v1 patches.

v3 changes:
  fix nvmem-cells Items value and add ump9620 dt sample
  add the correct signature for each patch
  fix the unused warning in 3/7, add explain for set the scales
  remove duplicate code,add goto label in sc27xx_adc_read
  pull out the refactor code into a single patch
  delete the suspend and resume pm for ump9620
  

 .../bindings/iio/adc/sprd,sc2720-adc.yaml     |  57 +-
 drivers/iio/adc/sc27xx_adc.c                  | 717 ++++++++++++++++--
 2 files changed, 710 insertions(+), 64 deletions(-)

-- 
2.25.1

[PATCH V3 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dt-binding

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

sprd,ump9620-adc is one variant of sc27xx series, add ump9620
description and sample in dt-bindings.

Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 57 +++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
index caa3ee0b4b8c..0d0f317b75c5 100644
--- a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
@@ -20,6 +20,7 @@ properties:
       - sprd,sc2723-adc
       - sprd,sc2730-adc
       - sprd,sc2731-adc
+      - sprd,ump9620-adc
 
   reg:
     maxItems: 1
@@ -34,12 +35,39 @@ properties:
     maxItems: 1
 
   nvmem-cells:
-    maxItems: 2
+    description: nvmem-cells.
 
   nvmem-cell-names:
-    items:
-      - const: big_scale_calib
-      - const: small_scale_calib
+    description: Names for each nvmem-cells specified.
+
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          enum:
+            - sprd,ump9620-adc
+then:
+  properties:
+    nvmem-cells:
+      maxItems: 2
+    nvmem-cell-names:
+      items:
+        - const: big_scale_calib
+        - const: small_scale_calib
+
+else:
+  properties:
+    nvmem-cells:
+      maxItems: 6
+    nvmem-cell-names:
+      items:
+        - const: big_scale_calib1
+        - const: big_scale_calib2
+        - const: small_scale_calib1
+        - const: small_scale_calib2
+        - const: vbat_det_cal1
+        - const: vbat_det_cal2
 
 required:
   - compatible
@@ -69,4 +97,25 @@ examples:
             nvmem-cell-names = "big_scale_calib", "small_scale_calib";
         };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    pmic {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        adc@504 {
+            compatible = "sprd,ump9620-adc";
+            reg = <0x504>;
+            interrupt-parent = <&ump9620_pmic>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            #io-channel-cells = <1>;
+            hwlocks = <&hwlock 4>;
+            nvmem-cells = <&adc_bcal1>, <&adc_bcal2>,
+                          <&adc_scal1>, <&adc_scal2>,
+                          <&vbat_det_cal1>, <&vbat_det_cal2>;
+            nvmem-cell-names = "big_scale_calib1", "big_scale_calib2",
+                               "small_scale_calib1", "small_scale_calib2",
+                               "vbat_det_cal1", "vbat_det_cal2";
+        };
+    };
 ...
-- 
2.25.1

Re: [PATCH V3 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dt-binding

[Rob Herring]

On Thu, Apr 07, 2022 at 04:21:42PM +0800, Cixi Geng wrote:
> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> sprd,ump9620-adc is one variant of sc27xx series, add ump9620
> description and sample in dt-bindings.
> 
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 57 +++++++++++++++++--
>  1 file changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> index caa3ee0b4b8c..0d0f317b75c5 100644
> --- a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> @@ -20,6 +20,7 @@ properties:
>        - sprd,sc2723-adc
>        - sprd,sc2730-adc
>        - sprd,sc2731-adc
> +      - sprd,ump9620-adc
>  
>    reg:
>      maxItems: 1
> @@ -34,12 +35,39 @@ properties:
>      maxItems: 1
>  
>    nvmem-cells:
> -    maxItems: 2
> +    description: nvmem-cells.
>  
>    nvmem-cell-names:
> -    items:
> -      - const: big_scale_calib
> -      - const: small_scale_calib
> +    description: Names for each nvmem-cells specified.

These descriptions of common properties are redundant. Just use 'true' 
for the property values.

> +
> +if:
> +  not:
> +    properties:
> +      compatible:
> +        contains:
> +          enum:
> +            - sprd,ump9620-adc

Use 'const'

> +then:
> +  properties:
> +    nvmem-cells:
> +      maxItems: 2
> +    nvmem-cell-names:
> +      items:
> +        - const: big_scale_calib
> +        - const: small_scale_calib
> +
> +else:
> +  properties:
> +    nvmem-cells:
> +      maxItems: 6
> +    nvmem-cell-names:
> +      items:
> +        - const: big_scale_calib1
> +        - const: big_scale_calib2
> +        - const: small_scale_calib1
> +        - const: small_scale_calib2
> +        - const: vbat_det_cal1
> +        - const: vbat_det_cal2
>  
>  required:
>    - compatible
> @@ -69,4 +97,25 @@ examples:
>              nvmem-cell-names = "big_scale_calib", "small_scale_calib";
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    pmic {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        adc@504 {
> +            compatible = "sprd,ump9620-adc";
> +            reg = <0x504>;
> +            interrupt-parent = <&ump9620_pmic>;
> +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            #io-channel-cells = <1>;
> +            hwlocks = <&hwlock 4>;
> +            nvmem-cells = <&adc_bcal1>, <&adc_bcal2>,
> +                          <&adc_scal1>, <&adc_scal2>,
> +                          <&vbat_det_cal1>, <&vbat_det_cal2>;
> +            nvmem-cell-names = "big_scale_calib1", "big_scale_calib2",
> +                               "small_scale_calib1", "small_scale_calib2",
> +                               "vbat_det_cal1", "vbat_det_cal2";
> +        };
> +    };
>  ...
> -- 
> 2.25.1
> 
> 

Re: [PATCH V3 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dt-binding

[Cixi Geng]

Rob Herring <robh@kernel.org> 于2022年4月14日周四 03:12写道：
>
> On Thu, Apr 07, 2022 at 04:21:42PM +0800, Cixi Geng wrote:
> > From: Cixi Geng <cixi.geng1@unisoc.com>
> >
> > sprd,ump9620-adc is one variant of sc27xx series, add ump9620
> > description and sample in dt-bindings.
> >
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > ---
> >  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 57 +++++++++++++++++--
> >  1 file changed, 53 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > index caa3ee0b4b8c..0d0f317b75c5 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > @@ -20,6 +20,7 @@ properties:
> >        - sprd,sc2723-adc
> >        - sprd,sc2730-adc
> >        - sprd,sc2731-adc
> > +      - sprd,ump9620-adc
> >
> >    reg:
> >      maxItems: 1
> > @@ -34,12 +35,39 @@ properties:
> >      maxItems: 1
> >
> >    nvmem-cells:
> > -    maxItems: 2
> > +    description: nvmem-cells.
> >
> >    nvmem-cell-names:
> > -    items:
> > -      - const: big_scale_calib
> > -      - const: small_scale_calib
> > +    description: Names for each nvmem-cells specified.
>
> These descriptions of common properties are redundant. Just use 'true'
> for the property values.
>
> > +
> > +if:
> > +  not:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          enum:
> > +            - sprd,ump9620-adc
>
> Use 'const'

Hi Rob Herring：
did you mean I should use "- const: sprd,ump9620-adc"? or change the
enum to const?
but the above two modification methods have failed for me to test
dt-bindings-check.
>
> > +then:
> > +  properties:
> > +    nvmem-cells:
> > +      maxItems: 2
> > +    nvmem-cell-names:
> > +      items:
> > +        - const: big_scale_calib
> > +        - const: small_scale_calib
> > +
> > +else:
> > +  properties:
> > +    nvmem-cells:
> > +      maxItems: 6
> > +    nvmem-cell-names:
> > +      items:
> > +        - const: big_scale_calib1
> > +        - const: big_scale_calib2
> > +        - const: small_scale_calib1
> > +        - const: small_scale_calib2
> > +        - const: vbat_det_cal1
> > +        - const: vbat_det_cal2
> >
> >  required:
> >    - compatible
> > @@ -69,4 +97,25 @@ examples:
> >              nvmem-cell-names = "big_scale_calib", "small_scale_calib";
> >          };
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    pmic {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        adc@504 {
> > +            compatible = "sprd,ump9620-adc";
> > +            reg = <0x504>;
> > +            interrupt-parent = <&ump9620_pmic>;
> > +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +            #io-channel-cells = <1>;
> > +            hwlocks = <&hwlock 4>;
> > +            nvmem-cells = <&adc_bcal1>, <&adc_bcal2>,
> > +                          <&adc_scal1>, <&adc_scal2>,
> > +                          <&vbat_det_cal1>, <&vbat_det_cal2>;
> > +            nvmem-cell-names = "big_scale_calib1", "big_scale_calib2",
> > +                               "small_scale_calib1", "small_scale_calib2",
> > +                               "vbat_det_cal1", "vbat_det_cal2";
> > +        };
> > +    };
> >  ...
> > --
> > 2.25.1
> >
> >

Re: [PATCH V3 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dt-binding

[Rob Herring]

On Mon, Apr 18, 2022 at 02:26:29PM +0800, Cixi Geng wrote:
> Rob Herring <robh@kernel.org> 于2022年4月14日周四 03:12写道：
> >
> > On Thu, Apr 07, 2022 at 04:21:42PM +0800, Cixi Geng wrote:
> > > From: Cixi Geng <cixi.geng1@unisoc.com>
> > >
> > > sprd,ump9620-adc is one variant of sc27xx series, add ump9620
> > > description and sample in dt-bindings.
> > >
> > > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > > ---
> > >  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 57 +++++++++++++++++--
> > >  1 file changed, 53 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > > index caa3ee0b4b8c..0d0f317b75c5 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
> > > @@ -20,6 +20,7 @@ properties:
> > >        - sprd,sc2723-adc
> > >        - sprd,sc2730-adc
> > >        - sprd,sc2731-adc
> > > +      - sprd,ump9620-adc
> > >
> > >    reg:
> > >      maxItems: 1
> > > @@ -34,12 +35,39 @@ properties:
> > >      maxItems: 1
> > >
> > >    nvmem-cells:
> > > -    maxItems: 2
> > > +    description: nvmem-cells.
> > >
> > >    nvmem-cell-names:
> > > -    items:
> > > -      - const: big_scale_calib
> > > -      - const: small_scale_calib
> > > +    description: Names for each nvmem-cells specified.
> >
> > These descriptions of common properties are redundant. Just use 'true'
> > for the property values.
> >
> > > +
> > > +if:
> > > +  not:
> > > +    properties:
> > > +      compatible:
> > > +        contains:
> > > +          enum:
> > > +            - sprd,ump9620-adc
> >
> > Use 'const'
> 
> Hi Rob Herring：
> did you mean I should use "- const: sprd,ump9620-adc"? or change the
> enum to const?
> but the above two modification methods have failed for me to test
> dt-bindings-check.

contains:
  const: sprd,ump9620-adc

[PATCH V3 2/7] iio: adc: sc27xx: fix read big scale voltage not right

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

Fix wrong configuration value of SC27XX_ADC_SCALE_MASK and
SC27XX_ADC_SCALE_SHIFT by spec documetation.

Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/iio/adc/sc27xx_adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 00098caf6d9e..aee076c8e2b1 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -36,8 +36,8 @@
 
 /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
 #define SC27XX_ADC_CHN_ID_MASK		GENMASK(4, 0)
-#define SC27XX_ADC_SCALE_MASK		GENMASK(10, 8)
-#define SC27XX_ADC_SCALE_SHIFT		8
+#define SC27XX_ADC_SCALE_MASK		GENMASK(10, 9)
+#define SC27XX_ADC_SCALE_SHIFT		9
 
 /* Bits definitions for SC27XX_ADC_INT_EN registers */
 #define SC27XX_ADC_IRQ_EN		BIT(0)
-- 
2.25.1

Re: [PATCH V3 2/7] iio: adc: sc27xx: fix read big scale voltage not right

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:43 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> Fix wrong configuration value of SC27XX_ADC_SCALE_MASK and
> SC27XX_ADC_SCALE_SHIFT by spec documetation.
> 
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
Fixes tag?
No need to resend for this though, just reply with the appropriate
tag so when I use b4 it'll be picked up.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/sc27xx_adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 00098caf6d9e..aee076c8e2b1 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -36,8 +36,8 @@
>  
>  /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
>  #define SC27XX_ADC_CHN_ID_MASK		GENMASK(4, 0)
> -#define SC27XX_ADC_SCALE_MASK		GENMASK(10, 8)
> -#define SC27XX_ADC_SCALE_SHIFT		8
> +#define SC27XX_ADC_SCALE_MASK		GENMASK(10, 9)
> +#define SC27XX_ADC_SCALE_SHIFT		9
>  
>  /* Bits definitions for SC27XX_ADC_INT_EN registers */
>  #define SC27XX_ADC_IRQ_EN		BIT(0)

Re: [PATCH V3 2/7] iio: adc: sc27xx: fix read big scale voltage not right

[Cixi Geng]

Jonathan Cameron <jic23@kernel.org> 于2022年4月11日周一 00:02写道：
>
> On Thu,  7 Apr 2022 16:21:43 +0800
> Cixi Geng <gengcixi@gmail.com> wrote:
>
> > From: Cixi Geng <cixi.geng1@unisoc.com>
> >
> > Fix wrong configuration value of SC27XX_ADC_SCALE_MASK and
> > SC27XX_ADC_SCALE_SHIFT by spec documetation.
> >
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
> Fixes tag?
Fixes: 5df362a6cf49c (iio: adc: Add Spreadtrum SC27XX PMICs ADC support)
> No need to resend for this though, just reply with the appropriate
> tag so when I use b4 it'll be picked up.
>
> Thanks,
>
> Jonathan
>
> > ---
> >  drivers/iio/adc/sc27xx_adc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index 00098caf6d9e..aee076c8e2b1 100644
> > --- a/drivers/iio/adc/sc27xx_adc.c
> > +++ b/drivers/iio/adc/sc27xx_adc.c
> > @@ -36,8 +36,8 @@
> >
> >  /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
> >  #define SC27XX_ADC_CHN_ID_MASK               GENMASK(4, 0)
> > -#define SC27XX_ADC_SCALE_MASK                GENMASK(10, 8)
> > -#define SC27XX_ADC_SCALE_SHIFT               8
> > +#define SC27XX_ADC_SCALE_MASK                GENMASK(10, 9)
> > +#define SC27XX_ADC_SCALE_SHIFT               9
> >
> >  /* Bits definitions for SC27XX_ADC_INT_EN registers */
> >  #define SC27XX_ADC_IRQ_EN            BIT(0)
>

[PATCH V3 3/7] iio: adc: sc27xx: structure adjuststment and optimization

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

Introduce one variant device data structure to be compatible
with SC2731 PMIC since it has different scale and ratio calculation
and so on.

Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 drivers/iio/adc/sc27xx_adc.c | 99 ++++++++++++++++++++++++++++--------
 1 file changed, 78 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index aee076c8e2b1..28bd70c27420 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -12,9 +12,9 @@
 #include <linux/slab.h>
 
 /* PMIC global registers definition */
-#define SC27XX_MODULE_EN		0xc08
+#define SC2731_MODULE_EN		0xc08
 #define SC27XX_MODULE_ADC_EN		BIT(5)
-#define SC27XX_ARM_CLK_EN		0xc10
+#define SC2731_ARM_CLK_EN		0xc10
 #define SC27XX_CLK_ADC_EN		BIT(5)
 #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
 
@@ -78,6 +78,23 @@ struct sc27xx_adc_data {
 	int channel_scale[SC27XX_ADC_CHANNEL_MAX];
 	u32 base;
 	int irq;
+	const struct sc27xx_adc_variant_data *var_data;
+};
+
+/*
+ * Since different PMICs of SC27xx series can have different
+ * address and ratio, we should save ratio config and base
+ * in the device data structure.
+ */
+struct sc27xx_adc_variant_data {
+	u32 module_en;
+	u32 clk_en;
+	u32 scale_shift;
+	u32 scale_mask;
+	const struct sc27xx_adc_linear_graph *bscale_cal;
+	const struct sc27xx_adc_linear_graph *sscale_cal;
+	void (*init_scale)(struct sc27xx_adc_data *data);
+	int (*get_ratio)(int channel, int scale);
 };
 
 struct sc27xx_adc_linear_graph {
@@ -103,14 +120,15 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
 	100, 341,
 };
 
-static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
-	4200, 856,
-	3600, 733,
+/* Add these for sc2731 pmic, and the [big|small]_scale_graph_calib for common's */
+static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
+	4200, 850,
+	3600, 728,
 };
 
-static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
-	1000, 833,
-	100, 80,
+static const struct sc27xx_adc_linear_graph sc2731_small_scale_graph_calib = {
+	1000, 838,
+	100, 84,
 };
 
 static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
@@ -130,11 +148,11 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 	size_t len;
 
 	if (big_scale) {
-		calib_graph = &big_scale_graph_calib;
+		calib_graph = data->var_data->bscale_cal;
 		graph = &big_scale_graph;
 		cell_name = "big_scale_calib";
 	} else {
-		calib_graph = &small_scale_graph_calib;
+		calib_graph = data->var_data->sscale_cal;
 		graph = &small_scale_graph;
 		cell_name = "small_scale_calib";
 	}
@@ -160,7 +178,7 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 	return 0;
 }
 
-static int sc27xx_adc_get_ratio(int channel, int scale)
+static int sc2731_adc_get_ratio(int channel, int scale)
 {
 	switch (channel) {
 	case 1:
@@ -185,6 +203,23 @@ static int sc27xx_adc_get_ratio(int channel, int scale)
 	return SC27XX_VOLT_RATIO(1, 1);
 }
 
+/*
+ * According to the datasheet set specific value on some channel.
+ */
+static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
+{
+	int i;
+	/* In the current software design, SC2731 support 2 scales,
+	 * channels 5 uses big scale, others use smale.
+	 */
+	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
+		if (i == 5)
+			data->channel_scale[i] = 1;
+		else
+			data->channel_scale[i] = 0;
+	}
+}
+
 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 			   int scale, int *val)
 {
@@ -208,10 +243,11 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 		goto disable_adc;
 
 	/* Configure the channel id and scale */
-	tmp = (scale << SC27XX_ADC_SCALE_SHIFT) & SC27XX_ADC_SCALE_MASK;
+	tmp = (scale << data->var_data->scale_shift) & data->var_data->scale_mask;
 	tmp |= channel & SC27XX_ADC_CHN_ID_MASK;
 	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CH_CFG,
-				 SC27XX_ADC_CHN_ID_MASK | SC27XX_ADC_SCALE_MASK,
+				 SC27XX_ADC_CHN_ID_MASK |
+				 data->var_data->scale_mask,
 				 tmp);
 	if (ret)
 		goto disable_adc;
@@ -262,8 +298,9 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
 				  int channel, int scale,
 				  u32 *div_numerator, u32 *div_denominator)
 {
-	u32 ratio = sc27xx_adc_get_ratio(channel, scale);
+	u32 ratio;
 
+	ratio = data->var_data->get_ratio(channel, scale);
 	*div_numerator = ratio >> SC27XX_RATIO_NUMERATOR_OFFSET;
 	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
 }
@@ -432,13 +469,13 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 {
 	int ret;
 
-	ret = regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
+	ret = regmap_update_bits(data->regmap, data->var_data->module_en,
 				 SC27XX_MODULE_ADC_EN, SC27XX_MODULE_ADC_EN);
 	if (ret)
 		return ret;
 
 	/* Enable ADC work clock and controller clock */
-	ret = regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
+	ret = regmap_update_bits(data->regmap, data->var_data->clk_en,
 				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN,
 				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
 	if (ret)
@@ -456,10 +493,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 	return 0;
 
 disable_clk:
-	regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
+	regmap_update_bits(data->regmap, data->var_data->clk_en,
 			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
 disable_adc:
-	regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
+	regmap_update_bits(data->regmap, data->var_data->module_en,
 			   SC27XX_MODULE_ADC_EN, 0);
 
 	return ret;
@@ -470,21 +507,39 @@ static void sc27xx_adc_disable(void *_data)
 	struct sc27xx_adc_data *data = _data;
 
 	/* Disable ADC work clock and controller clock */
-	regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
+	regmap_update_bits(data->regmap, data->var_data->clk_en,
 			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
 
-	regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
+	regmap_update_bits(data->regmap, data->var_data->module_en,
 			   SC27XX_MODULE_ADC_EN, 0);
 }
 
+static const struct sc27xx_adc_variant_data sc2731_data = {
+	.module_en = SC2731_MODULE_EN,
+	.clk_en = SC2731_ARM_CLK_EN,
+	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
+	.scale_mask = SC27XX_ADC_SCALE_MASK,
+	.bscale_cal = &sc2731_big_scale_graph_calib,
+	.sscale_cal = &sc2731_small_scale_graph_calib,
+	.init_scale = sc2731_adc_scale_init,
+	.get_ratio = sc2731_adc_get_ratio,
+};
+
 static int sc27xx_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct sc27xx_adc_data *sc27xx_data;
+	const struct sc27xx_adc_variant_data *pdata;
 	struct iio_dev *indio_dev;
 	int ret;
 
+	pdata = of_device_get_match_data(dev);
+	if (!pdata) {
+		dev_err(dev, "No matching driver data found\n");
+		return -EINVAL;
+	}
+
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*sc27xx_data));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -520,6 +575,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 	}
 
 	sc27xx_data->dev = dev;
+	sc27xx_data->var_data = pdata;
+	sc27xx_data->var_data->init_scale(sc27xx_data);
 
 	ret = sc27xx_adc_enable(sc27xx_data);
 	if (ret) {
@@ -546,7 +603,7 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id sc27xx_adc_of_match[] = {
-	{ .compatible = "sprd,sc2731-adc", },
+	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
-- 
2.25.1

Re: [PATCH V3 3/7] iio: adc: sc27xx: structure adjuststment and optimization

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:44 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
In patch title "adjustment"
> 
> Introduce one variant device data structure to be compatible
> with SC2731 PMIC since it has different scale and ratio calculation
> and so on.
> 
> Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
Hi Cixi,

Some minor comments inline. Biggest one is you have some (small) functional
changes in this patch that should be pulled out to their own patch with
explanation of why the scales are changing slightly.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/sc27xx_adc.c | 99 ++++++++++++++++++++++++++++--------
>  1 file changed, 78 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index aee076c8e2b1..28bd70c27420 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -12,9 +12,9 @@
>  #include <linux/slab.h>
>  
>  /* PMIC global registers definition */
> -#define SC27XX_MODULE_EN		0xc08
> +#define SC2731_MODULE_EN		0xc08
>  #define SC27XX_MODULE_ADC_EN		BIT(5)
> -#define SC27XX_ARM_CLK_EN		0xc10
> +#define SC2731_ARM_CLK_EN		0xc10
>  #define SC27XX_CLK_ADC_EN		BIT(5)
>  #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
>  
> @@ -78,6 +78,23 @@ struct sc27xx_adc_data {
>  	int channel_scale[SC27XX_ADC_CHANNEL_MAX];
>  	u32 base;
>  	int irq;
> +	const struct sc27xx_adc_variant_data *var_data;
> +};
> +
> +/*
> + * Since different PMICs of SC27xx series can have different
> + * address and ratio, we should save ratio config and base
> + * in the device data structure.
> + */
> +struct sc27xx_adc_variant_data {
> +	u32 module_en;
> +	u32 clk_en;
> +	u32 scale_shift;
> +	u32 scale_mask;
> +	const struct sc27xx_adc_linear_graph *bscale_cal;
> +	const struct sc27xx_adc_linear_graph *sscale_cal;
> +	void (*init_scale)(struct sc27xx_adc_data *data);
> +	int (*get_ratio)(int channel, int scale);
>  };
>  
>  struct sc27xx_adc_linear_graph {
> @@ -103,14 +120,15 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
>  	100, 341,
>  };
>  
> -static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> -	4200, 856,
> -	3600, 733,
> +/* Add these for sc2731 pmic, and the [big|small]_scale_graph_calib for common's */
> +static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
> +	4200, 850,
> +	3600, 728,
>  };
>  
> -static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
> -	1000, 833,
> -	100, 80,
> +static const struct sc27xx_adc_linear_graph sc2731_small_scale_graph_calib = {
> +	1000, 838,

These changes are small, but they aren't the main topic of the patch
so should be in a separate patch. I expected this patch to be a no-op refactoring
but it's not with these value changes.

> +	100, 84,
>  };
>  
>  static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
> @@ -130,11 +148,11 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  	size_t len;
>  
>  	if (big_scale) {
> -		calib_graph = &big_scale_graph_calib;
> +		calib_graph = data->var_data->bscale_cal;
>  		graph = &big_scale_graph;
>  		cell_name = "big_scale_calib";
>  	} else {
> -		calib_graph = &small_scale_graph_calib;
> +		calib_graph = data->var_data->sscale_cal;
>  		graph = &small_scale_graph;
>  		cell_name = "small_scale_calib";
>  	}
> @@ -160,7 +178,7 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  	return 0;
>  }
>  
> -static int sc27xx_adc_get_ratio(int channel, int scale)
> +static int sc2731_adc_get_ratio(int channel, int scale)
>  {
>  	switch (channel) {
>  	case 1:
> @@ -185,6 +203,23 @@ static int sc27xx_adc_get_ratio(int channel, int scale)
>  	return SC27XX_VOLT_RATIO(1, 1);
>  }
>  
> +/*
> + * According to the datasheet set specific value on some channel.
> + */
> +static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
> +{
> +	int i;
> +	/* In the current software design, SC2731 support 2 scales,
Trivial, but in IIO we use
/*
 * In the current software design...
 * channels...
 */
style for multi line comments.

> +	 * channels 5 uses big scale, others use smale.
> +	 */
> +	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> +		if (i == 5)
> +			data->channel_scale[i] = 1;
> +		else
> +			data->channel_scale[i] = 0;
> +	}
> +}
> +
>  static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  			   int scale, int *val)
>  {
> @@ -208,10 +243,11 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  		goto disable_adc;
>  
>  	/* Configure the channel id and scale */
> -	tmp = (scale << SC27XX_ADC_SCALE_SHIFT) & SC27XX_ADC_SCALE_MASK;
> +	tmp = (scale << data->var_data->scale_shift) & data->var_data->scale_mask;
>  	tmp |= channel & SC27XX_ADC_CHN_ID_MASK;
>  	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CH_CFG,
> -				 SC27XX_ADC_CHN_ID_MASK | SC27XX_ADC_SCALE_MASK,
> +				 SC27XX_ADC_CHN_ID_MASK |
> +				 data->var_data->scale_mask,
>  				 tmp);
>  	if (ret)
>  		goto disable_adc;
> @@ -262,8 +298,9 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
>  				  int channel, int scale,
>  				  u32 *div_numerator, u32 *div_denominator)
>  {
> -	u32 ratio = sc27xx_adc_get_ratio(channel, scale);
> +	u32 ratio;
>  
> +	ratio = data->var_data->get_ratio(channel, scale);
>  	*div_numerator = ratio >> SC27XX_RATIO_NUMERATOR_OFFSET;
>  	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
>  }
> @@ -432,13 +469,13 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  {
>  	int ret;
>  
> -	ret = regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
> +	ret = regmap_update_bits(data->regmap, data->var_data->module_en,
>  				 SC27XX_MODULE_ADC_EN, SC27XX_MODULE_ADC_EN);
>  	if (ret)
>  		return ret;
>  
>  	/* Enable ADC work clock and controller clock */
> -	ret = regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
> +	ret = regmap_update_bits(data->regmap, data->var_data->clk_en,
>  				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN,
>  				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
>  	if (ret)
> @@ -456,10 +493,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  	return 0;
>  
>  disable_clk:
> -	regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
> +	regmap_update_bits(data->regmap, data->var_data->clk_en,
>  			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
>  disable_adc:
> -	regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
> +	regmap_update_bits(data->regmap, data->var_data->module_en,
>  			   SC27XX_MODULE_ADC_EN, 0);
>  
>  	return ret;
> @@ -470,21 +507,39 @@ static void sc27xx_adc_disable(void *_data)
>  	struct sc27xx_adc_data *data = _data;
>  
>  	/* Disable ADC work clock and controller clock */
> -	regmap_update_bits(data->regmap, SC27XX_ARM_CLK_EN,
> +	regmap_update_bits(data->regmap, data->var_data->clk_en,
>  			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
>  
> -	regmap_update_bits(data->regmap, SC27XX_MODULE_EN,
> +	regmap_update_bits(data->regmap, data->var_data->module_en,
>  			   SC27XX_MODULE_ADC_EN, 0);
>  }
>  
> +static const struct sc27xx_adc_variant_data sc2731_data = {
> +	.module_en = SC2731_MODULE_EN,
> +	.clk_en = SC2731_ARM_CLK_EN,
> +	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
> +	.scale_mask = SC27XX_ADC_SCALE_MASK,
> +	.bscale_cal = &sc2731_big_scale_graph_calib,
> +	.sscale_cal = &sc2731_small_scale_graph_calib,
> +	.init_scale = sc2731_adc_scale_init,
> +	.get_ratio = sc2731_adc_get_ratio,
> +};
> +
>  static int sc27xx_adc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
>  	struct sc27xx_adc_data *sc27xx_data;
> +	const struct sc27xx_adc_variant_data *pdata;
>  	struct iio_dev *indio_dev;
>  	int ret;
>  
> +	pdata = of_device_get_match_data(dev);
> +	if (!pdata) {
> +		dev_err(dev, "No matching driver data found\n");
> +		return -EINVAL;
> +	}
> +
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*sc27xx_data));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -520,6 +575,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  	}
>  
>  	sc27xx_data->dev = dev;
> +	sc27xx_data->var_data = pdata;
> +	sc27xx_data->var_data->init_scale(sc27xx_data);
>  
>  	ret = sc27xx_adc_enable(sc27xx_data);
>  	if (ret) {
> @@ -546,7 +603,7 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id sc27xx_adc_of_match[] = {
> -	{ .compatible = "sprd,sc2731-adc", },
> +	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);

[PATCH V3 4/7] iio: adc: refactor some functions for support more PMiCs

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

refactor the common adc_nvmem_cell_calib_data,adc_to_volt and call
these in the origin sc27xx_adc_scale_calibration,sc27xx_adc_to_volt

Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 drivers/iio/adc/sc27xx_adc.c | 57 ++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 28bd70c27420..60c0a6aa3f45 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -136,16 +136,41 @@ static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
 	return ((calib_data & 0xff) + calib_adc - 128) * 4;
 }
 
+/* get the adc nvmem cell calibration data */
+static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
+{
+	struct nvmem_cell *cell;
+	void *buf;
+	u32 origin_calib_data = 0;
+	size_t len;
+
+	if (!data)
+		return -EINVAL;
+
+	cell = nvmem_cell_get(data->dev, cell_name);
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &len);
+	if (IS_ERR(buf)) {
+		nvmem_cell_put(cell);
+		return PTR_ERR(buf);
+	}
+
+	memcpy(&origin_calib_data, buf, min(len, sizeof(u32)));
+
+	kfree(buf);
+	nvmem_cell_put(cell);
+	return origin_calib_data;
+}
+
 static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 					bool big_scale)
 {
 	const struct sc27xx_adc_linear_graph *calib_graph;
 	struct sc27xx_adc_linear_graph *graph;
-	struct nvmem_cell *cell;
 	const char *cell_name;
 	u32 calib_data = 0;
-	void *buf;
-	size_t len;
 
 	if (big_scale) {
 		calib_graph = data->var_data->bscale_cal;
@@ -157,24 +182,13 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 		cell_name = "small_scale_calib";
 	}
 
-	cell = nvmem_cell_get(data->dev, cell_name);
-	if (IS_ERR(cell))
-		return PTR_ERR(cell);
-
-	buf = nvmem_cell_read(cell, &len);
-	nvmem_cell_put(cell);
-
-	if (IS_ERR(buf))
-		return PTR_ERR(buf);
-
-	memcpy(&calib_data, buf, min(len, sizeof(u32)));
+	calib_data = adc_nvmem_cell_calib_data(data, cell_name);
 
 	/* Only need to calibrate the adc values in the linear graph. */
 	graph->adc0 = sc27xx_adc_get_calib_data(calib_data, calib_graph->adc0);
 	graph->adc1 = sc27xx_adc_get_calib_data(calib_data >> 8,
 						calib_graph->adc1);
 
-	kfree(buf);
 	return 0;
 }
 
@@ -285,6 +299,7 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 disable_adc:
 	regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
 			   SC27XX_ADC_EN, 0);
+
 unlock_adc:
 	hwspin_unlock_raw(data->hwlock);
 
@@ -305,7 +320,7 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
 	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
 }
 
-static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
+static int adc_to_volt(struct sc27xx_adc_linear_graph *graph,
 			      int raw_adc)
 {
 	int tmp;
@@ -314,6 +329,16 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
 	tmp /= (graph->adc0 - graph->adc1);
 	tmp += graph->volt1;
 
+	return tmp;
+}
+
+static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
+			      int raw_adc)
+{
+	int tmp;
+
+	tmp = adc_to_volt(graph, raw_adc);
+
 	return tmp < 0 ? 0 : tmp;
 }
 
-- 
2.25.1

Re: [PATCH V3 4/7] iio: adc: refactor some functions for support more PMiCs

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:45 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> refactor the common adc_nvmem_cell_calib_data,adc_to_volt and call
> these in the origin sc27xx_adc_scale_calibration,sc27xx_adc_to_volt
> 
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
One trivial comment inline.

> ---
>  drivers/iio/adc/sc27xx_adc.c | 57 ++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 28bd70c27420..60c0a6aa3f45 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -136,16 +136,41 @@ static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
>  	return ((calib_data & 0xff) + calib_adc - 128) * 4;
>  }
>  
> +/* get the adc nvmem cell calibration data */
> +static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
> +{
> +	struct nvmem_cell *cell;
> +	void *buf;
> +	u32 origin_calib_data = 0;
> +	size_t len;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	cell = nvmem_cell_get(data->dev, cell_name);
> +	if (IS_ERR(cell))
> +		return PTR_ERR(cell);
> +
> +	buf = nvmem_cell_read(cell, &len);
> +	if (IS_ERR(buf)) {
> +		nvmem_cell_put(cell);
> +		return PTR_ERR(buf);
> +	}
> +
> +	memcpy(&origin_calib_data, buf, min(len, sizeof(u32)));
> +
> +	kfree(buf);
> +	nvmem_cell_put(cell);
> +	return origin_calib_data;
> +}
> +
>  static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  					bool big_scale)
>  {
>  	const struct sc27xx_adc_linear_graph *calib_graph;
>  	struct sc27xx_adc_linear_graph *graph;
> -	struct nvmem_cell *cell;
>  	const char *cell_name;
>  	u32 calib_data = 0;
> -	void *buf;
> -	size_t len;
>  
>  	if (big_scale) {
>  		calib_graph = data->var_data->bscale_cal;
> @@ -157,24 +182,13 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  		cell_name = "small_scale_calib";
>  	}
>  
> -	cell = nvmem_cell_get(data->dev, cell_name);
> -	if (IS_ERR(cell))
> -		return PTR_ERR(cell);
> -
> -	buf = nvmem_cell_read(cell, &len);
> -	nvmem_cell_put(cell);
> -
> -	if (IS_ERR(buf))
> -		return PTR_ERR(buf);
> -
> -	memcpy(&calib_data, buf, min(len, sizeof(u32)));
> +	calib_data = adc_nvmem_cell_calib_data(data, cell_name);
>  
>  	/* Only need to calibrate the adc values in the linear graph. */
>  	graph->adc0 = sc27xx_adc_get_calib_data(calib_data, calib_graph->adc0);
>  	graph->adc1 = sc27xx_adc_get_calib_data(calib_data >> 8,
>  						calib_graph->adc1);
>  
> -	kfree(buf);
>  	return 0;
>  }
>  
> @@ -285,6 +299,7 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  disable_adc:
>  	regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>  			   SC27XX_ADC_EN, 0);
> +

Unrelated change that shouldn't be in this patch.

>  unlock_adc:
>  	hwspin_unlock_raw(data->hwlock);
>  
> @@ -305,7 +320,7 @@ static void sc27xx_adc_volt_ratio(struct sc27xx_adc_data *data,
>  	*div_denominator = ratio & SC27XX_RATIO_DENOMINATOR_MASK;
>  }
>  
> -static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
> +static int adc_to_volt(struct sc27xx_adc_linear_graph *graph,
>  			      int raw_adc)
>  {
>  	int tmp;
> @@ -314,6 +329,16 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
>  	tmp /= (graph->adc0 - graph->adc1);
>  	tmp += graph->volt1;
>  
> +	return tmp;
> +}
> +
> +static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
> +			      int raw_adc)
> +{
> +	int tmp;
> +
> +	tmp = adc_to_volt(graph, raw_adc);
> +
>  	return tmp < 0 ? 0 : tmp;
>  }
>  

[PATCH V3 5/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

sc2720 and sc2721 is the product of sc27xx series.

Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 drivers/iio/adc/sc27xx_adc.c | 211 ++++++++++++++++++++++++++++++++++-
 1 file changed, 209 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 60c0a6aa3f45..eb9e789dd8ee 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -9,11 +9,13 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
 /* PMIC global registers definition */
 #define SC2731_MODULE_EN		0xc08
 #define SC27XX_MODULE_ADC_EN		BIT(5)
+#define SC2721_ARM_CLK_EN		0xc0c
 #define SC2731_ARM_CLK_EN		0xc10
 #define SC27XX_CLK_ADC_EN		BIT(5)
 #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
@@ -37,7 +39,9 @@
 /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
 #define SC27XX_ADC_CHN_ID_MASK		GENMASK(4, 0)
 #define SC27XX_ADC_SCALE_MASK		GENMASK(10, 9)
+#define SC2721_ADC_SCALE_MASK		BIT(5)
 #define SC27XX_ADC_SCALE_SHIFT		9
+#define SC2721_ADC_SCALE_SHIFT		5
 
 /* Bits definitions for SC27XX_ADC_INT_EN registers */
 #define SC27XX_ADC_IRQ_EN		BIT(0)
@@ -67,8 +71,20 @@
 #define SC27XX_RATIO_NUMERATOR_OFFSET	16
 #define SC27XX_RATIO_DENOMINATOR_MASK	GENMASK(15, 0)
 
+/* ADC specific channel reference voltage 3.5V */
+#define SC27XX_ADC_REFVOL_VDD35		3500000
+
+/* ADC default channel reference voltage is 2.8V */
+#define SC27XX_ADC_REFVOL_VDD28		2800000
+
+enum sc27xx_pmic_type {
+	SC27XX_ADC,
+	SC2721_ADC,
+};
+
 struct sc27xx_adc_data {
 	struct device *dev;
+	struct regulator *volref;
 	struct regmap *regmap;
 	/*
 	 * One hardware spinlock to synchronize between the multiple
@@ -87,6 +103,7 @@ struct sc27xx_adc_data {
  * in the device data structure.
  */
 struct sc27xx_adc_variant_data {
+	enum sc27xx_pmic_type pmic_type;
 	u32 module_en;
 	u32 clk_en;
 	u32 scale_shift;
@@ -131,6 +148,16 @@ static const struct sc27xx_adc_linear_graph sc2731_small_scale_graph_calib = {
 	100, 84,
 };
 
+static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
+	4200, 856,
+	3600, 733,
+};
+
+static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
+	1000, 833,
+	100, 80,
+};
+
 static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
 {
 	return ((calib_data & 0xff) + calib_adc - 128) * 4;
@@ -192,6 +219,94 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 	return 0;
 }
 
+static int sc2720_adc_get_ratio(int channel, int scale)
+{
+	switch (channel) {
+	case 14:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(68, 900);
+		case 1:
+			return SC27XX_VOLT_RATIO(68, 1760);
+		case 2:
+			return SC27XX_VOLT_RATIO(68, 2327);
+		case 3:
+			return SC27XX_VOLT_RATIO(68, 3654);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 16:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(48, 100);
+		case 1:
+			return SC27XX_VOLT_RATIO(480, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(480, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(48, 406);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 21:
+	case 22:
+	case 23:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(3, 8);
+		case 1:
+			return SC27XX_VOLT_RATIO(375, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(375, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(300, 3248);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	default:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(1, 1);
+		case 1:
+			return SC27XX_VOLT_RATIO(1000, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(1000, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(100, 406);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	}
+	return SC27XX_VOLT_RATIO(1, 1);
+}
+
+static int sc2721_adc_get_ratio(int channel, int scale)
+{
+	switch (channel) {
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		return scale ? SC27XX_VOLT_RATIO(400, 1025) :
+			SC27XX_VOLT_RATIO(1, 1);
+	case 5:
+		return SC27XX_VOLT_RATIO(7, 29);
+	case 7:
+	case 9:
+		return scale ? SC27XX_VOLT_RATIO(100, 125) :
+			SC27XX_VOLT_RATIO(1, 1);
+	case 14:
+		return SC27XX_VOLT_RATIO(68, 900);
+	case 16:
+		return SC27XX_VOLT_RATIO(48, 100);
+	case 19:
+		return SC27XX_VOLT_RATIO(1, 3);
+	default:
+		return SC27XX_VOLT_RATIO(1, 1);
+	}
+	return SC27XX_VOLT_RATIO(1, 1);
+}
+
 static int sc2731_adc_get_ratio(int channel, int scale)
 {
 	switch (channel) {
@@ -220,6 +335,34 @@ static int sc2731_adc_get_ratio(int channel, int scale)
 /*
  * According to the datasheet set specific value on some channel.
  */
+static void sc2720_adc_scale_init(struct sc27xx_adc_data *data)
+{
+	int i;
+
+	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
+		switch (i) {
+		case 5:
+			data->channel_scale[i] = 3;
+			break;
+		case 7:
+		case 9:
+			data->channel_scale[i] = 2;
+			break;
+		case 13:
+			data->channel_scale[i] = 1;
+			break;
+		case 19:
+		case 30:
+		case 31:
+			data->channel_scale[i] = 3;
+			break;
+		default:
+			data->channel_scale[i] = 0;
+			break;
+		}
+	}
+}
+
 static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
 {
 	int i;
@@ -237,7 +380,7 @@ static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 			   int scale, int *val)
 {
-	int ret;
+	int ret, ret_volref;
 	u32 tmp, value, status;
 
 	ret = hwspin_lock_timeout_raw(data->hwlock, SC27XX_ADC_HWLOCK_TIMEOUT);
@@ -246,10 +389,25 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 		return ret;
 	}
 
+	/*
+	 * According to the sc2721 chip data sheet, the reference voltage of
+	 * specific channel 30 and channel 31 in ADC module needs to be set from
+	 * the default 2.8v to 3.5v.
+	 */
+	if ((data->var_data->pmic_type == SC2721_ADC) && (channel == 30 || channel == 31)) {
+		ret = regulator_set_voltage(data->volref,
+					SC27XX_ADC_REFVOL_VDD35,
+					SC27XX_ADC_REFVOL_VDD35);
+		if (ret) {
+			dev_err(data->dev, "failed to set the volref 3.5v\n");
+			goto unlock_adc;
+		}
+	}
+
 	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
 				 SC27XX_ADC_EN, SC27XX_ADC_EN);
 	if (ret)
-		goto unlock_adc;
+		goto regulator_restore;
 
 	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
 				 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
@@ -300,6 +458,17 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 	regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
 			   SC27XX_ADC_EN, 0);
 
+regulator_restore:
+	if ((data->var_data->pmic_type == SC2721_ADC) && (channel == 30 || channel == 31)) {
+		ret_volref = regulator_set_voltage(data->volref,
+					    SC27XX_ADC_REFVOL_VDD28,
+					    SC27XX_ADC_REFVOL_VDD28);
+		if (ret_volref) {
+			dev_err(data->dev, "failed to set the volref 2.8v,ret_volref = 0x%x\n",
+					 ret_volref);
+			ret = ret || ret_volref;
+		}
+	}
 unlock_adc:
 	hwspin_unlock_raw(data->hwlock);
 
@@ -540,6 +709,7 @@ static void sc27xx_adc_disable(void *_data)
 }
 
 static const struct sc27xx_adc_variant_data sc2731_data = {
+	.pmic_type = SC27XX_ADC,
 	.module_en = SC2731_MODULE_EN,
 	.clk_en = SC2731_ARM_CLK_EN,
 	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
@@ -550,6 +720,30 @@ static const struct sc27xx_adc_variant_data sc2731_data = {
 	.get_ratio = sc2731_adc_get_ratio,
 };
 
+static const struct sc27xx_adc_variant_data sc2721_data = {
+	.pmic_type = SC2721_ADC,
+	.module_en = SC2731_MODULE_EN,
+	.clk_en = SC2721_ARM_CLK_EN,
+	.scale_shift = SC2721_ADC_SCALE_SHIFT,
+	.scale_mask = SC2721_ADC_SCALE_MASK,
+	.bscale_cal = &sc2731_big_scale_graph_calib,
+	.sscale_cal = &sc2731_small_scale_graph_calib,
+	.init_scale = sc2731_adc_scale_init,
+	.get_ratio = sc2721_adc_get_ratio,
+};
+
+static const struct sc27xx_adc_variant_data sc2720_data = {
+	.pmic_type = SC27XX_ADC,
+	.module_en = SC2731_MODULE_EN,
+	.clk_en = SC2721_ARM_CLK_EN,
+	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
+	.scale_mask = SC27XX_ADC_SCALE_MASK,
+	.bscale_cal = &big_scale_graph_calib,
+	.sscale_cal = &small_scale_graph_calib,
+	.init_scale = sc2720_adc_scale_init,
+	.get_ratio = sc2720_adc_get_ratio,
+};
+
 static int sc27xx_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -600,6 +794,17 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 	}
 
 	sc27xx_data->dev = dev;
+	if (pdata->pmic_type == SC2721_ADC) {
+		sc27xx_data->volref = devm_regulator_get(dev, "vref");
+		if (IS_ERR(sc27xx_data->volref)) {
+			ret = PTR_ERR(sc27xx_data->volref);
+			if (ret == -ENODEV)
+				dev_err(dev, "failed to supply the regulator\n");
+			dev_err(dev, "failed to get ADC volref, the err volref: %d\n", ret);
+			return ret;
+		}
+	}
+
 	sc27xx_data->var_data = pdata;
 	sc27xx_data->var_data->init_scale(sc27xx_data);
 
@@ -629,6 +834,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 
 static const struct of_device_id sc27xx_adc_of_match[] = {
 	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
+	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
+	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
-- 
2.25.1

Re: [PATCH V3 5/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:46 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> sc2720 and sc2721 is the product of sc27xx series.
> 
> Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Reported-by: kernel test robot <lkp@intel.com>
Please state what the kernel test robot / Dan reported.
Currently this tag implies the whole patch is fixing something they
reported.

Reported-by: kernel test robot <lkp@intel.com> #whatever it was
works for this.  Note that you don't have to credit them in
a patch. That comment in the autogenerated email is really
intended for when things have already be merged and a patch fixing
the issue they have detected alone is sent.
 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

I don't like having a mixture of a nice chip model specific structure and
an enum. So please encode everything in the chip model specific structure
explicitly.  See below for what I mean about the handling of the vref
supply.

Jonathan

> ---
>  drivers/iio/adc/sc27xx_adc.c | 211 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 209 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 60c0a6aa3f45..eb9e789dd8ee 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -9,11 +9,13 @@
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
>  /* PMIC global registers definition */
>  #define SC2731_MODULE_EN		0xc08
>  #define SC27XX_MODULE_ADC_EN		BIT(5)
> +#define SC2721_ARM_CLK_EN		0xc0c
>  #define SC2731_ARM_CLK_EN		0xc10
>  #define SC27XX_CLK_ADC_EN		BIT(5)
>  #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
> @@ -37,7 +39,9 @@
>  /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
>  #define SC27XX_ADC_CHN_ID_MASK		GENMASK(4, 0)
>  #define SC27XX_ADC_SCALE_MASK		GENMASK(10, 9)
> +#define SC2721_ADC_SCALE_MASK		BIT(5)
>  #define SC27XX_ADC_SCALE_SHIFT		9
> +#define SC2721_ADC_SCALE_SHIFT		5
>  
>  /* Bits definitions for SC27XX_ADC_INT_EN registers */
>  #define SC27XX_ADC_IRQ_EN		BIT(0)
> @@ -67,8 +71,20 @@
>  #define SC27XX_RATIO_NUMERATOR_OFFSET	16
>  #define SC27XX_RATIO_DENOMINATOR_MASK	GENMASK(15, 0)
>  
> +/* ADC specific channel reference voltage 3.5V */
> +#define SC27XX_ADC_REFVOL_VDD35		3500000
> +
> +/* ADC default channel reference voltage is 2.8V */
> +#define SC27XX_ADC_REFVOL_VDD28		2800000
> +
> +enum sc27xx_pmic_type {
> +	SC27XX_ADC,
> +	SC2721_ADC,
> +};
> +
>  struct sc27xx_adc_data {
>  	struct device *dev;
> +	struct regulator *volref;
>  	struct regmap *regmap;
>  	/*
>  	 * One hardware spinlock to synchronize between the multiple
> @@ -87,6 +103,7 @@ struct sc27xx_adc_data {
>   * in the device data structure.
>   */
>  struct sc27xx_adc_variant_data {
> +	enum sc27xx_pmic_type pmic_type;
>  	u32 module_en;
>  	u32 clk_en;
>  	u32 scale_shift;
> @@ -131,6 +148,16 @@ static const struct sc27xx_adc_linear_graph sc2731_small_scale_graph_calib = {
>  	100, 84,
>  };
>  
> +static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> +	4200, 856,
> +	3600, 733,
> +};
> +
> +static const struct sc27xx_adc_linear_graph small_scale_graph_calib = {
> +	1000, 833,
> +	100, 80,
> +};
> +
>  static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
>  {
>  	return ((calib_data & 0xff) + calib_adc - 128) * 4;
> @@ -192,6 +219,94 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  	return 0;
>  }
>  
> +static int sc2720_adc_get_ratio(int channel, int scale)
> +{
> +	switch (channel) {
> +	case 14:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(68, 900);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(68, 1760);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(68, 2327);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(68, 3654);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 16:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(48, 100);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(480, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(480, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(48, 406);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 21:
> +	case 22:
> +	case 23:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(3, 8);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(375, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(375, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(300, 3248);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	default:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(1000, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(1000, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(100, 406);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	}
> +	return SC27XX_VOLT_RATIO(1, 1);
> +}
> +
> +static int sc2721_adc_get_ratio(int channel, int scale)
> +{
> +	switch (channel) {
> +	case 1:
> +	case 2:
> +	case 3:
> +	case 4:
> +		return scale ? SC27XX_VOLT_RATIO(400, 1025) :
> +			SC27XX_VOLT_RATIO(1, 1);
> +	case 5:
> +		return SC27XX_VOLT_RATIO(7, 29);
> +	case 7:
> +	case 9:
> +		return scale ? SC27XX_VOLT_RATIO(100, 125) :
> +			SC27XX_VOLT_RATIO(1, 1);
> +	case 14:
> +		return SC27XX_VOLT_RATIO(68, 900);
> +	case 16:
> +		return SC27XX_VOLT_RATIO(48, 100);
> +	case 19:
> +		return SC27XX_VOLT_RATIO(1, 3);
> +	default:
> +		return SC27XX_VOLT_RATIO(1, 1);
> +	}
> +	return SC27XX_VOLT_RATIO(1, 1);
> +}
> +
>  static int sc2731_adc_get_ratio(int channel, int scale)
>  {
>  	switch (channel) {
> @@ -220,6 +335,34 @@ static int sc2731_adc_get_ratio(int channel, int scale)
>  /*
>   * According to the datasheet set specific value on some channel.
>   */
> +static void sc2720_adc_scale_init(struct sc27xx_adc_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> +		switch (i) {
> +		case 5:
> +			data->channel_scale[i] = 3;
> +			break;
> +		case 7:
> +		case 9:
> +			data->channel_scale[i] = 2;
> +			break;
> +		case 13:
> +			data->channel_scale[i] = 1;
> +			break;
> +		case 19:
> +		case 30:
> +		case 31:
> +			data->channel_scale[i] = 3;
> +			break;
> +		default:
> +			data->channel_scale[i] = 0;
> +			break;
> +		}
> +	}
> +}
> +
>  static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
>  {
>  	int i;
> @@ -237,7 +380,7 @@ static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
>  static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  			   int scale, int *val)
>  {
> -	int ret;
> +	int ret, ret_volref;
>  	u32 tmp, value, status;
>  
>  	ret = hwspin_lock_timeout_raw(data->hwlock, SC27XX_ADC_HWLOCK_TIMEOUT);
> @@ -246,10 +389,25 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  		return ret;
>  	}
>  
> +	/*
> +	 * According to the sc2721 chip data sheet, the reference voltage of
> +	 * specific channel 30 and channel 31 in ADC module needs to be set from
> +	 * the default 2.8v to 3.5v.
> +	 */
> +	if ((data->var_data->pmic_type == SC2721_ADC) && (channel == 30 || channel == 31)) {
As below. I'd prefer
	if (data->var_data->vref && (channel...

> +		ret = regulator_set_voltage(data->volref,
> +					SC27XX_ADC_REFVOL_VDD35,
> +					SC27XX_ADC_REFVOL_VDD35);
> +		if (ret) {
> +			dev_err(data->dev, "failed to set the volref 3.5v\n");
> +			goto unlock_adc;
> +		}
> +	}
> +
>  	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>  				 SC27XX_ADC_EN, SC27XX_ADC_EN);
>  	if (ret)
> -		goto unlock_adc;
> +		goto regulator_restore;
>  
>  	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
>  				 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
> @@ -300,6 +458,17 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  	regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>  			   SC27XX_ADC_EN, 0);
>  
> +regulator_restore:
> +	if ((data->var_data->pmic_type == SC2721_ADC) && (channel == 30 || channel == 31)) {
> +		ret_volref = regulator_set_voltage(data->volref,
> +					    SC27XX_ADC_REFVOL_VDD28,
> +					    SC27XX_ADC_REFVOL_VDD28);
> +		if (ret_volref) {
> +			dev_err(data->dev, "failed to set the volref 2.8v,ret_volref = 0x%x\n",
> +					 ret_volref);
> +			ret = ret || ret_volref;
> +		}
> +	}
>  unlock_adc:
>  	hwspin_unlock_raw(data->hwlock);
>  
> @@ -540,6 +709,7 @@ static void sc27xx_adc_disable(void *_data)
>  }
>  
>  static const struct sc27xx_adc_variant_data sc2731_data = {
> +	.pmic_type = SC27XX_ADC,
>  	.module_en = SC2731_MODULE_EN,
>  	.clk_en = SC2731_ARM_CLK_EN,
>  	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
> @@ -550,6 +720,30 @@ static const struct sc27xx_adc_variant_data sc2731_data = {
>  	.get_ratio = sc2731_adc_get_ratio,
>  };
>  
> +static const struct sc27xx_adc_variant_data sc2721_data = {
> +	.pmic_type = SC2721_ADC,
> +	.module_en = SC2731_MODULE_EN,
> +	.clk_en = SC2721_ARM_CLK_EN,
> +	.scale_shift = SC2721_ADC_SCALE_SHIFT,
> +	.scale_mask = SC2721_ADC_SCALE_MASK,
> +	.bscale_cal = &sc2731_big_scale_graph_calib,
> +	.sscale_cal = &sc2731_small_scale_graph_calib,
> +	.init_scale = sc2731_adc_scale_init,
> +	.get_ratio = sc2721_adc_get_ratio,
> +};
> +
> +static const struct sc27xx_adc_variant_data sc2720_data = {
> +	.pmic_type = SC27XX_ADC,
> +	.module_en = SC2731_MODULE_EN,
> +	.clk_en = SC2721_ARM_CLK_EN,
> +	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
> +	.scale_mask = SC27XX_ADC_SCALE_MASK,
> +	.bscale_cal = &big_scale_graph_calib,
> +	.sscale_cal = &small_scale_graph_calib,
> +	.init_scale = sc2720_adc_scale_init,
> +	.get_ratio = sc2720_adc_get_ratio,
> +};
> +
>  static int sc27xx_adc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -600,6 +794,17 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  	}
>  
>  	sc27xx_data->dev = dev;
> +	if (pdata->pmic_type == SC2721_ADC) {

I'd prefer it if this was encoded explicitly in the pdata.
Perhaps add a bool pdata.vref that is set only for
the SC2721.  That builds in flexibility for future variants that need
this feature.  Then you don't need to carry a pmic_type around, just
use the vref flag for that.

You may need to make that more complex long term to cope with
devices that need vref to be tweaked for different sets of channels, but
that can be solved when the need comes up.

> +		sc27xx_data->volref = devm_regulator_get(dev, "vref");
> +		if (IS_ERR(sc27xx_data->volref)) {
> +			ret = PTR_ERR(sc27xx_data->volref);
> +			if (ret == -ENODEV)
> +				dev_err(dev, "failed to supply the regulator\n");

Why is it useful to have a double error print in this path?  Next message will print that
it was an ENODEV anyway.

> +			dev_err(dev, "failed to get ADC volref, the err volref: %d\n", ret);
This could be a deferred probe situation so you should use
dev_err_probe()


> +			return ret;
> +		}
> +	}
> +
>  	sc27xx_data->var_data = pdata;
>  	sc27xx_data->var_data->init_scale(sc27xx_data);
>  
> @@ -629,6 +834,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  
>  static const struct of_device_id sc27xx_adc_of_match[] = {
>  	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
> +	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
> +	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);

[PATCH V3 6/7] iio: adc: sc27xx: add support for PMIC sc2730

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

sc2730 is the product of sc27xx series.

Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 drivers/iio/adc/sc27xx_adc.c | 105 +++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index eb9e789dd8ee..43655a818d09 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -13,9 +13,11 @@
 #include <linux/slab.h>
 
 /* PMIC global registers definition */
+#define SC2730_MODULE_EN		0x1808
 #define SC2731_MODULE_EN		0xc08
 #define SC27XX_MODULE_ADC_EN		BIT(5)
 #define SC2721_ARM_CLK_EN		0xc0c
+#define SC2730_ARM_CLK_EN		0x180c
 #define SC2731_ARM_CLK_EN		0xc10
 #define SC27XX_CLK_ADC_EN		BIT(5)
 #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
@@ -307,6 +309,80 @@ static int sc2721_adc_get_ratio(int channel, int scale)
 	return SC27XX_VOLT_RATIO(1, 1);
 }
 
+static int sc2730_adc_get_ratio(int channel, int scale)
+{
+	switch (channel) {
+	case 14:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(68, 900);
+		case 1:
+			return SC27XX_VOLT_RATIO(68, 1760);
+		case 2:
+			return SC27XX_VOLT_RATIO(68, 2327);
+		case 3:
+			return SC27XX_VOLT_RATIO(68, 3654);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 15:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(1, 3);
+		case 1:
+			return SC27XX_VOLT_RATIO(1000, 5865);
+		case 2:
+			return SC27XX_VOLT_RATIO(500, 3879);
+		case 3:
+			return SC27XX_VOLT_RATIO(500, 6090);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 16:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(48, 100);
+		case 1:
+			return SC27XX_VOLT_RATIO(480, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(480, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(48, 406);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 21:
+	case 22:
+	case 23:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(3, 8);
+		case 1:
+			return SC27XX_VOLT_RATIO(375, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(375, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(300, 3248);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	default:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(1, 1);
+		case 1:
+			return SC27XX_VOLT_RATIO(1000, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(1000, 2586);
+		case 3:
+			return SC27XX_VOLT_RATIO(1000, 4060);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	}
+	return SC27XX_VOLT_RATIO(1, 1);
+}
+
 static int sc2731_adc_get_ratio(int channel, int scale)
 {
 	switch (channel) {
@@ -363,6 +439,22 @@ static void sc2720_adc_scale_init(struct sc27xx_adc_data *data)
 	}
 }
 
+static void sc2730_adc_scale_init(struct sc27xx_adc_data *data)
+{
+	int i;
+
+	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
+		if (i == 5 || i == 10 || i == 19 || i == 30 || i == 31)
+			data->channel_scale[i] = 3;
+		else if (i == 7 || i == 9)
+			data->channel_scale[i] = 2;
+		else if (i == 13)
+			data->channel_scale[i] = 1;
+		else
+			data->channel_scale[i] = 0;
+	}
+}
+
 static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
 {
 	int i;
@@ -720,6 +812,18 @@ static const struct sc27xx_adc_variant_data sc2731_data = {
 	.get_ratio = sc2731_adc_get_ratio,
 };
 
+static const struct sc27xx_adc_variant_data sc2730_data = {
+	.pmic_type = SC27XX_ADC,
+	.module_en = SC2730_MODULE_EN,
+	.clk_en = SC2730_ARM_CLK_EN,
+	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
+	.scale_mask = SC27XX_ADC_SCALE_MASK,
+	.bscale_cal = &big_scale_graph_calib,
+	.sscale_cal = &small_scale_graph_calib,
+	.init_scale = sc2730_adc_scale_init,
+	.get_ratio = sc2730_adc_get_ratio,
+};
+
 static const struct sc27xx_adc_variant_data sc2721_data = {
 	.pmic_type = SC2721_ADC,
 	.module_en = SC2731_MODULE_EN,
@@ -834,6 +938,7 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 
 static const struct of_device_id sc27xx_adc_of_match[] = {
 	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
+	{ .compatible = "sprd,sc2730-adc", .data = &sc2730_data},
 	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
 	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
 	{ }
-- 
2.25.1

Re: [PATCH V3 6/7] iio: adc: sc27xx: add support for PMIC sc2730

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:47 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> sc2730 is the product of sc27xx series.
> 
> Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

One minor comment inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/sc27xx_adc.c | 105 +++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index eb9e789dd8ee..43655a818d09 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -13,9 +13,11 @@
>  #include <linux/slab.h>
>  
>  /* PMIC global registers definition */
> +#define SC2730_MODULE_EN		0x1808
>  #define SC2731_MODULE_EN		0xc08
>  #define SC27XX_MODULE_ADC_EN		BIT(5)
>  #define SC2721_ARM_CLK_EN		0xc0c
> +#define SC2730_ARM_CLK_EN		0x180c
>  #define SC2731_ARM_CLK_EN		0xc10
>  #define SC27XX_CLK_ADC_EN		BIT(5)
>  #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
> @@ -307,6 +309,80 @@ static int sc2721_adc_get_ratio(int channel, int scale)
>  	return SC27XX_VOLT_RATIO(1, 1);
>  }
>  
> +static int sc2730_adc_get_ratio(int channel, int scale)
> +{
> +	switch (channel) {
> +	case 14:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(68, 900);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(68, 1760);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(68, 2327);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(68, 3654);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 15:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(1, 3);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(1000, 5865);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(500, 3879);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(500, 6090);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 16:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(48, 100);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(480, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(480, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(48, 406);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 21:
> +	case 22:
> +	case 23:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(3, 8);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(375, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(375, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(300, 3248);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	default:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(1000, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(1000, 2586);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(1000, 4060);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	}
> +	return SC27XX_VOLT_RATIO(1, 1);
> +}
> +
>  static int sc2731_adc_get_ratio(int channel, int scale)
>  {
>  	switch (channel) {
> @@ -363,6 +439,22 @@ static void sc2720_adc_scale_init(struct sc27xx_adc_data *data)
>  	}
>  }
>  
> +static void sc2730_adc_scale_init(struct sc27xx_adc_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> +		if (i == 5 || i == 10 || i == 19 || i == 30 || i == 31)

I'd keep this looking like the other instances (in the previous patch)
and use a switch statement.  They are more lines, but easier to read.

> +			data->channel_scale[i] = 3;
> +		else if (i == 7 || i == 9)
> +			data->channel_scale[i] = 2;
> +		else if (i == 13)
> +			data->channel_scale[i] = 1;
> +		else
> +			data->channel_scale[i] = 0;
> +	}
> +}
> +
>  static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
>  {
>  	int i;
> @@ -720,6 +812,18 @@ static const struct sc27xx_adc_variant_data sc2731_data = {
>  	.get_ratio = sc2731_adc_get_ratio,
>  };
>  
> +static const struct sc27xx_adc_variant_data sc2730_data = {
> +	.pmic_type = SC27XX_ADC,
> +	.module_en = SC2730_MODULE_EN,
> +	.clk_en = SC2730_ARM_CLK_EN,
> +	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
> +	.scale_mask = SC27XX_ADC_SCALE_MASK,
> +	.bscale_cal = &big_scale_graph_calib,
> +	.sscale_cal = &small_scale_graph_calib,
> +	.init_scale = sc2730_adc_scale_init,
> +	.get_ratio = sc2730_adc_get_ratio,
> +};
> +
>  static const struct sc27xx_adc_variant_data sc2721_data = {
>  	.pmic_type = SC2721_ADC,
>  	.module_en = SC2731_MODULE_EN,
> @@ -834,6 +938,7 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  
>  static const struct of_device_id sc27xx_adc_of_match[] = {
>  	{ .compatible = "sprd,sc2731-adc", .data = &sc2731_data},
> +	{ .compatible = "sprd,sc2730-adc", .data = &sc2730_data},
>  	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
>  	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
>  	{ }

[PATCH V3 7/7] iio: adc: sc27xx: add support for PMIC ump9620

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

The ump9620 is variant from sc27xx chip, add it in here.

Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 drivers/iio/adc/sc27xx_adc.c | 249 +++++++++++++++++++++++++++++++----
 1 file changed, 226 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 43655a818d09..fe4a45f61ac8 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -15,12 +15,16 @@
 /* PMIC global registers definition */
 #define SC2730_MODULE_EN		0x1808
 #define SC2731_MODULE_EN		0xc08
+#define UMP9620_MODULE_EN		0x2008
 #define SC27XX_MODULE_ADC_EN		BIT(5)
 #define SC2721_ARM_CLK_EN		0xc0c
 #define SC2730_ARM_CLK_EN		0x180c
 #define SC2731_ARM_CLK_EN		0xc10
+#define UMP9620_ARM_CLK_EN		0x200c
+#define UMP9620_XTL_WAIT_CTRL0		0x2378
 #define SC27XX_CLK_ADC_EN		BIT(5)
 #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
+#define UMP9620_XTL_WAIT_CTRL0_EN	BIT(8)
 
 /* ADC controller registers definition */
 #define SC27XX_ADC_CTL			0x0
@@ -82,6 +86,13 @@
 enum sc27xx_pmic_type {
 	SC27XX_ADC,
 	SC2721_ADC,
+	UMP9620_ADC,
+};
+
+enum ump96xx_scale_cal {
+	UMP96XX_VBAT_SENSES_CAL,
+	UMP96XX_VBAT_DET_CAL,
+	UMP96XX_CH1_CAL,
 };
 
 struct sc27xx_adc_data {
@@ -139,6 +150,11 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
 	100, 341,
 };
 
+static struct sc27xx_adc_linear_graph ump9620_bat_det_graph = {
+	1400, 3482,
+	200, 476,
+};
+
 /* Add these for sc2731 pmic, and the [big|small]_scale_graph_calib for common's */
 static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
 	4200, 850,
@@ -221,6 +237,56 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
 	return 0;
 }
 
+static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
+				enum ump96xx_scale_cal cal_type)
+{
+	struct sc27xx_adc_linear_graph *graph;
+	const char *cell_name1, *cell_name2;
+	int adc_calib_data1, adc_calib_data2;
+
+	if (!data)
+		return -EINVAL;
+
+	if (cal_type == UMP96XX_VBAT_DET_CAL) {
+		graph = &ump9620_bat_det_graph;
+		cell_name1 = "vbat_det_cal1";
+		cell_name2 = "vbat_det_cal2";
+	} else if (cal_type == UMP96XX_VBAT_SENSES_CAL) {
+		graph = &big_scale_graph;
+		cell_name1 = "big_scale_calib1";
+		cell_name2 = "big_scale_calib2";
+	} else if (cal_type == UMP96XX_CH1_CAL) {
+		graph = &small_scale_graph;
+		cell_name1 = "small_scale_calib1";
+		cell_name2 = "small_scale_calib2";
+	} else {
+		graph = &small_scale_graph;
+		cell_name1 = "small_scale_calib1";
+		cell_name2 = "small_scale_calib2";
+	}
+
+	adc_calib_data1 = adc_nvmem_cell_calib_data(data, cell_name1);
+	if (adc_calib_data1 < 0) {
+		dev_err(data->dev, "err! %s:%d\n", cell_name1, adc_calib_data1);
+		return adc_calib_data1;
+	}
+
+	adc_calib_data2 = adc_nvmem_cell_calib_data(data, cell_name2);
+	if (adc_calib_data2 < 0) {
+		dev_err(data->dev, "err! %s:%d\n", cell_name2, adc_calib_data2);
+		return adc_calib_data2;
+	}
+
+	/*
+	 *Read the data in the two blocks of efuse and convert them into the
+	 *calibration value in the ump9620 adc linear graph.
+	 */
+	graph->adc0 = (adc_calib_data1 & 0xfff0) >> 4;
+	graph->adc1 = (adc_calib_data2 & 0xfff0) >> 4;
+
+	return 0;
+}
+
 static int sc2720_adc_get_ratio(int channel, int scale)
 {
 	switch (channel) {
@@ -408,6 +474,50 @@ static int sc2731_adc_get_ratio(int channel, int scale)
 	return SC27XX_VOLT_RATIO(1, 1);
 }
 
+static int ump9620_adc_get_ratio(int channel, int scale)
+{
+	switch (channel) {
+	case 11:
+		return SC27XX_VOLT_RATIO(1, 1);
+	case 14:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(68, 900);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 15:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(1, 3);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	case 21:
+	case 22:
+	case 23:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(3, 8);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	default:
+		switch (scale) {
+		case 0:
+			return SC27XX_VOLT_RATIO(1, 1);
+		case 1:
+			return SC27XX_VOLT_RATIO(1000, 1955);
+		case 2:
+			return SC27XX_VOLT_RATIO(1000, 2600);
+		case 3:
+			return SC27XX_VOLT_RATIO(1000, 4060);
+		default:
+			return SC27XX_VOLT_RATIO(1, 1);
+		}
+	}
+}
+
 /*
  * According to the datasheet set specific value on some channel.
  */
@@ -469,6 +579,22 @@ static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
 	}
 }
 
+static void ump9620_adc_scale_init(struct sc27xx_adc_data *data)
+{
+	int i;
+
+	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
+		if (i == 10 || i == 19 || i == 30 || i == 31)
+			data->channel_scale[i] = 3;
+		else if (i == 7 || i == 9)
+			data->channel_scale[i] = 2;
+		else if (i == 0 || i == 13)
+			data->channel_scale[i] = 1;
+		else
+			data->channel_scale[i] = 0;
+	}
+}
+
 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 			   int scale, int *val)
 {
@@ -603,29 +729,67 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
 	return tmp < 0 ? 0 : tmp;
 }
 
+static int ump96xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph, int scale,
+		int raw_adc)
+{
+	int tmp;
+
+	tmp = adc_to_volt(graph, raw_adc);
+
+	if (scale == 2)
+		tmp = tmp * 2600 / 1000;
+	else if (scale == 3)
+		tmp = tmp * 4060 / 1000;
+
+	return tmp < 0 ? 0 : tmp;
+}
+
+
 static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
 				   int scale, int raw_adc)
 {
 	u32 numerator, denominator;
 	u32 volt;
 
-	/*
-	 * Convert ADC values to voltage values according to the linear graph,
-	 * and channel 5 and channel 1 has been calibrated, so we can just
-	 * return the voltage values calculated by the linear graph. But other
-	 * channels need be calculated to the real voltage values with the
-	 * voltage ratio.
-	 */
-	switch (channel) {
-	case 5:
-		return sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
+	if (data->var_data->pmic_type == UMP9620_ADC) {
+		switch (channel) {
+		case 0:
+			if (scale == 1)
+				volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
+			else
+				volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
+			break;
+		case 11:
+			volt = sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
+			break;
+		default:
+			if (scale == 1)
+				volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
+			else
+				volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
+			break;
+		}
+		if (channel == 0 && scale == 1)
+			return volt;
+	} else {
+		/*
+		 * Convert ADC values to voltage values according to the linear graph,
+		 * and channel 5 and channel 1 has been calibrated, so we can just
+		 * return the voltage values calculated by the linear graph. But other
+		 * channels need be calculated to the real voltage values with the
+		 * voltage ratio.
+		 */
+		switch (channel) {
+		case 5:
+			return sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
 
-	case 1:
-		return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
+		case 1:
+			return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
 
-	default:
-		volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
-		break;
+		default:
+			volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
+			break;
+		}
 	}
 
 	sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
@@ -760,21 +924,42 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 	if (ret)
 		return ret;
 
-	/* Enable ADC work clock and controller clock */
+	/* Enable 26MHz crvstal oscillator wait cycles for UMP9620 ADC */
+	if (data->var_data->pmic_type == UMP9620_ADC) {
+		ret = regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
+				UMP9620_XTL_WAIT_CTRL0_EN,
+				UMP9620_XTL_WAIT_CTRL0_EN);
+	}
+
+	/* Enable ADC work clock */
 	ret = regmap_update_bits(data->regmap, data->var_data->clk_en,
 				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN,
 				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
 	if (ret)
 		goto disable_adc;
 
-	/* ADC channel scales' calibration from nvmem device */
-	ret = sc27xx_adc_scale_calibration(data, true);
-	if (ret)
-		goto disable_clk;
+	/* ADC channel scales calibration from nvmem device */
+	if (data->var_data->pmic_type == UMP9620_ADC) {
+		ret = ump96xx_adc_scale_cal(data, UMP96XX_VBAT_SENSES_CAL);
+		if (ret)
+			goto disable_clk;
 
-	ret = sc27xx_adc_scale_calibration(data, false);
-	if (ret)
-		goto disable_clk;
+		ret = ump96xx_adc_scale_cal(data, UMP96XX_VBAT_DET_CAL);
+		if (ret)
+			goto disable_clk;
+
+		ret = ump96xx_adc_scale_cal(data, UMP96XX_CH1_CAL);
+		if (ret)
+			goto disable_clk;
+	} else {
+		ret = sc27xx_adc_scale_calibration(data, true);
+		if (ret)
+			goto disable_clk;
+
+		ret = sc27xx_adc_scale_calibration(data, false);
+		if (ret)
+			goto disable_clk;
+	}
 
 	return 0;
 
@@ -798,6 +983,11 @@ static void sc27xx_adc_disable(void *_data)
 
 	regmap_update_bits(data->regmap, data->var_data->module_en,
 			   SC27XX_MODULE_ADC_EN, 0);
+
+	if (data->var_data->pmic_type == UMP9620_ADC)
+		regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
+				UMP9620_XTL_WAIT_CTRL0_EN, 0);
+
 }
 
 static const struct sc27xx_adc_variant_data sc2731_data = {
@@ -848,6 +1038,18 @@ static const struct sc27xx_adc_variant_data sc2720_data = {
 	.get_ratio = sc2720_adc_get_ratio,
 };
 
+static const struct sc27xx_adc_variant_data ump9620_data = {
+	.pmic_type = UMP9620_ADC,
+	.module_en = UMP9620_MODULE_EN,
+	.clk_en = UMP9620_ARM_CLK_EN,
+	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
+	.scale_mask = SC27XX_ADC_SCALE_MASK,
+	.bscale_cal = &big_scale_graph,
+	.sscale_cal = &small_scale_graph,
+	.init_scale = ump9620_adc_scale_init,
+	.get_ratio = ump9620_adc_get_ratio,
+};
+
 static int sc27xx_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -941,6 +1143,7 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
 	{ .compatible = "sprd,sc2730-adc", .data = &sc2730_data},
 	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
 	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
+	{ .compatible = "sprd,ump9620-adc", .data = &ump9620_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
-- 
2.25.1

Re: [PATCH V3 7/7] iio: adc: sc27xx: add support for PMIC ump9620

[Jonathan Cameron]

On Thu,  7 Apr 2022 16:21:48 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> The ump9620 is variant from sc27xx chip, add it in here.
> 
> Co-developed-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
Hi Cixi, Yuming,

A follow up to the earlier comment on trying to avoid using
a part enum when we have a structure to hold part specific data and
can describe better what the feature is / use a callback to avoid
having to handle it in the same code as the case where the feature
isn't present.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/sc27xx_adc.c | 249 +++++++++++++++++++++++++++++++----
>  1 file changed, 226 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 43655a818d09..fe4a45f61ac8 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -15,12 +15,16 @@
>  /* PMIC global registers definition */
>  #define SC2730_MODULE_EN		0x1808
>  #define SC2731_MODULE_EN		0xc08
> +#define UMP9620_MODULE_EN		0x2008
>  #define SC27XX_MODULE_ADC_EN		BIT(5)
>  #define SC2721_ARM_CLK_EN		0xc0c
>  #define SC2730_ARM_CLK_EN		0x180c
>  #define SC2731_ARM_CLK_EN		0xc10
> +#define UMP9620_ARM_CLK_EN		0x200c
> +#define UMP9620_XTL_WAIT_CTRL0		0x2378
>  #define SC27XX_CLK_ADC_EN		BIT(5)
>  #define SC27XX_CLK_ADC_CLK_EN		BIT(6)
> +#define UMP9620_XTL_WAIT_CTRL0_EN	BIT(8)
>  
>  /* ADC controller registers definition */
>  #define SC27XX_ADC_CTL			0x0
> @@ -82,6 +86,13 @@
>  enum sc27xx_pmic_type {
>  	SC27XX_ADC,
>  	SC2721_ADC,
> +	UMP9620_ADC,
> +};
> +
> +enum ump96xx_scale_cal {
> +	UMP96XX_VBAT_SENSES_CAL,
> +	UMP96XX_VBAT_DET_CAL,
> +	UMP96XX_CH1_CAL,
>  };
>  
>  struct sc27xx_adc_data {
> @@ -139,6 +150,11 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
>  	100, 341,
>  };
>  
> +static struct sc27xx_adc_linear_graph ump9620_bat_det_graph = {
> +	1400, 3482,
> +	200, 476,
> +};
> +
>  /* Add these for sc2731 pmic, and the [big|small]_scale_graph_calib for common's */
>  static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
>  	4200, 850,
> @@ -221,6 +237,56 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
>  	return 0;
>  }
>  
> +static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
> +				enum ump96xx_scale_cal cal_type)
> +{
> +	struct sc27xx_adc_linear_graph *graph;
> +	const char *cell_name1, *cell_name2;
> +	int adc_calib_data1, adc_calib_data2;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	if (cal_type == UMP96XX_VBAT_DET_CAL) {
> +		graph = &ump9620_bat_det_graph;
> +		cell_name1 = "vbat_det_cal1";
> +		cell_name2 = "vbat_det_cal2";
> +	} else if (cal_type == UMP96XX_VBAT_SENSES_CAL) {
> +		graph = &big_scale_graph;
> +		cell_name1 = "big_scale_calib1";
> +		cell_name2 = "big_scale_calib2";
> +	} else if (cal_type == UMP96XX_CH1_CAL) {
> +		graph = &small_scale_graph;
> +		cell_name1 = "small_scale_calib1";
> +		cell_name2 = "small_scale_calib2";
> +	} else {
> +		graph = &small_scale_graph;
> +		cell_name1 = "small_scale_calib1";
> +		cell_name2 = "small_scale_calib2";
> +	}
> +
> +	adc_calib_data1 = adc_nvmem_cell_calib_data(data, cell_name1);
> +	if (adc_calib_data1 < 0) {
> +		dev_err(data->dev, "err! %s:%d\n", cell_name1, adc_calib_data1);
> +		return adc_calib_data1;
> +	}
> +
> +	adc_calib_data2 = adc_nvmem_cell_calib_data(data, cell_name2);
> +	if (adc_calib_data2 < 0) {
> +		dev_err(data->dev, "err! %s:%d\n", cell_name2, adc_calib_data2);
> +		return adc_calib_data2;
> +	}
> +
> +	/*
> +	 *Read the data in the two blocks of efuse and convert them into the
> +	 *calibration value in the ump9620 adc linear graph.
> +	 */
> +	graph->adc0 = (adc_calib_data1 & 0xfff0) >> 4;
> +	graph->adc1 = (adc_calib_data2 & 0xfff0) >> 4;
> +
> +	return 0;
> +}
> +
>  static int sc2720_adc_get_ratio(int channel, int scale)
>  {
>  	switch (channel) {
> @@ -408,6 +474,50 @@ static int sc2731_adc_get_ratio(int channel, int scale)
>  	return SC27XX_VOLT_RATIO(1, 1);
>  }
>  
> +static int ump9620_adc_get_ratio(int channel, int scale)
> +{
> +	switch (channel) {
> +	case 11:
> +		return SC27XX_VOLT_RATIO(1, 1);
> +	case 14:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(68, 900);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 15:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(1, 3);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	case 21:
> +	case 22:
> +	case 23:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(3, 8);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	default:
> +		switch (scale) {
> +		case 0:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		case 1:
> +			return SC27XX_VOLT_RATIO(1000, 1955);
> +		case 2:
> +			return SC27XX_VOLT_RATIO(1000, 2600);
> +		case 3:
> +			return SC27XX_VOLT_RATIO(1000, 4060);
> +		default:
> +			return SC27XX_VOLT_RATIO(1, 1);
> +		}
> +	}
> +}
> +
>  /*
>   * According to the datasheet set specific value on some channel.
>   */
> @@ -469,6 +579,22 @@ static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
>  	}
>  }
>  
> +static void ump9620_adc_scale_init(struct sc27xx_adc_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {

As for previous patch, a switch statement here would be easier to read.

> +		if (i == 10 || i == 19 || i == 30 || i == 31)
> +			data->channel_scale[i] = 3;
> +		else if (i == 7 || i == 9)
> +			data->channel_scale[i] = 2;
> +		else if (i == 0 || i == 13)
> +			data->channel_scale[i] = 1;
> +		else
> +			data->channel_scale[i] = 0;
> +	}
> +}
> +
>  static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  			   int scale, int *val)
>  {
> @@ -603,29 +729,67 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
>  	return tmp < 0 ? 0 : tmp;
>  }
>  
> +static int ump96xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph, int scale,
> +		int raw_adc)
> +{
> +	int tmp;
> +
> +	tmp = adc_to_volt(graph, raw_adc);
> +
> +	if (scale == 2)
> +		tmp = tmp * 2600 / 1000;
> +	else if (scale == 3)
> +		tmp = tmp * 4060 / 1000;
> +
> +	return tmp < 0 ? 0 : tmp;
> +}
> +
> +
>  static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
>  				   int scale, int raw_adc)
>  {
>  	u32 numerator, denominator;
>  	u32 volt;
>  
> -	/*
> -	 * Convert ADC values to voltage values according to the linear graph,
> -	 * and channel 5 and channel 1 has been calibrated, so we can just
> -	 * return the voltage values calculated by the linear graph. But other
> -	 * channels need be calculated to the real voltage values with the
> -	 * voltage ratio.
> -	 */
> -	switch (channel) {
> -	case 5:
> -		return sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
> +	if (data->var_data->pmic_type == UMP9620_ADC) {

Looks like a case for a callback to me so we end up without the complexity of
handling multiple types via an if / else.

> +		switch (channel) {
> +		case 0:
> +			if (scale == 1)
> +				volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
> +			else
> +				volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
> +			break;
> +		case 11:
> +			volt = sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
> +			break;
> +		default:
> +			if (scale == 1)
> +				volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
> +			else
> +				volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
> +			break;
> +		}
> +		if (channel == 0 && scale == 1)
> +			return volt;
> +	} else {
> +		/*
> +		 * Convert ADC values to voltage values according to the linear graph,
> +		 * and channel 5 and channel 1 has been calibrated, so we can just
> +		 * return the voltage values calculated by the linear graph. But other
> +		 * channels need be calculated to the real voltage values with the
> +		 * voltage ratio.
> +		 */
> +		switch (channel) {
> +		case 5:
> +			return sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
>  
> -	case 1:
> -		return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> +		case 1:
> +			return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
>  
> -	default:
> -		volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> -		break;
> +		default:
> +			volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> +			break;
> +		}
>  	}
>  
>  	sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
> @@ -760,21 +924,42 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  	if (ret)
>  		return ret;
>  
> -	/* Enable ADC work clock and controller clock */
> +	/* Enable 26MHz crvstal oscillator wait cycles for UMP9620 ADC */
> +	if (data->var_data->pmic_type == UMP9620_ADC) {
> +		ret = regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> +				UMP9620_XTL_WAIT_CTRL0_EN,
> +				UMP9620_XTL_WAIT_CTRL0_EN);
> +	}
> +
> +	/* Enable ADC work clock */
>  	ret = regmap_update_bits(data->regmap, data->var_data->clk_en,
>  				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN,
>  				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
>  	if (ret)
>  		goto disable_adc;
>  
> -	/* ADC channel scales' calibration from nvmem device */
> -	ret = sc27xx_adc_scale_calibration(data, true);
> -	if (ret)
> -		goto disable_clk;
> +	/* ADC channel scales calibration from nvmem device */
> +	if (data->var_data->pmic_type == UMP9620_ADC) {

As below.  If we need to use a callback for this that is fine too, but
I don't want to see matching on pmic_type as that tends to just make
the code harder to read as we'll gain more and more cases over time.

> +		ret = ump96xx_adc_scale_cal(data, UMP96XX_VBAT_SENSES_CAL);
> +		if (ret)
> +			goto disable_clk;
>  
> -	ret = sc27xx_adc_scale_calibration(data, false);
> -	if (ret)
> -		goto disable_clk;
> +		ret = ump96xx_adc_scale_cal(data, UMP96XX_VBAT_DET_CAL);
> +		if (ret)
> +			goto disable_clk;
> +
> +		ret = ump96xx_adc_scale_cal(data, UMP96XX_CH1_CAL);
> +		if (ret)
> +			goto disable_clk;
> +	} else {
> +		ret = sc27xx_adc_scale_calibration(data, true);
> +		if (ret)
> +			goto disable_clk;
> +
> +		ret = sc27xx_adc_scale_calibration(data, false);
> +		if (ret)
> +			goto disable_clk;
> +	}
>  
>  	return 0;
>  
> @@ -798,6 +983,11 @@ static void sc27xx_adc_disable(void *_data)
>  
>  	regmap_update_bits(data->regmap, data->var_data->module_en,
>  			   SC27XX_MODULE_ADC_EN, 0);
> +
> +	if (data->var_data->pmic_type == UMP9620_ADC)

As with earlier vref bool suggestion, I'd like to see something
descriptive added to the variant_data structure (only currently set for
this device type) rather than using an enum value.
Features descriptions always end up scaling better to lots of
supported parts than matching on particular part IDs.

> +		regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> +				UMP9620_XTL_WAIT_CTRL0_EN, 0);
> +
>  }
>  
>  static const struct sc27xx_adc_variant_data sc2731_data = {
> @@ -848,6 +1038,18 @@ static const struct sc27xx_adc_variant_data sc2720_data = {
>  	.get_ratio = sc2720_adc_get_ratio,
>  };
>  
> +static const struct sc27xx_adc_variant_data ump9620_data = {
> +	.pmic_type = UMP9620_ADC,
> +	.module_en = UMP9620_MODULE_EN,
> +	.clk_en = UMP9620_ARM_CLK_EN,
> +	.scale_shift = SC27XX_ADC_SCALE_SHIFT,
> +	.scale_mask = SC27XX_ADC_SCALE_MASK,
> +	.bscale_cal = &big_scale_graph,
> +	.sscale_cal = &small_scale_graph,
> +	.init_scale = ump9620_adc_scale_init,
> +	.get_ratio = ump9620_adc_get_ratio,
> +};
> +
>  static int sc27xx_adc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -941,6 +1143,7 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
>  	{ .compatible = "sprd,sc2730-adc", .data = &sc2730_data},
>  	{ .compatible = "sprd,sc2721-adc", .data = &sc2721_data},
>  	{ .compatible = "sprd,sc2720-adc", .data = &sc2720_data},
> +	{ .compatible = "sprd,ump9620-adc", .data = &ump9620_data},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);