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

[PATCH V2 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.
also add ump9620 PMIC suspend and resume pm implement.

Cixi Geng (7):
  dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings
  iio: adc: sc27xx: fix read big scale voltage not right
  iio: adc: sc27xx: structure adjuststment and optimization
  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
  iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support

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.

 .../bindings/iio/adc/sprd,sc2720-adc.yaml     |  30 +-
 drivers/iio/adc/sc27xx_adc.c                  | 797 ++++++++++++++++--
 2 files changed, 764 insertions(+), 63 deletions(-)

-- 
2.25.1

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

[Cixi Geng]

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

sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
dtbindings.

Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
 .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
 1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
@@ -37,9 +38,32 @@ properties:
     maxItems: 2
 
   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-cell-names:
+      items:
+        - const: big_scale_calib
+        - const: small_scale_calib
+
+else:
+  properties:
+    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
-- 
2.25.1

[PATCH V2 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: Yuming Zhu <yuming.zhu1@unisoc.com>
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

[PATCH V2 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.

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

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index aee076c8e2b1..68629fbcfec5 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,6 +120,17 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
 	100, 341,
 };
 
+/* 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 sc2731_small_scale_graph_calib = {
+	1000, 838,
+	100, 84,
+};
+
 static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
 	4200, 856,
 	3600, 733,
@@ -130,11 +158,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 +188,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 +213,21 @@ 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;
+
+	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 +251,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 +306,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 +477,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 +501,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 +515,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 +583,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 +611,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

[PATCH V2 4/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.

Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

v2 changes:

1. modify code by the baolin's comment

Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>

2.fix smatch warnings in sc27xx_adc_read()

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/iio/adc/sc27xx_adc.c | 201 ++++++++++++++++++++++++++++++++++-
 1 file changed, 200 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 68629fbcfec5..2603ce313b07 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;
@@ -188,6 +205,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) {
@@ -216,6 +321,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;
@@ -231,7 +364,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);
@@ -240,6 +373,22 @@ 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");
+			hwspin_unlock_raw(data->hwlock);
+			return ret;
+		}
+	}
+
 	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
 				 SC27XX_ADC_EN, SC27XX_ADC_EN);
 	if (ret)
@@ -294,6 +443,18 @@ 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);
 unlock_adc:
+	if (data->var_data->pmic_type == SC2721_ADC) {
+		if ((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;
+			}
+		}
+	}
+
 	hwspin_unlock_raw(data->hwlock);
 
 	if (!ret)
@@ -523,6 +684,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,
@@ -533,6 +695,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;
@@ -583,6 +769,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_optional(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);
 
@@ -612,6 +809,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

[PATCH V2 5/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.

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

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 2603ce313b07..b89637c051ac 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)
@@ -293,6 +295,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) {
@@ -349,6 +425,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;
@@ -449,7 +541,8 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 						    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");
+				dev_err(data->dev, "failed to set the volref 2.8V, ret_volref = 0x%x\n",
+					ret_volref);
 				ret = ret || ret_volref;
 			}
 		}
@@ -695,6 +788,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,
@@ -809,6 +914,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

[PATCH V2 6/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.

Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

V2 changes:
1. remove duplicated function
Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>

2. fix the smatch warnings
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/iio/adc/sc27xx_adc.c | 305 ++++++++++++++++++++++++++++++-----
 1 file changed, 266 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index b89637c051ac..e9b680e9c275 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,
@@ -165,16 +181,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 = 0;
+
+	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;
@@ -186,24 +227,63 @@ 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;
+}
+
+static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
+					enum ump96xx_scale_cal cal_type)
+{
+	struct sc27xx_adc_linear_graph *graph = NULL;
+	const char *cell_name1 = NULL, *cell_name2 = NULL;
+	int adc_calib_data1 = 0, adc_calib_data2 = 0;
+
+	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;
 }
 
@@ -394,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.
  */
@@ -453,6 +577,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)
 {
@@ -567,7 +707,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;
@@ -576,6 +716,31 @@ 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;
+}
+
+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;
 }
 
@@ -585,23 +750,46 @@ static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
 	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;
+		}
 
-	case 1:
-		return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
+		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);
 
-	default:
-		volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
-		break;
+		case 1:
+			return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
+
+		default:
+			volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
+			break;
+		}
 	}
 
 	sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
@@ -619,6 +807,7 @@ static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
 		return ret;
 
 	*val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
+
 	return 0;
 }
 
@@ -736,21 +925,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;
 
@@ -774,6 +984,10 @@ 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 = {
@@ -824,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;
@@ -917,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

[PATCH V2 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support

[Cixi Geng]

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

Ump9620 ADC suspend and resume pm optimization, configuration
0x6490_ 0350(PAD_ CLK26M_ SINOUT_ PMIC_ 1P8 ) bit 8.

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

diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index e9b680e9c275..b038b1dfc91f 100644
--- a/drivers/iio/adc/sc27xx_adc.c
+++ b/drivers/iio/adc/sc27xx_adc.c
@@ -11,6 +11,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 /* PMIC global registers definition */
 #define SC2730_MODULE_EN		0x1808
@@ -83,6 +84,9 @@
 /* ADC default channel reference voltage is 2.8V */
 #define SC27XX_ADC_REFVOL_VDD28		2800000
 
+/* 10s delay before suspending the ADC IP */
+#define SC27XX_ADC_AUTOSUSPEND_DELAY	10000
+
 enum sc27xx_pmic_type {
 	SC27XX_ADC,
 	SC2721_ADC,
@@ -96,6 +100,7 @@ enum ump96xx_scale_cal {
 };
 
 struct sc27xx_adc_data {
+	struct iio_dev *indio_dev;
 	struct device *dev;
 	struct regulator *volref;
 	struct regmap *regmap;
@@ -605,6 +610,9 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 		return ret;
 	}
 
+	if (data->var_data->pmic_type == UMP9620_ADC)
+		pm_runtime_get_sync(data->indio_dev->dev.parent);
+
 	/*
 	 * 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
@@ -688,6 +696,11 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
 		}
 	}
 
+	if (data->var_data->pmic_type == UMP9620_ADC) {
+		pm_runtime_mark_last_busy(data->indio_dev->dev.parent);
+		pm_runtime_put_autosuspend(data->indio_dev->dev.parent);
+	}
+
 	hwspin_unlock_raw(data->hwlock);
 
 	if (!ret)
@@ -927,9 +940,11 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 
 	/* 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);
+		pm_runtime_get(data->dev);
+		if (ret) {
+			dev_err(data->dev, "failed to set the UMP9620 ADC clk26m bit8 on IP\n");
+			goto clean_adc_clk26m_bit8;
+		}
 	}
 
 	/* Enable ADC work clock */
@@ -971,6 +986,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
 	regmap_update_bits(data->regmap, data->var_data->module_en,
 			   SC27XX_MODULE_ADC_EN, 0);
 
+clean_adc_clk26m_bit8:
+	if (data->var_data->pmic_type == UMP9620_ADC)
+		pm_runtime_put(data->dev);
+
 	return ret;
 }
 
@@ -1069,6 +1088,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 	if (!indio_dev)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, indio_dev);
+
 	sc27xx_data = iio_priv(indio_dev);
 
 	sc27xx_data->regmap = dev_get_regmap(dev->parent, NULL);
@@ -1111,7 +1132,10 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 		}
 	}
 
+	sc27xx_data->dev = dev;
 	sc27xx_data->var_data = pdata;
+	sc27xx_data->indio_dev = indio_dev;
+
 	sc27xx_data->var_data->init_scale(sc27xx_data);
 
 	ret = sc27xx_adc_enable(sc27xx_data);
@@ -1126,14 +1150,35 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	indio_dev->dev.parent = dev;
 	indio_dev->name = dev_name(dev);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &sc27xx_info;
 	indio_dev->channels = sc27xx_channels;
 	indio_dev->num_channels = ARRAY_SIZE(sc27xx_channels);
+
+	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
+		pm_runtime_set_autosuspend_delay(dev,
+						 SC27XX_ADC_AUTOSUSPEND_DELAY);
+		pm_runtime_use_autosuspend(dev);
+		pm_runtime_set_suspended(dev);
+		pm_runtime_enable(dev);
+		pm_runtime_get(dev);
+	}
+
 	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "could not register iio (ADC)");
+		goto err_iio_register;
+	}
+
+	return 0;
+
+err_iio_register:
+	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
+		pm_runtime_put(dev);
+		pm_runtime_disable(dev);
+	}
 
 	return ret;
 }
@@ -1148,11 +1193,46 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
 
+static int sc27xx_adc_runtime_suspend(struct device *dev)
+{
+	struct sc27xx_adc_data *sc27xx_data = iio_priv(dev_get_drvdata(dev));
+
+	/* clean the UMP9620 ADC clk26m bit8 on IP */
+	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC)
+		regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
+				UMP9620_XTL_WAIT_CTRL0_EN, 0);
+
+	return 0;
+}
+
+static int sc27xx_adc_runtime_resume(struct device *dev)
+{
+	int ret = 0;
+	struct sc27xx_adc_data *sc27xx_data = iio_priv(dev_get_drvdata(dev));
+
+	/* set the UMP9620 ADC clk26m bit8 on IP */
+	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
+		ret = regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
+				UMP9620_XTL_WAIT_CTRL0_EN, UMP9620_XTL_WAIT_CTRL0_EN);
+		if (ret) {
+			dev_err(dev, "failed to set the UMP9620 ADC clk26m bit8 on IP\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops sc27xx_adc_pm_ops = {
+	SET_RUNTIME_PM_OPS(sc27xx_adc_runtime_suspend, sc27xx_adc_runtime_resume, NULL)
+};
+
 static struct platform_driver sc27xx_adc_driver = {
 	.probe = sc27xx_adc_probe,
 	.driver = {
 		.name = "sc27xx-adc",
 		.of_match_table = sc27xx_adc_of_match,
+		.pm	= &sc27xx_adc_pm_ops,
 	},
 };
 
-- 
2.25.1

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

[Jonathan Cameron]

On Sat, 12 Mar 2022 00:46:22 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

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

In title, use dt-binding instead of dtbindings at the end.

> 
> sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
> dtbindings.

dt-bindings.

> 
> Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

If the patch was authored by Chunyan Zhang, it should be From: Chunyan Zhang,
if it is more complex than you simply passing the patch on then we should
see a co-developed to indicate that.

Patch looks good to me

Jonathan

> ---
>  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
>  1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
> @@ -37,9 +38,32 @@ properties:
>      maxItems: 2
>  
>    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-cell-names:
> +      items:
> +        - const: big_scale_calib
> +        - const: small_scale_calib
> +
> +else:
> +  properties:
> +    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

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

[Jonathan Cameron]

On Sat, 12 Mar 2022 00:46:23 +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: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> 
No blank lines in a tag block (they break people's scripts)
Also, if this is a fix, I'd expect a fixes tag.

> 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

This driver would benefit from use of FIELD_GET() / FIELD_PREP()
but that is obviously unrelated to this particular series.


Jonathan

>  
>  /* Bits definitions for SC27XX_ADC_INT_EN registers */
>  #define SC27XX_ADC_IRQ_EN		BIT(0)

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

[Jonathan Cameron]

On Sat, 12 Mar 2022 00:46:25 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

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

For IIO change log should be under the ---
as I don't want to see it in the final git log.

> 
> v2 changes:
> 
> 1. modify code by the baolin's comment
> 
> Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
Did Boalin give an RB?  I'm not seeing it from a quick look
at the v1 patch.  You should not add Reviewed-by or
Acked-by unless explicitly given by the person in question

> 
> 2.fix smatch warnings in sc27xx_adc_read()
> 

Again, blank lines in tag block.
If you want to acknowledge the reports which is great, then you need
to do something like.

Reported-by: kernel test robot <lpk@intel.com> #smatch warnings
Reported-by: Dan Carpenter <dan.carpenter@oracle.com> #as above.

So we stick to one line tags with no gaps, but with a comment after them
to provide context.  It is also common to acknowledge these sort of
changes in the change description below the --- only and not in the
tag block.  Either is acceptable in IIO but other maintainers may
disagree in their areas of the kernel.

Comments inline,

Thanks,

Jonathan


> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 201 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 200 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 68629fbcfec5..2603ce313b07 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;
> @@ -188,6 +205,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) {
> @@ -216,6 +321,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;
> @@ -231,7 +364,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);
> @@ -240,6 +373,22 @@ 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");
> +			hwspin_unlock_raw(data->hwlock);
> +			return ret;
> +		}
> +	}
> +
>  	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>  				 SC27XX_ADC_EN, SC27XX_ADC_EN);
>  	if (ret)
> @@ -294,6 +443,18 @@ 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);
>  unlock_adc:
> +	if (data->var_data->pmic_type == SC2721_ADC) {
> +		if ((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;
> +			}
> +		}
> +	}
> +
>  	hwspin_unlock_raw(data->hwlock);
>  
>  	if (!ret)
> @@ -523,6 +684,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,
> @@ -533,6 +695,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;
> @@ -583,6 +769,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_optional(dev, "vref");

What you are doing here is using get_optional to avoid a stub regulator, rather than
because the regulator is in any sense optional.

Perhaps a better approach would be do a normal get() and then check the supplied
regulator is capable of setting the voltage to the two levels required?

> +		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);
>  
> @@ -612,6 +809,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);

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

[Jonathan Cameron]

On Sat, 12 Mar 2022 00:46:27 +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.
> 
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
Hi,

Same comments as made earlier apply here.
* Should there be a co-developed-by marking here or should
the from reflect Yuming Zhu as the patch author with you in
the role of being on the path to upstream? 
* change log below the ---
* No blank lines or non tag lines in the tag block - use
comments at the end of the lines if you want to give
reported-by for particularly things. 

Also, I'm not immediately spotting where Baolin gave
a tag.

Comments inline.

There are a few places in here where you have significant deviation between
the code that runs for this new device and previously supported parts.
Doing that via if / else tends not to scale as yet more parts are added in
future. Perhaps it would be better to move to some device type specific
callbacks for more of these cases.  This would be similar to the init_scale
and get_ratio function pointers you already have in the variant data.

Thanks,

Jonathan

> 
> V2 changes:
> 1. remove duplicated function
> Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
> 
> 2. fix the smatch warnings
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 305 ++++++++++++++++++++++++++++++-----
>  1 file changed, 266 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index b89637c051ac..e9b680e9c275 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,
> @@ -165,16 +181,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)
> +{

This looks to be a bit of refactoring that could be sensibly pulled out before this
patch.

> +	struct nvmem_cell *cell;
> +	void *buf;
> +	u32 origin_calib_data = 0;

Why initialise the above?   I don't see it being used
in any paths where it isn't already initialised.

> +	size_t len = 0;
> +
> +	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;
> @@ -186,24 +227,63 @@ 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;
> +}
> +
> +static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
> +					enum ump96xx_scale_cal cal_type)
> +{
> +	struct sc27xx_adc_linear_graph *graph = NULL;

Always set below, so don't initialize here. Same for other
local variables.

> +	const char *cell_name1 = NULL, *cell_name2 = NULL;
> +	int adc_calib_data1 = 0, adc_calib_data2 = 0;
> +
> +	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;
>  }
>  
> @@ -394,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.
>   */
> @@ -453,6 +577,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)
>  {
> @@ -567,7 +707,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;
> @@ -576,6 +716,31 @@ 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;
> +}
> +
> +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;
>  }
>  
> @@ -585,23 +750,46 @@ static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
>  	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;
> +		}
>  
> -	case 1:
> -		return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> +		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);
>  
> -	default:
> -		volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> -		break;
> +		case 1:
> +			return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> +
> +		default:
> +			volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> +			break;
> +		}
>  	}
>  
>  	sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
> @@ -619,6 +807,7 @@ static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
>  		return ret;
>  
>  	*val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
> +
>  	return 0;
>  }
>  
> @@ -736,21 +925,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;
>  
> @@ -774,6 +984,10 @@ 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 = {
> @@ -824,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;
> @@ -917,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);

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

[Krzysztof Kozlowski]

On 11/03/2022 17:46, Cixi Geng wrote:
> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
> dtbindings.
> 
> Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
>  1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
> @@ -37,9 +38,32 @@ properties:
>      maxItems: 2
>  
>    nvmem-cell-names:
> -    items:
> -      - const: big_scale_calib
> -      - const: small_scale_calib

Please test your changes with dt_binding_check and dtbs_check. Your
change looks not complete - you have still nvmem-cells = 2.


Best regards,
Krzysztof

Re: [PATCH V2 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support

[Jonathan Cameron]

On Sat, 12 Mar 2022 00:46:28 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> Ump9620 ADC suspend and resume pm optimization, configuration
> 0x6490_ 0350(PAD_ CLK26M_ SINOUT_ PMIC_ 1P8 ) bit 8.
> 
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 88 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 84 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index e9b680e9c275..b038b1dfc91f 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -11,6 +11,7 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  
>  /* PMIC global registers definition */
>  #define SC2730_MODULE_EN		0x1808
> @@ -83,6 +84,9 @@
>  /* ADC default channel reference voltage is 2.8V */
>  #define SC27XX_ADC_REFVOL_VDD28		2800000
>  
> +/* 10s delay before suspending the ADC IP */
> +#define SC27XX_ADC_AUTOSUSPEND_DELAY	10000
> +
>  enum sc27xx_pmic_type {
>  	SC27XX_ADC,
>  	SC2721_ADC,
> @@ -96,6 +100,7 @@ enum ump96xx_scale_cal {
>  };
>  
>  struct sc27xx_adc_data {
> +	struct iio_dev *indio_dev;
>  	struct device *dev;
>  	struct regulator *volref;
>  	struct regmap *regmap;
> @@ -605,6 +610,9 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  		return ret;
>  	}
>  
> +	if (data->var_data->pmic_type == UMP9620_ADC)
> +		pm_runtime_get_sync(data->indio_dev->dev.parent);

If you need the indio_dev in here then pass it in to the function. 
Don't introduce a hack / loop of references like this.

> +
>  	/*
>  	 * 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
> @@ -688,6 +696,11 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>  		}
>  	}
>  
> +	if (data->var_data->pmic_type == UMP9620_ADC) {
> +		pm_runtime_mark_last_busy(data->indio_dev->dev.parent);
> +		pm_runtime_put_autosuspend(data->indio_dev->dev.parent);
> +	}
> +
>  	hwspin_unlock_raw(data->hwlock);
>  
>  	if (!ret)
> @@ -927,9 +940,11 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  
>  	/* 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);
> +		pm_runtime_get(data->dev);

Why the queued version?  Is there a race condition, or do we not actually need it
turned on at all at this point?

> +		if (ret) {
> +			dev_err(data->dev, "failed to set the UMP9620 ADC clk26m bit8 on IP\n");

This level of detail may make sense in the implementation of the runtime pm, but it doesn't
make sense here where that detail isn't available.

> +			goto clean_adc_clk26m_bit8;

As such I'd also rename this label.  It's just runtime_get failed - we shouldn't care about
the details here.

> +		}
>  	}
>  
>  	/* Enable ADC work clock */
> @@ -971,6 +986,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>  	regmap_update_bits(data->regmap, data->var_data->module_en,
>  			   SC27XX_MODULE_ADC_EN, 0);
>  
> +clean_adc_clk26m_bit8:
> +	if (data->var_data->pmic_type == UMP9620_ADC)
> +		pm_runtime_put(data->dev);
> +
>  	return ret;
>  }
>  
> @@ -1069,6 +1088,8 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> +	platform_set_drvdata(pdev, indio_dev);
> +
>  	sc27xx_data = iio_priv(indio_dev);
>  
>  	sc27xx_data->regmap = dev_get_regmap(dev->parent, NULL);
> @@ -1111,7 +1132,10 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	sc27xx_data->dev = dev;
>  	sc27xx_data->var_data = pdata;
> +	sc27xx_data->indio_dev = indio_dev;
Why?  Any time I see the iio_dev pointed to by the iio_priv() data it
rings alarm bells.  There should be no situation where you need to
do this.

> +
>  	sc27xx_data->var_data->init_scale(sc27xx_data);
>  
>  	ret = sc27xx_adc_enable(sc27xx_data);
> @@ -1126,14 +1150,35 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	indio_dev->dev.parent = dev;
>  	indio_dev->name = dev_name(dev);
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &sc27xx_info;
>  	indio_dev->channels = sc27xx_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(sc27xx_channels);
> +
> +	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
> +		pm_runtime_set_autosuspend_delay(dev,
> +						 SC27XX_ADC_AUTOSUSPEND_DELAY);
> +		pm_runtime_use_autosuspend(dev);
> +		pm_runtime_set_suspended(dev);
> +		pm_runtime_enable(dev);
> +		pm_runtime_get(dev);

Why does turning it on here make sense?  If it is already on you can
set the state with pm_runtime_set_active().

> +	}
> +
>  	ret = devm_iio_device_register(dev, indio_dev);
> -	if (ret)
> +	if (ret) {
>  		dev_err(dev, "could not register iio (ADC)");
> +		goto err_iio_register;
> +	}
> +
> +	return 0;
> +
> +err_iio_register:
> +	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
> +		pm_runtime_put(dev);
> +		pm_runtime_disable(dev);
> +	}
>  
>  	return ret;
>  }
> @@ -1148,11 +1193,46 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
>  
> +static int sc27xx_adc_runtime_suspend(struct device *dev)
> +{
> +	struct sc27xx_adc_data *sc27xx_data = iio_priv(dev_get_drvdata(dev));
> +
> +	/* clean the UMP9620 ADC clk26m bit8 on IP */
> +	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC)
> +		regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
> +				UMP9620_XTL_WAIT_CTRL0_EN, 0);
> +
> +	return 0;
> +}
> +
> +static int sc27xx_adc_runtime_resume(struct device *dev)
> +{
> +	int ret = 0;

Value not used, so don't set it.

> +	struct sc27xx_adc_data *sc27xx_data = iio_priv(dev_get_drvdata(dev));
> +
> +	/* set the UMP9620 ADC clk26m bit8 on IP */
> +	if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
> +		ret = regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
> +				UMP9620_XTL_WAIT_CTRL0_EN, UMP9620_XTL_WAIT_CTRL0_EN);
> +		if (ret) {
> +			dev_err(dev, "failed to set the UMP9620 ADC clk26m bit8 on IP\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops sc27xx_adc_pm_ops = {
> +	SET_RUNTIME_PM_OPS(sc27xx_adc_runtime_suspend, sc27xx_adc_runtime_resume, NULL)

I'm curious, could you safely use DEFINE_RUNTIME_DEV_PM_OPS()
which would also supply sleep functions via pm_runtime_force_suspend.
Seems likely to work 'fine' though may not be the best possible implementation
of suspend and resume ops.

Even if not, for new code this should be using the new macro 
RUNTIME_PM_OPS() in conjunction with pm_ptr() below.

> +};
> +
>  static struct platform_driver sc27xx_adc_driver = {
>  	.probe = sc27xx_adc_probe,
>  	.driver = {
>  		.name = "sc27xx-adc",
>  		.of_match_table = sc27xx_adc_of_match,
> +		.pm	= &sc27xx_adc_pm_ops,
pm_ptr()

>  	},
>  };
>  

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

[Baolin Wang]

On Sat, Mar 12, 2022 at 12:47 AM Cixi Geng <gengcixi@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 95 ++++++++++++++++++++++++++++++------
>  1 file changed, 80 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index aee076c8e2b1..68629fbcfec5 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,6 +120,17 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
>         100, 341,
>  };
>
> +/* 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 sc2731_small_scale_graph_calib = {
> +       1000, 838,
> +       100, 84,
> +};
> +
>  static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
>         4200, 856,
>         3600, 733,
> @@ -130,11 +158,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;

Now which function will use big_scale_graph_calib and
small_scale_graph_calib?  Seems you add an unused warning?

>                 graph = &small_scale_graph;
>                 cell_name = "small_scale_calib";
>         }
> @@ -160,7 +188,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 +213,21 @@ 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;
> +
> +       for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> +               if (i == 5)
> +                       data->channel_scale[i] = 1;

Can you add some comments to explain why channel 5 needs to set scale to 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 +251,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 +306,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 +477,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 +501,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 +515,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 +583,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 +611,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
>


-- 
Baolin Wang

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

[Baolin Wang]

On Sat, Mar 12, 2022 at 12:47 AM Cixi Geng <gengcixi@gmail.com> wrote:
>
> From: Cixi Geng <cixi.geng1@unisoc.com>
>
> sc2720 and sc2721 is the product of sc27xx series.
>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
>
> v2 changes:
>
> 1. modify code by the baolin's comment
>
> Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>

I think I did not add a reviewed-by tag for this patch before, please drop it.

>
> 2.fix smatch warnings in sc27xx_adc_read()
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 201 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 200 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 68629fbcfec5..2603ce313b07 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;
> @@ -188,6 +205,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) {
> @@ -216,6 +321,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;
> @@ -231,7 +364,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);
> @@ -240,6 +373,22 @@ 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");
> +                       hwspin_unlock_raw(data->hwlock);

Please remove duplicate code, just "goto unlock_adc". Then add a new
label like "regultor_restore" to restore the regulator in the error
path.

> +                       return ret;
> +               }
> +       }
> +
>         ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
>                                  SC27XX_ADC_EN, SC27XX_ADC_EN);
>         if (ret)
> @@ -294,6 +443,18 @@ 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);
>  unlock_adc:
> +       if (data->var_data->pmic_type == SC2721_ADC) {
> +               if ((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;
> +                       }
> +               }
> +       }
> +
>         hwspin_unlock_raw(data->hwlock);
>
>         if (!ret)
> @@ -523,6 +684,7 @@ static void sc27xx_adc_disable(void *_data)
>  }
>



-- 
Baolin Wang

Re: [PATCH V2 5/7] iio: adc: sc27xx: add support for PMIC sc2730

[Baolin Wang]

On Sat, Mar 12, 2022 at 12:47 AM Cixi Geng <gengcixi@gmail.com> wrote:
>
> From: Cixi Geng <cixi.geng1@unisoc.com>
>
> sc2730 is the product of sc27xx series.
>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  drivers/iio/adc/sc27xx_adc.c | 108 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 2603ce313b07..b89637c051ac 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)
> @@ -293,6 +295,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) {
> @@ -349,6 +425,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;
> @@ -449,7 +541,8 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>                                                     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");
> +                               dev_err(data->dev, "failed to set the volref 2.8V, ret_volref = 0x%x\n",
> +                                       ret_volref);

Do not add unrelated changes in this patch.

-- 
Baolin Wang

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

[Cixi Geng]

Jonathan Cameron <jic23@kernel.org> 于2022年3月20日周日 21:22写道：
>
> On Sat, 12 Mar 2022 00:46:22 +0800
> Cixi Geng <gengcixi@gmail.com> wrote:
>
> > From: Cixi Geng <cixi.geng1@unisoc.com>
>
> In title, use dt-binding instead of dtbindings at the end.
>
> >
> > sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
> > dtbindings.
>
> dt-bindings.
>
> >
> > Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
>
> If the patch was authored by Chunyan Zhang, it should be From: Chunyan Zhang,
> if it is more complex than you simply passing the patch on then we should
> see a co-developed to indicate that.
Thanks your advise, I will fix the comment and add correct Signed message
>
> Patch looks good to me
>
> Jonathan
>
> > ---
> >  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
> >  1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
> > @@ -37,9 +38,32 @@ properties:
> >      maxItems: 2
> >
> >    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-cell-names:
> > +      items:
> > +        - const: big_scale_calib
> > +        - const: small_scale_calib
> > +
> > +else:
> > +  properties:
> > +    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
>

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

[Cixi Geng]

Krzysztof Kozlowski <krzk@kernel.org> 于2022年3月20日周日 22:50写道：
>
> On 11/03/2022 17:46, Cixi Geng wrote:
> > From: Cixi Geng <cixi.geng1@unisoc.com>
> >
> > sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
> > dtbindings.
> >
> > Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > ---
> >  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
> >  1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
> > @@ -37,9 +38,32 @@ properties:
> >      maxItems: 2
> >
> >    nvmem-cell-names:
> > -    items:
> > -      - const: big_scale_calib
> > -      - const: small_scale_calib
>
> Please test your changes with dt_binding_check and dtbs_check. Your
> change looks not complete - you have still nvmem-cells = 2.
>
Hi Krzysztof
I test all is PASS on my local.  could you tell how did you test?
my_logs:
cixi.geng1@tj10039pcu:~/upsteatming/linux$ make DT_CHECKER_FLAGS=-m
dt_binding_check &>dt_check.log
cixi.geng1@tj10039pcu:~/upsteatming/linux$ cat dt_check.log |grep sprd,sc2720
  DTEX    Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dts
  DTC     Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dt.yaml
cixi.geng1@tj10039pcu:~/upsteatming/linux$ tuxmake -C ${kernel_src} -b
${topdir}/obj/gcc -o ${topdir}/dist/gcc -a $ARCH -t gcc  -K
CONFIG_ARCH_${PLAT}=y -K CONFIG_MFD_SC27XX_PMIC=y -K
CONFIG_SC27XX_ADC=y

I: config: PASS in 0:00:00.000549
I: default: PASS in 0:10:20.931602
I: kernel: PASS in 0:01:10.643458
I: xipkernel: SKIP in 0:00:00.003244
I: modules: PASS in 0:00:35.658938
I: dtbs: PASS in 0:00:18.696416
I: dtbs-legacy: SKIP in 0:00:00.005625
I: debugkernel: PASS in 0:00:11.541855
I: headers: PASS in 0:00:11.778253
I: build output in /home/cixi.geng1/upsteatming/dist/gcc


>
> Best regards,
> Krzysztof

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

[Cixi Geng]

Jonathan Cameron <jic23@kernel.org> 于2022年3月20日周日 21:25写道：
>
> On Sat, 12 Mar 2022 00:46:23 +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: Yuming Zhu <yuming.zhu1@unisoc.com>
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> >
> No blank lines in a tag block (they break people's scripts)
> Also, if this is a fix, I'd expect a fixes tag.
I will add in next version
>
> > 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
>
> This driver would benefit from use of FIELD_GET() / FIELD_PREP()
> but that is obviously unrelated to this particular series.
>
the next patch in this set need to use the fixed define value
>
> Jonathan
>
> >
> >  /* Bits definitions for SC27XX_ADC_INT_EN registers */
> >  #define SC27XX_ADC_IRQ_EN            BIT(0)
>

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

[Krzysztof Kozlowski]

On 24/03/2022 07:22, Cixi Geng wrote:
> Krzysztof Kozlowski <krzk@kernel.org> 于2022年3月20日周日 22:50写道：
>>
>> On 11/03/2022 17:46, Cixi Geng wrote:
>>> From: Cixi Geng <cixi.geng1@unisoc.com>
>>>
>>> sprd,ump9620-adc is one variant of sc27xx series, add ump9620 in
>>> dtbindings.
>>>
>>> Signed-off-by: Chunyan Zhang <zhang.lyra@gmail.com>
>>> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
>>> ---
>>>  .../bindings/iio/adc/sprd,sc2720-adc.yaml     | 30 +++++++++++++++++--
>>>  1 file changed, 27 insertions(+), 3 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..331b08fb1761 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
>>> @@ -37,9 +38,32 @@ properties:
>>>      maxItems: 2
>>>
>>>    nvmem-cell-names:
>>> -    items:
>>> -      - const: big_scale_calib
>>> -      - const: small_scale_calib
>>
>> Please test your changes with dt_binding_check and dtbs_check. Your
>> change looks not complete - you have still nvmem-cells = 2.
>>
> Hi Krzysztof
> I test all is PASS on my local.  could you tell how did you test?
> my_logs:
> cixi.geng1@tj10039pcu:~/upsteatming/linux$ make DT_CHECKER_FLAGS=-m
> dt_binding_check &>dt_check.log
> cixi.geng1@tj10039pcu:~/upsteatming/linux$ cat dt_check.log |grep sprd,sc2720
>   DTEX    Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dts
>   DTC     Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dt.yaml
>   CHECK   Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dt.yaml
> cixi.geng1@tj10039pcu:~/upsteatming/linux$ tuxmake -C ${kernel_src} -b
> ${topdir}/obj/gcc -o ${topdir}/dist/gcc -a $ARCH -t gcc  -K
> CONFIG_ARCH_${PLAT}=y -K CONFIG_MFD_SC27XX_PMIC=y -K
> CONFIG_SC27XX_ADC=y
> 

The method is correct, just please test sprd,ump9620-adc (either in
example in the binding or your DTS with `make dtbs_check
DT_SCHEMA_FILES=sprd,sc2720-adc.yaml).

Best regards,
Krzysztof

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

[Jonathan Cameron]

On Thu, 24 Mar 2022 15:36:56 +0800
Cixi Geng <gengcixi@gmail.com> wrote:

> Jonathan Cameron <jic23@kernel.org> 于2022年3月20日周日 21:25写道：
> >
> > On Sat, 12 Mar 2022 00:46:23 +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: Yuming Zhu <yuming.zhu1@unisoc.com>
> > > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > >  
> > No blank lines in a tag block (they break people's scripts)
> > Also, if this is a fix, I'd expect a fixes tag.  
> I will add in next version
> >  
> > > 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  
> >
> > This driver would benefit from use of FIELD_GET() / FIELD_PREP()
> > but that is obviously unrelated to this particular series.
> >  
> the next patch in this set need to use the fixed define value

Understood.  I was suggesting a future cleanup on top of this series
to avoid the need for both MASK and SHIFT being defined for
each field.  FIELD_GET/FIELD_PREP() are used to do that.

What you have in this series is fine without that change.

Jonathan

> >
> > Jonathan
> >  
> > >
> > >  /* Bits definitions for SC27XX_ADC_INT_EN registers */
> > >  #define SC27XX_ADC_IRQ_EN            BIT(0)  
> >  

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

[Cixi Geng]

Baolin Wang <baolin.wang7@gmail.com> 于2022年3月23日周三 21:55写道：
>
> On Sat, Mar 12, 2022 at 12:47 AM Cixi Geng <gengcixi@gmail.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > ---
> >  drivers/iio/adc/sc27xx_adc.c | 95 ++++++++++++++++++++++++++++++------
> >  1 file changed, 80 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index aee076c8e2b1..68629fbcfec5 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,6 +120,17 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> >         100, 341,
> >  };
> >
> > +/* 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 sc2731_small_scale_graph_calib = {
> > +       1000, 838,
> > +       100, 84,
> > +};
> > +
> >  static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> >         4200, 856,
> >         3600, 733,
> > @@ -130,11 +158,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;
>
> Now which function will use big_scale_graph_calib and
> small_scale_graph_calib?  Seems you add an unused warning?
yes, it is a unused, since these calibs used for sc2730 and sc2720
I will remove in this patch and add in patch4/7.
>
> >                 graph = &small_scale_graph;
> >                 cell_name = "small_scale_calib";
> >         }
> > @@ -160,7 +188,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 +213,21 @@ 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;
> > +
> > +       for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> > +               if (i == 5)
> > +                       data->channel_scale[i] = 1;
>
> Can you add some comments to explain why channel 5 needs to set scale to 1?
In the current software design, SC2731 supports 2 scales, channel 5
uses big scale, others use smale
I will add the comment in next version.
>
> > +               else
> > +                       data->channel_scale[i] = 0;
> > +       }
> > +}
> > +
> >  static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
> >                            int scale, int *val)
> >  {
> > @@ -208,10 +251,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 +306,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 +477,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 +501,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 +515,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 +583,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 +611,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
> >
>
>
> --
> Baolin Wang

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

[Cixi Geng]

Jonathan Cameron <jic23@kernel.org> 于2022年3月20日周日 22:34写道：
>
> On Sat, 12 Mar 2022 00:46:27 +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.
> >
> > Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> Hi,
>
> Same comments as made earlier apply here.
> * Should there be a co-developed-by marking here or should
> the from reflect Yuming Zhu as the patch author with you in
> the role of being on the path to upstream?
> * change log below the ---
> * No blank lines or non tag lines in the tag block - use
> comments at the end of the lines if you want to give
> reported-by for particularly things.
>
> Also, I'm not immediately spotting where Baolin gave
> a tag.
>
> Comments inline.
>
> There are a few places in here where you have significant deviation between
> the code that runs for this new device and previously supported parts.
> Doing that via if / else tends not to scale as yet more parts are added in
> future. Perhaps it would be better to move to some device type specific
> callbacks for more of these cases.  This would be similar to the init_scale
> and get_ratio function pointers you already have in the variant data.
>
> Thanks,
>
> Jonathan
>
> >
> > V2 changes:
> > 1. remove duplicated function
> > Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
> >
> > 2. fix the smatch warnings
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >  drivers/iio/adc/sc27xx_adc.c | 305 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 266 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index b89637c051ac..e9b680e9c275 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,
> > @@ -165,16 +181,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)
> > +{
>
> This looks to be a bit of refactoring that could be sensibly pulled out before this
> patch.
Ok, I will do pull out the refactor part in a single patch
>
> > +     struct nvmem_cell *cell;
> > +     void *buf;
> > +     u32 origin_calib_data = 0;
>
> Why initialise the above?   I don't see it being used
> in any paths where it isn't already initialised.

Hi Jonathan:
It used origin_calib_data in memcpy,and the length of data
may less than sizeof(u32). we expect the other bits keeps 0;
So we initialise 0 in here.
>
> > +     size_t len = 0;
> > +
> > +     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;
> > @@ -186,24 +227,63 @@ 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;
> > +}
> > +
> > +static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
> > +                                     enum ump96xx_scale_cal cal_type)
> > +{
> > +     struct sc27xx_adc_linear_graph *graph = NULL;
>
> Always set below, so don't initialize here. Same for other
> local variables.
here do make no sense, remove the initalise in next version.
>
> > +     const char *cell_name1 = NULL, *cell_name2 = NULL;
> > +     int adc_calib_data1 = 0, adc_calib_data2 = 0;
> > +
> > +     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;
> >  }
> >
> > @@ -394,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.
> >   */
> > @@ -453,6 +577,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)
> >  {
> > @@ -567,7 +707,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;
> > @@ -576,6 +716,31 @@ 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;
> > +}
> > +
> > +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;
> >  }
> >
> > @@ -585,23 +750,46 @@ static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
> >       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;
> > +             }
> >
> > -     case 1:
> > -             return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> > +             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);
> >
> > -     default:
> > -             volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> > -             break;
> > +             case 1:
> > +                     return sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> > +
> > +             default:
> > +                     volt = sc27xx_adc_to_volt(&small_scale_graph, raw_adc);
> > +                     break;
> > +             }
> >       }
> >
> >       sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
> > @@ -619,6 +807,7 @@ static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
> >               return ret;
> >
> >       *val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
> > +
> >       return 0;
> >  }
> >
> > @@ -736,21 +925,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;
> >
> > @@ -774,6 +984,10 @@ 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 = {
> > @@ -824,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;
> > @@ -917,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);
>

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

[Jonathan Cameron]

> Hi Jonathan:
> It used origin_calib_data in memcpy,and the length of data
> may less than sizeof(u32). we expect the other bits keeps 0;
> So we initialise 0 in here.
Ah. I'd missed that.  Thanks for the explanation.

> >  
> > > +     size_t len = 0;
> > > +
> > > +     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;
> > > +}

Jonathan