* [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support
@ 2022-01-06 12:59 Cixi Geng
2022-01-06 12:59 ` [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings Cixi Geng
` (6 more replies)
0 siblings, 7 replies; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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
.../bindings/iio/adc/sprd,sc2720-adc.yaml | 19 +
drivers/iio/adc/sc27xx_adc.c | 767 +++++++++++++++++-
2 files changed, 759 insertions(+), 27 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-06 17:39 ` Rob Herring
2022-01-06 12:59 ` [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right Cixi Geng
` (5 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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 | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml b/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml
index caa3ee0b4b8c..cd20ff17e58c 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
@@ -36,11 +37,29 @@ properties:
nvmem-cells:
maxItems: 2
+if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - sprd,ump9620-adc
+then:
nvmem-cell-names:
items:
- const: big_scale_calib
- const: small_scale_calib
+else:
+ 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
- reg
--
2.25.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
2022-01-06 12:59 ` [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-07 6:55 ` Baolin Wang
2022-01-06 12:59 ` [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization Cixi Geng
` (4 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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>
---
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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
2022-01-06 12:59 ` [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings Cixi Geng
2022-01-06 12:59 ` [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-07 7:04 ` Baolin Wang
2022-01-06 12:59 ` [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 Cixi Geng
` (3 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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 | 94 ++++++++++++++++++++++++++++++------
1 file changed, 79 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
100, 341,
};
+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 +157,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 +187,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 +212,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 +250,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 +305,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 +476,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 +500,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 +514,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 +582,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 +610,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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
` (2 preceding siblings ...)
2022-01-06 12:59 ` [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-07 7:16 ` Baolin Wang
2022-01-06 12:59 ` [PATCH 5/7] iio: adc: sc27xx: add support for PMIC sc2730 Cixi Geng
` (2 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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>
---
drivers/iio/adc/sc27xx_adc.c | 198 +++++++++++++++++++++++++++++++++++
1 file changed, 198 insertions(+)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index d2712e54ee79..7b5c66660ac9 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,21 @@
#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 iio_dev *indio_dev;
struct device *dev;
+ struct regulator *volref;
struct regmap *regmap;
/*
* One hardware spinlock to synchronize between the multiple
@@ -87,6 +104,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;
@@ -187,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) {
@@ -215,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;
@@ -239,6 +373,24 @@ 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) {
+ if ((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)
@@ -293,6 +445,16 @@ 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 = regulator_set_voltage(data->volref,
+ SC27XX_ADC_REFVOL_VDD28,
+ SC27XX_ADC_REFVOL_VDD28);
+ if (ret)
+ dev_err(data->dev, "failed to set the volref 2.8V\n");
+ }
+ }
+
hwspin_unlock_raw(data->hwlock);
if (!ret)
@@ -522,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,
@@ -532,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;
@@ -582,6 +769,15 @@ 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_OR_NULL(sc27xx_data->volref)) {
+ ret = PTR_ERR(sc27xx_data->volref);
+ dev_err(dev, "err! ADC volref, err: %d\n", ret);
+ return ret;
+ }
+ }
+
sc27xx_data->var_data = pdata;
sc27xx_data->var_data->init_scale(sc27xx_data);
@@ -611,6 +807,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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 5/7] iio: adc: sc27xx: add support for PMIC sc2730
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
` (3 preceding siblings ...)
2022-01-06 12:59 ` [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-06 12:59 ` [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 Cixi Geng
2022-01-06 12:59 ` [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support Cixi Geng
6 siblings, 0 replies; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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 | 105 +++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 7b5c66660ac9..195f44cf61e1 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;
@@ -695,6 +787,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,
@@ -807,6 +911,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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
` (4 preceding siblings ...)
2022-01-06 12:59 ` [PATCH 5/7] iio: adc: sc27xx: add support for PMIC sc2730 Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-07 7:23 ` Baolin Wang
2022-01-06 12:59 ` [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support Cixi Geng
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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>
---
drivers/iio/adc/sc27xx_adc.c | 263 +++++++++++++++++++++++++++++++++--
1 file changed, 254 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 195f44cf61e1..68b967f32498 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 {
@@ -140,6 +151,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,
+};
+
static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
4200, 850,
3600, 728,
@@ -165,6 +181,33 @@ static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
return ((calib_data & 0xff) + calib_adc - 128) * 4;
}
+static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
+{
+ struct nvmem_cell *cell;
+ void *buf;
+ u32 calib_data = 0;
+ size_t len = 0;
+
+ if (!data)
+ return -EINVAL;
+
+ cell = nvmem_cell_get(data->dev, cell_name);
+ if (IS_ERR_OR_NULL(cell))
+ return PTR_ERR(cell);
+
+ buf = nvmem_cell_read(cell, &len);
+ if (IS_ERR_OR_NULL(buf)) {
+ nvmem_cell_put(cell);
+ return PTR_ERR(buf);
+ }
+
+ memcpy(&calib_data, buf, min(len, sizeof(u32)));
+
+ kfree(buf);
+ nvmem_cell_put(cell);
+ return calib_data;
+}
+
static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
bool big_scale)
{
@@ -207,6 +250,56 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
return 0;
}
+static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
+ enum ump96xx_scale_cal cal_type)
+{
+ struct sc27xx_adc_linear_graph *graph = 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;
+}
+
static int sc2720_adc_get_ratio(int channel, int scale)
{
switch (channel) {
@@ -394,6 +487,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 +590,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)
{
@@ -578,6 +731,23 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
return tmp < 0 ? 0 : tmp;
}
+static int ump96xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph, int scale,
+ int raw_adc)
+{
+ int tmp;
+
+ tmp = (graph->volt0 - graph->volt1) * (raw_adc - graph->adc1);
+ tmp /= (graph->adc0 - graph->adc1);
+ tmp += graph->volt1;
+
+ if (scale == 2)
+ tmp = tmp * 2600 / 1000;
+ else if (scale == 3)
+ tmp = tmp * 4060 / 1000;
+
+ return tmp < 0 ? 0 : tmp;
+}
+
static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
int scale, int raw_adc)
{
@@ -608,6 +778,39 @@ static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
return DIV_ROUND_CLOSEST(volt * denominator, numerator);
}
+static int ump96xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
+ int scale, int raw_adc)
+{
+ u32 numerator, denominator;
+ u32 volt;
+
+ switch (channel) {
+ case 0:
+ if (scale == 1)
+ volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
+ else
+ volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
+ break;
+ case 11:
+ volt = sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
+ break;
+ default:
+ if (scale == 1)
+ volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
+ else
+ volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
+ break;
+ }
+
+ if (channel == 0 && scale == 1)
+ return volt;
+
+ sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
+
+ return DIV_ROUND_CLOSEST(volt * denominator, numerator);
+}
+
+
static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
int channel, int scale, int *val)
{
@@ -617,7 +820,11 @@ static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
if (ret)
return ret;
- *val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
+ if (data->var_data->pmic_type == UMP9620_ADC)
+ *val = ump96xx_adc_convert_volt(data, channel, scale, raw_adc);
+ else
+ *val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
+
return 0;
}
@@ -735,21 +942,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;
@@ -773,6 +1001,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 = {
@@ -823,6 +1055,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;
@@ -914,6 +1158,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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
` (5 preceding siblings ...)
2022-01-06 12:59 ` [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 Cixi Geng
@ 2022-01-06 12:59 ` Cixi Geng
2022-01-07 7:34 ` Baolin Wang
6 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-06 12:59 UTC (permalink / raw)
To: orsonzhai, baolin.wang7, zhang.lyra, jic23, lars, robh+dt,
lgirdwood, broonie
Cc: yuming.zhu1, linux-iio, devicetree, linux-kernel
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: Cixi Geng <cixi.geng1@unisoc.com>
Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
---
drivers/iio/adc/sc27xx_adc.c | 103 ++++++++++++++++++++++++++++++++++-
1 file changed, 102 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
index 68b967f32498..cecda8d53474 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,
@@ -618,6 +622,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
@@ -700,6 +707,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)
@@ -947,6 +959,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
ret = regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
UMP9620_XTL_WAIT_CTRL0_EN,
UMP9620_XTL_WAIT_CTRL0_EN);
+ 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 */
@@ -988,6 +1004,11 @@ 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)
+ regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
+ UMP9620_XTL_WAIT_CTRL0_EN, 0);
+
return ret;
}
@@ -1086,6 +1107,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);
@@ -1126,7 +1149,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);
@@ -1137,18 +1163,39 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
ret = devm_add_action_or_reset(dev, sc27xx_adc_disable, sc27xx_data);
if (ret) {
+ sc27xx_adc_disable(sc27xx_data);
dev_err(dev, "failed to add ADC disable action\n");
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);
+ }
+
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;
}
@@ -1163,11 +1210,65 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
};
MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
+static int sc27xx_adc_remove(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct sc27xx_adc_data *sc27xx_data = iio_priv(indio_dev);
+
+ if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ /* set the UMP9620 ADC clk26m bit8 on IP */
+ regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
+ UMP9620_XTL_WAIT_CTRL0_EN, 0);
+ }
+
+ return 0;
+}
+
+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 = {
+ .runtime_suspend = &sc27xx_adc_runtime_suspend,
+ .runtime_resume = &sc27xx_adc_runtime_resume,
+};
+
static struct platform_driver sc27xx_adc_driver = {
.probe = sc27xx_adc_probe,
+ .remove = sc27xx_adc_remove,
.driver = {
.name = "sc27xx-adc",
.of_match_table = sc27xx_adc_of_match,
+ .pm = &sc27xx_adc_pm_ops,
},
};
--
2.25.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings
2022-01-06 12:59 ` [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings Cixi Geng
@ 2022-01-06 17:39 ` Rob Herring
0 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2022-01-06 17:39 UTC (permalink / raw)
To: Cixi Geng
Cc: lgirdwood, lars, jic23, baolin.wang7, broonie, robh+dt,
linux-iio, linux-kernel, devicetree, zhang.lyra, orsonzhai,
yuming.zhu1
On Thu, 06 Jan 2022 20:59:41 +0800, 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 | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml: else: 'nvmem-cell-names' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml: then: 'nvmem-cell-names' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.yaml: ignoring, error in schema: else
Documentation/devicetree/bindings/iio/adc/sprd,sc2720-adc.example.dt.yaml:0:0: /example-0/pmic/adc@480: failed to match any schema with compatible: ['sprd,sc2731-adc']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1576074
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right
2022-01-06 12:59 ` [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right Cixi Geng
@ 2022-01-07 6:55 ` Baolin Wang
2022-01-09 16:06 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-01-07 6:55 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Thu, Jan 6, 2022 at 9:00 PM 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>
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
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-06 12:59 ` [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization Cixi Geng
@ 2022-01-07 7:04 ` Baolin Wang
2022-01-13 1:53 ` Cixi Geng
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-01-07 7:04 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> 1 file changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> 100, 341,
> };
>
> +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,
> +};
The original big_scale_graph_calib and small_scale_graph_calib are for
SC2731 PMIC, why add new structure definition for SC2731?
> +
> static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> 4200, 856,
> 3600, 733,
> @@ -130,11 +157,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 +187,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 +212,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;
> + }
> +}
This is unnecessary I think, please see sc27xx_adc_write_raw() that
can set the channel scale.
> +
> static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
> int scale, int *val)
> {
> @@ -208,10 +250,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 +305,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 +476,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 +500,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 +514,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 +582,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 +610,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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
2022-01-06 12:59 ` [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 Cixi Geng
@ 2022-01-07 7:16 ` Baolin Wang
2022-01-09 16:13 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-01-07 7:16 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Thu, Jan 6, 2022 at 9:00 PM 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>
> ---
> drivers/iio/adc/sc27xx_adc.c | 198 +++++++++++++++++++++++++++++++++++
> 1 file changed, 198 insertions(+)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index d2712e54ee79..7b5c66660ac9 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,21 @@
> #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 iio_dev *indio_dev;
Why add an unused member?
> struct device *dev;
> + struct regulator *volref;
> struct regmap *regmap;
> /*
> * One hardware spinlock to synchronize between the multiple
> @@ -87,6 +104,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;
> @@ -187,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) {
> @@ -215,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;
> + }
> + }
> +}
Like previous comments, this is not needed.
> +
> static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
> {
> int i;
> @@ -239,6 +373,24 @@ 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) {
> + if ((channel == 30) || (channel == 31)) {
Combine the two branches please.
> + 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)
> @@ -293,6 +445,16 @@ 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 = regulator_set_voltage(data->volref,
> + SC27XX_ADC_REFVOL_VDD28,
> + SC27XX_ADC_REFVOL_VDD28);
> + if (ret)
> + dev_err(data->dev, "failed to set the volref 2.8V\n");
> + }
> + }
> +
> hwspin_unlock_raw(data->hwlock);
>
> if (!ret)
> @@ -522,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,
> @@ -532,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;
> @@ -582,6 +769,15 @@ 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_OR_NULL(sc27xx_data->volref)) {
devm_regulator_get_optional() never return NULL, please use IS_ERR().
> + ret = PTR_ERR(sc27xx_data->volref);
Should check ret == -ENODEV, since -ENODEV means the regulator is not
supplied which is not a error for 'OPTIONAL_GET' type.
> + dev_err(dev, "err! ADC volref, err: %d\n", ret);
Can you elaborate on the error message like other places in this driver?
> + return ret;
> + }
> + }
> +
> sc27xx_data->var_data = pdata;
> sc27xx_data->var_data->init_scale(sc27xx_data);
>
> @@ -611,6 +807,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
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620
2022-01-06 12:59 ` [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 Cixi Geng
@ 2022-01-07 7:23 ` Baolin Wang
0 siblings, 0 replies; 30+ messages in thread
From: Baolin Wang @ 2022-01-07 7:23 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Thu, Jan 6, 2022 at 9:00 PM 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>
> ---
> drivers/iio/adc/sc27xx_adc.c | 263 +++++++++++++++++++++++++++++++++--
> 1 file changed, 254 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 195f44cf61e1..68b967f32498 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 {
> @@ -140,6 +151,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,
> +};
> +
> static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
> 4200, 850,
> 3600, 728,
> @@ -165,6 +181,33 @@ static int sc27xx_adc_get_calib_data(u32 calib_data, int calib_adc)
> return ((calib_data & 0xff) + calib_adc - 128) * 4;
> }
>
> +static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
> +{
> + struct nvmem_cell *cell;
> + void *buf;
> + u32 calib_data = 0;
> + size_t len = 0;
> +
> + if (!data)
> + return -EINVAL;
> +
> + cell = nvmem_cell_get(data->dev, cell_name);
> + if (IS_ERR_OR_NULL(cell))
> + return PTR_ERR(cell);
> +
> + buf = nvmem_cell_read(cell, &len);
> + if (IS_ERR_OR_NULL(buf)) {
> + nvmem_cell_put(cell);
> + return PTR_ERR(buf);
> + }
> +
> + memcpy(&calib_data, buf, min(len, sizeof(u32)));
> +
> + kfree(buf);
> + nvmem_cell_put(cell);
> + return calib_data;
> +}
These are some duplicated code in sc27xx_adc_scale_calibration(),
please factor out the sc27xx_adc_scale_calibration() firstly.
> +
> static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
> bool big_scale)
> {
> @@ -207,6 +250,56 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
> return 0;
> }
>
> +static int ump96xx_adc_scale_cal(struct sc27xx_adc_data *data,
> + enum ump96xx_scale_cal cal_type)
> +{
> + struct sc27xx_adc_linear_graph *graph = 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;
> +}
> +
> static int sc2720_adc_get_ratio(int channel, int scale)
> {
> switch (channel) {
> @@ -394,6 +487,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 +590,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)
> {
> @@ -578,6 +731,23 @@ static int sc27xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph,
> return tmp < 0 ? 0 : tmp;
> }
>
> +static int ump96xx_adc_to_volt(struct sc27xx_adc_linear_graph *graph, int scale,
> + int raw_adc)
> +{
> + int tmp;
> +
> + tmp = (graph->volt0 - graph->volt1) * (raw_adc - graph->adc1);
> + tmp /= (graph->adc0 - graph->adc1);
> + tmp += graph->volt1;
These are also copy-paste from sc27xx_adc_to_volt(), please avoid
duplicate code.
> +
> + if (scale == 2)
> + tmp = tmp * 2600 / 1000;
> + else if (scale == 3)
> + tmp = tmp * 4060 / 1000;
> +
> + return tmp < 0 ? 0 : tmp;
> +}
> +
> static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
> int scale, int raw_adc)
> {
> @@ -608,6 +778,39 @@ static int sc27xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
> return DIV_ROUND_CLOSEST(volt * denominator, numerator);
> }
>
> +static int ump96xx_adc_convert_volt(struct sc27xx_adc_data *data, int channel,
> + int scale, int raw_adc)
> +{
> + u32 numerator, denominator;
> + u32 volt;
> +
> + switch (channel) {
> + case 0:
> + if (scale == 1)
> + volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
> + else
> + volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
> + break;
> + case 11:
> + volt = sc27xx_adc_to_volt(&big_scale_graph, raw_adc);
> + break;
> + default:
> + if (scale == 1)
> + volt = sc27xx_adc_to_volt(&ump9620_bat_det_graph, raw_adc);
> + else
> + volt = ump96xx_adc_to_volt(&small_scale_graph, scale, raw_adc);
> + break;
> + }
> +
> + if (channel == 0 && scale == 1)
> + return volt;
> +
> + sc27xx_adc_volt_ratio(data, channel, scale, &numerator, &denominator);
> +
> + return DIV_ROUND_CLOSEST(volt * denominator, numerator);
> +}
> +
> +
> static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
> int channel, int scale, int *val)
> {
> @@ -617,7 +820,11 @@ static int sc27xx_adc_read_processed(struct sc27xx_adc_data *data,
> if (ret)
> return ret;
>
> - *val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
> + if (data->var_data->pmic_type == UMP9620_ADC)
> + *val = ump96xx_adc_convert_volt(data, channel, scale, raw_adc);
> + else
> + *val = sc27xx_adc_convert_volt(data, channel, scale, raw_adc);
> +
> return 0;
> }
>
> @@ -735,21 +942,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;
>
> @@ -773,6 +1001,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 = {
> @@ -823,6 +1055,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;
> @@ -914,6 +1158,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
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support
2022-01-06 12:59 ` [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support Cixi Geng
@ 2022-01-07 7:34 ` Baolin Wang
2022-01-09 16:22 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-01-07 7:34 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Thu, Jan 6, 2022 at 9:00 PM 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: Cixi Geng <cixi.geng1@unisoc.com>
> Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
> ---
> drivers/iio/adc/sc27xx_adc.c | 103 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index 68b967f32498..cecda8d53474 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,
> @@ -618,6 +622,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
> @@ -700,6 +707,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)
> @@ -947,6 +959,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
> ret = regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> UMP9620_XTL_WAIT_CTRL0_EN,
> UMP9620_XTL_WAIT_CTRL0_EN);
> + 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 */
> @@ -988,6 +1004,11 @@ 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)
> + regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> + UMP9620_XTL_WAIT_CTRL0_EN, 0);
Can you hide this into the pm runtime callbacks?
> +
> return ret;
> }
>
> @@ -1086,6 +1107,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);
> @@ -1126,7 +1149,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);
> @@ -1137,18 +1163,39 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
>
> ret = devm_add_action_or_reset(dev, sc27xx_adc_disable, sc27xx_data);
> if (ret) {
> + sc27xx_adc_disable(sc27xx_data);
> dev_err(dev, "failed to add ADC disable action\n");
> 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);
> + }
> +
> 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);
I don't think the pm_runtime_put() is needed, since you did not get
the counter before, right?
> + pm_runtime_disable(dev);
> + }
>
> return ret;
> }
> @@ -1163,11 +1210,65 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
>
> +static int sc27xx_adc_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct sc27xx_adc_data *sc27xx_data = iio_priv(indio_dev);
> +
> + if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
> + pm_runtime_put(&pdev->dev);
You did not get the pm count, why put it firstly?
> + pm_runtime_disable(&pdev->dev);
> +
> + /* set the UMP9620 ADC clk26m bit8 on IP */
> + regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
> + UMP9620_XTL_WAIT_CTRL0_EN, 0);
> + }
> +
> + return 0;
> +}
> +
> +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;
no need to initialize 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 = {
> + .runtime_suspend = &sc27xx_adc_runtime_suspend,
> + .runtime_resume = &sc27xx_adc_runtime_resume,
> +};
Please use SET_RUNTIME_PM_OPS macro.
> +
> static struct platform_driver sc27xx_adc_driver = {
> .probe = sc27xx_adc_probe,
> + .remove = sc27xx_adc_remove,
> .driver = {
> .name = "sc27xx-adc",
> .of_match_table = sc27xx_adc_of_match,
> + .pm = &sc27xx_adc_pm_ops,
> },
> };
>
> --
> 2.25.1
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
2022-01-06 12:59 ` [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 Cixi Geng
2022-01-07 7:16 ` Baolin Wang
@ 2022-01-10 5:17 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2022-01-07 16:21 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9284 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20220106125947.139523-5-gengcixi@gmail.com>
References: <20220106125947.139523-5-gengcixi@gmail.com>
TO: Cixi Geng <gengcixi@gmail.com>
TO: orsonzhai(a)gmail.com
TO: baolin.wang7(a)gmail.com
TO: zhang.lyra(a)gmail.com
TO: jic23(a)kernel.org
TO: lars(a)metafoo.de
TO: robh+dt(a)kernel.org
TO: lgirdwood(a)gmail.com
TO: broonie(a)kernel.org
CC: yuming.zhu1(a)unisoc.com
CC: linux-iio(a)vger.kernel.org
Hi Cixi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.16-rc8 next-20220106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
:::::: branch date: 27 hours ago
:::::: commit date: 27 hours ago
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080030.L51zYw0G-lkp(a)intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/iio/adc/sc27xx_adc.c:461 sc27xx_adc_read() error: uninitialized symbol 'value'.
drivers/iio/adc/sc27xx_adc.c:775 sc27xx_adc_probe() warn: passing zero to 'PTR_ERR'
vim +/value +461 drivers/iio/adc/sc27xx_adc.c
b39db3bcbc9a96 Cixi Geng 2022-01-06 363
5df362a6cf49ca Freeman Liu 2018-06-21 364 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
5df362a6cf49ca Freeman Liu 2018-06-21 365 int scale, int *val)
5df362a6cf49ca Freeman Liu 2018-06-21 366 {
5df362a6cf49ca Freeman Liu 2018-06-21 367 int ret;
8de877d2bba5c3 Freeman Liu 2019-07-25 368 u32 tmp, value, status;
5df362a6cf49ca Freeman Liu 2018-06-21 369
5df362a6cf49ca Freeman Liu 2018-06-21 370 ret = hwspin_lock_timeout_raw(data->hwlock, SC27XX_ADC_HWLOCK_TIMEOUT);
5df362a6cf49ca Freeman Liu 2018-06-21 371 if (ret) {
5df362a6cf49ca Freeman Liu 2018-06-21 372 dev_err(data->dev, "timeout to get the hwspinlock\n");
5df362a6cf49ca Freeman Liu 2018-06-21 373 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 374 }
5df362a6cf49ca Freeman Liu 2018-06-21 375
cd6ab0edd81be2 Cixi Geng 2022-01-06 376 /*
cd6ab0edd81be2 Cixi Geng 2022-01-06 377 * According to the sc2721 chip data sheet, the reference voltage of
cd6ab0edd81be2 Cixi Geng 2022-01-06 378 * specific channel 30 and channel 31 in ADC module needs to be set from
cd6ab0edd81be2 Cixi Geng 2022-01-06 379 * the default 2.8v to 3.5v.
cd6ab0edd81be2 Cixi Geng 2022-01-06 380 */
cd6ab0edd81be2 Cixi Geng 2022-01-06 381 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 382 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 383 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 384 SC27XX_ADC_REFVOL_VDD35,
cd6ab0edd81be2 Cixi Geng 2022-01-06 385 SC27XX_ADC_REFVOL_VDD35);
cd6ab0edd81be2 Cixi Geng 2022-01-06 386 if (ret) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 387 dev_err(data->dev, "failed to set the volref 3.5V\n");
cd6ab0edd81be2 Cixi Geng 2022-01-06 388 hwspin_unlock_raw(data->hwlock);
cd6ab0edd81be2 Cixi Geng 2022-01-06 389 return ret;
cd6ab0edd81be2 Cixi Geng 2022-01-06 390 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 391 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 392 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 393
5df362a6cf49ca Freeman Liu 2018-06-21 394 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 395 SC27XX_ADC_EN, SC27XX_ADC_EN);
5df362a6cf49ca Freeman Liu 2018-06-21 396 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 397 goto unlock_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 398
8de877d2bba5c3 Freeman Liu 2019-07-25 399 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
8de877d2bba5c3 Freeman Liu 2019-07-25 400 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
8de877d2bba5c3 Freeman Liu 2019-07-25 401 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 402 goto disable_adc;
8de877d2bba5c3 Freeman Liu 2019-07-25 403
5df362a6cf49ca Freeman Liu 2018-06-21 404 /* Configure the channel id and scale */
b39db3bcbc9a96 Cixi Geng 2022-01-06 405 tmp = (scale << data->var_data->scale_shift) & data->var_data->scale_mask;
5df362a6cf49ca Freeman Liu 2018-06-21 406 tmp |= channel & SC27XX_ADC_CHN_ID_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 407 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CH_CFG,
b39db3bcbc9a96 Cixi Geng 2022-01-06 408 SC27XX_ADC_CHN_ID_MASK |
b39db3bcbc9a96 Cixi Geng 2022-01-06 409 data->var_data->scale_mask,
5df362a6cf49ca Freeman Liu 2018-06-21 410 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 411 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 412 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 413
5df362a6cf49ca Freeman Liu 2018-06-21 414 /* Select 12bit conversion mode, and only sample 1 time */
5df362a6cf49ca Freeman Liu 2018-06-21 415 tmp = SC27XX_ADC_12BIT_MODE;
5df362a6cf49ca Freeman Liu 2018-06-21 416 tmp |= (0 << SC27XX_ADC_RUN_NUM_SHIFT) & SC27XX_ADC_RUN_NUM_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 417 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 418 SC27XX_ADC_RUN_NUM_MASK | SC27XX_ADC_12BIT_MODE,
5df362a6cf49ca Freeman Liu 2018-06-21 419 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 420 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 421 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 422
5df362a6cf49ca Freeman Liu 2018-06-21 423 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 424 SC27XX_ADC_CHN_RUN, SC27XX_ADC_CHN_RUN);
5df362a6cf49ca Freeman Liu 2018-06-21 425 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 426 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 427
8de877d2bba5c3 Freeman Liu 2019-07-25 428 ret = regmap_read_poll_timeout(data->regmap,
8de877d2bba5c3 Freeman Liu 2019-07-25 429 data->base + SC27XX_ADC_INT_RAW,
8de877d2bba5c3 Freeman Liu 2019-07-25 430 status, (status & SC27XX_ADC_IRQ_RAW),
8de877d2bba5c3 Freeman Liu 2019-07-25 431 SC27XX_ADC_POLL_RAW_STATUS,
8de877d2bba5c3 Freeman Liu 2019-07-25 432 SC27XX_ADC_RDY_TIMEOUT);
8de877d2bba5c3 Freeman Liu 2019-07-25 433 if (ret) {
8de877d2bba5c3 Freeman Liu 2019-07-25 434 dev_err(data->dev, "read adc timeout, status = 0x%x\n", status);
8de877d2bba5c3 Freeman Liu 2019-07-25 435 goto disable_adc;
750ac07eb2c856 Freeman Liu 2018-11-09 436 }
5df362a6cf49ca Freeman Liu 2018-06-21 437
8de877d2bba5c3 Freeman Liu 2019-07-25 438 ret = regmap_read(data->regmap, data->base + SC27XX_ADC_DATA, &value);
8de877d2bba5c3 Freeman Liu 2019-07-25 439 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 440 goto disable_adc;
8de877d2bba5c3 Freeman Liu 2019-07-25 441
8de877d2bba5c3 Freeman Liu 2019-07-25 442 value &= SC27XX_ADC_DATA_MASK;
8de877d2bba5c3 Freeman Liu 2019-07-25 443
5df362a6cf49ca Freeman Liu 2018-06-21 444 disable_adc:
5df362a6cf49ca Freeman Liu 2018-06-21 445 regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 446 SC27XX_ADC_EN, 0);
5df362a6cf49ca Freeman Liu 2018-06-21 447 unlock_adc:
cd6ab0edd81be2 Cixi Geng 2022-01-06 448 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 449 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 450 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 451 SC27XX_ADC_REFVOL_VDD28,
cd6ab0edd81be2 Cixi Geng 2022-01-06 452 SC27XX_ADC_REFVOL_VDD28);
cd6ab0edd81be2 Cixi Geng 2022-01-06 453 if (ret)
cd6ab0edd81be2 Cixi Geng 2022-01-06 454 dev_err(data->dev, "failed to set the volref 2.8V\n");
cd6ab0edd81be2 Cixi Geng 2022-01-06 455 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 456 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 457
5df362a6cf49ca Freeman Liu 2018-06-21 458 hwspin_unlock_raw(data->hwlock);
5df362a6cf49ca Freeman Liu 2018-06-21 459
5df362a6cf49ca Freeman Liu 2018-06-21 460 if (!ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 @461 *val = value;
5df362a6cf49ca Freeman Liu 2018-06-21 462
5df362a6cf49ca Freeman Liu 2018-06-21 463 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 464 }
5df362a6cf49ca Freeman Liu 2018-06-21 465
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620
2022-01-06 12:59 ` [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 Cixi Geng
2022-01-07 7:23 ` Baolin Wang
@ 2022-01-10 6:30 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2022-01-07 19:17 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20220106125947.139523-7-gengcixi@gmail.com>
References: <20220106125947.139523-7-gengcixi@gmail.com>
TO: Cixi Geng <gengcixi@gmail.com>
TO: orsonzhai(a)gmail.com
TO: baolin.wang7(a)gmail.com
TO: zhang.lyra(a)gmail.com
TO: jic23(a)kernel.org
TO: lars(a)metafoo.de
TO: robh+dt(a)kernel.org
TO: lgirdwood(a)gmail.com
TO: broonie(a)kernel.org
CC: yuming.zhu1(a)unisoc.com
CC: linux-iio(a)vger.kernel.org
Hi Cixi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.16-rc8 next-20220106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
:::::: branch date: 30 hours ago
:::::: commit date: 30 hours ago
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080313.70ZrhufR-lkp(a)intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/iio/adc/sc27xx_adc.c:196 adc_nvmem_cell_calib_data() warn: passing zero to 'PTR_ERR'
Old smatch warnings:
drivers/iio/adc/sc27xx_adc.c:201 adc_nvmem_cell_calib_data() warn: passing zero to 'PTR_ERR'
drivers/iio/adc/sc27xx_adc.c:706 sc27xx_adc_read() error: uninitialized symbol 'value'.
drivers/iio/adc/sc27xx_adc.c:1123 sc27xx_adc_probe() warn: passing zero to 'PTR_ERR'
vim +/PTR_ERR +196 drivers/iio/adc/sc27xx_adc.c
8ba0dbfd07a358 Baolin Wang 2018-08-29 183
87da8dcc76b4aa Cixi Geng 2022-01-06 184 static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
87da8dcc76b4aa Cixi Geng 2022-01-06 185 {
87da8dcc76b4aa Cixi Geng 2022-01-06 186 struct nvmem_cell *cell;
87da8dcc76b4aa Cixi Geng 2022-01-06 187 void *buf;
87da8dcc76b4aa Cixi Geng 2022-01-06 188 u32 calib_data = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 189 size_t len = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 190
87da8dcc76b4aa Cixi Geng 2022-01-06 191 if (!data)
87da8dcc76b4aa Cixi Geng 2022-01-06 192 return -EINVAL;
87da8dcc76b4aa Cixi Geng 2022-01-06 193
87da8dcc76b4aa Cixi Geng 2022-01-06 194 cell = nvmem_cell_get(data->dev, cell_name);
87da8dcc76b4aa Cixi Geng 2022-01-06 195 if (IS_ERR_OR_NULL(cell))
87da8dcc76b4aa Cixi Geng 2022-01-06 @196 return PTR_ERR(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 197
87da8dcc76b4aa Cixi Geng 2022-01-06 198 buf = nvmem_cell_read(cell, &len);
87da8dcc76b4aa Cixi Geng 2022-01-06 199 if (IS_ERR_OR_NULL(buf)) {
87da8dcc76b4aa Cixi Geng 2022-01-06 200 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 201 return PTR_ERR(buf);
87da8dcc76b4aa Cixi Geng 2022-01-06 202 }
87da8dcc76b4aa Cixi Geng 2022-01-06 203
87da8dcc76b4aa Cixi Geng 2022-01-06 204 memcpy(&calib_data, buf, min(len, sizeof(u32)));
87da8dcc76b4aa Cixi Geng 2022-01-06 205
87da8dcc76b4aa Cixi Geng 2022-01-06 206 kfree(buf);
87da8dcc76b4aa Cixi Geng 2022-01-06 207 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 208 return calib_data;
87da8dcc76b4aa Cixi Geng 2022-01-06 209 }
87da8dcc76b4aa Cixi Geng 2022-01-06 210
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right
2022-01-07 6:55 ` Baolin Wang
@ 2022-01-09 16:06 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-09 16:06 UTC (permalink / raw)
To: Baolin Wang
Cc: Cixi Geng, Orson Zhai, Chunyan Zhang, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Fri, 7 Jan 2022 14:55:15 +0800
Baolin Wang <baolin.wang7@gmail.com> wrote:
> On Thu, Jan 6, 2022 at 9:00 PM 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>
>
> Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>
Fixes: tag for backports?
or is this having no visible result today?
Jonathan
>
> > ---
> > drivers/iio/adc/sc27xx_adc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index 00098caf6d9e..aee076c8e2b1 100644
> > --- a/drivers/iio/adc/sc27xx_adc.c
> > +++ b/drivers/iio/adc/sc27xx_adc.c
> > @@ -36,8 +36,8 @@
> >
> > /* Bits and mask definition for SC27XX_ADC_CH_CFG register */
> > #define SC27XX_ADC_CHN_ID_MASK GENMASK(4, 0)
> > -#define SC27XX_ADC_SCALE_MASK GENMASK(10, 8)
> > -#define SC27XX_ADC_SCALE_SHIFT 8
> > +#define SC27XX_ADC_SCALE_MASK GENMASK(10, 9)
> > +#define SC27XX_ADC_SCALE_SHIFT 9
> >
> > /* Bits definitions for SC27XX_ADC_INT_EN registers */
> > #define SC27XX_ADC_IRQ_EN BIT(0)
> > --
> > 2.25.1
> >
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
2022-01-07 7:16 ` Baolin Wang
@ 2022-01-09 16:13 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-09 16:13 UTC (permalink / raw)
To: Baolin Wang
Cc: Cixi Geng, Orson Zhai, Chunyan Zhang, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Fri, 7 Jan 2022 15:16:15 +0800
Baolin Wang <baolin.wang7@gmail.com> wrote:
> On Thu, Jan 6, 2022 at 9:00 PM 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>
> > ---
> > drivers/iio/adc/sc27xx_adc.c | 198 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 198 insertions(+)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index d2712e54ee79..7b5c66660ac9 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,21 @@
> > #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 iio_dev *indio_dev;
>
> Why add an unused member?
It's very very rarely a good architecture structure to have
the data stored in iio_priv() have a pointer back to the indio_dev.
Normally it implies somewhere the wrong level of structure is being
passed to a function.
So I'm glad it's not used :)
>
> > struct device *dev;
> > + struct regulator *volref;
> > struct regmap *regmap;
> > /*
> > * One hardware spinlock to synchronize between the multiple
> > @@ -87,6 +104,7 @@ struct sc27xx_adc_data {
> > * in the device data structure.
> > */
...
>
> > +
> > static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
> > {
> > int i;
> > @@ -239,6 +373,24 @@ 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.
That's horrible... :) Ah well...
> > + */
> > + if (data->var_data->pmic_type == SC2721_ADC) {
> > + if ((channel == 30) || (channel == 31)) {
>
> Combine the two branches please.
>
> > + 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)
> > @@ -293,6 +445,16 @@ 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 = regulator_set_voltage(data->volref,
> > + SC27XX_ADC_REFVOL_VDD28,
> > + SC27XX_ADC_REFVOL_VDD28);
> > + if (ret)
> > + dev_err(data->dev, "failed to set the volref 2.8V\n");
> > + }
> > + }
> > +
> > hwspin_unlock_raw(data->hwlock);
> >
> > if (!ret)
> > @@ -522,6 +684,7 @@ static void sc27xx_adc_disable(void *_data)
> > }
...
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support
2022-01-07 7:34 ` Baolin Wang
@ 2022-01-09 16:22 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-09 16:22 UTC (permalink / raw)
To: Baolin Wang
Cc: Cixi Geng, Orson Zhai, Chunyan Zhang, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown, yuming.zhu1, linux-iio,
Devicetree List, LKML
On Fri, 7 Jan 2022 15:34:32 +0800
Baolin Wang <baolin.wang7@gmail.com> wrote:
> On Thu, Jan 6, 2022 at 9:00 PM 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: Cixi Geng <cixi.geng1@unisoc.com>
> > Signed-off-by: Yuming Zhu <yuming.zhu1@unisoc.com>
A few additional comments from me inline,
Thanks,
Jonathan
> > ---
> > drivers/iio/adc/sc27xx_adc.c | 103 ++++++++++++++++++++++++++++++++++-
> > 1 file changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index 68b967f32498..cecda8d53474 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,
> > @@ -618,6 +622,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
> > @@ -700,6 +707,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)
> > @@ -947,6 +959,10 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
> > ret = regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> > UMP9620_XTL_WAIT_CTRL0_EN,
> > UMP9620_XTL_WAIT_CTRL0_EN);
> > + 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 */
> > @@ -988,6 +1004,11 @@ 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)
> > + regmap_update_bits(data->regmap, UMP9620_XTL_WAIT_CTRL0,
> > + UMP9620_XTL_WAIT_CTRL0_EN, 0);
>
> Can you hide this into the pm runtime callbacks?
>
> > +
> > return ret;
> > }
> >
> > @@ -1086,6 +1107,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);
> > @@ -1126,7 +1149,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);
> > @@ -1137,18 +1163,39 @@ static int sc27xx_adc_probe(struct platform_device *pdev)
> >
> > ret = devm_add_action_or_reset(dev, sc27xx_adc_disable, sc27xx_data);
> > if (ret) {
> > + sc27xx_adc_disable(sc27xx_data);
No. That's what the _or_reset() bit of the above call is about. It will have already
called this if the devm registration failed.
> > dev_err(dev, "failed to add ADC disable action\n");
> > 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);
> > + }
> > +
> > 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);
>
> I don't think the pm_runtime_put() is needed, since you did not get
> the counter before, right?
Please try to avoid mixing up devm_ managed cleanup and manual cleanup.
devm_add_action_or_reset() can be used to ensure the pm_runtime_disable
occurs on error and in remove function.
>
> > + pm_runtime_disable(dev);
> > + }
> >
> > return ret;
> > }
> > @@ -1163,11 +1210,65 @@ static const struct of_device_id sc27xx_adc_of_match[] = {
> > };
> > MODULE_DEVICE_TABLE(of, sc27xx_adc_of_match);
> >
> > +static int sc27xx_adc_remove(struct platform_device *pdev)
> > +{
> > + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > + struct sc27xx_adc_data *sc27xx_data = iio_priv(indio_dev);
> > +
> > + if (sc27xx_data->var_data->pmic_type == UMP9620_ADC) {
> > + pm_runtime_put(&pdev->dev);
>
> You did not get the pm count, why put it firstly?
>
> > + pm_runtime_disable(&pdev->dev);
> > +
> > + /* set the UMP9620 ADC clk26m bit8 on IP */
> > + regmap_update_bits(sc27xx_data->regmap, UMP9620_XTL_WAIT_CTRL0,
> > + UMP9620_XTL_WAIT_CTRL0_EN, 0);
Why is this not called in error path of the probe() function?
I suspect because it also doesn't need to be called here as you have it automatically
called in the sc27xx_adc_disable() call during device managed cleanup.
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +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;
>
> no need to initialize 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 = {
> > + .runtime_suspend = &sc27xx_adc_runtime_suspend,
> > + .runtime_resume = &sc27xx_adc_runtime_resume,
> > +};
>
> Please use SET_RUNTIME_PM_OPS macro.
>
> > +
> > static struct platform_driver sc27xx_adc_driver = {
> > .probe = sc27xx_adc_probe,
> > + .remove = sc27xx_adc_remove,
> > .driver = {
> > .name = "sc27xx-adc",
> > .of_match_table = sc27xx_adc_of_match,
> > + .pm = &sc27xx_adc_pm_ops,
> > },
> > };
> >
> > --
> > 2.25.1
> >
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
@ 2022-01-10 5:17 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: Dan Carpenter @ 2022-01-10 5:17 UTC (permalink / raw)
To: kbuild, Cixi Geng, orsonzhai, baolin.wang7, zhang.lyra, jic23,
lars, robh+dt, lgirdwood, broonie
Cc: lkp, kbuild-all, yuming.zhu1, linux-iio
Hi Cixi,
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080030.L51zYw0G-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/iio/adc/sc27xx_adc.c:461 sc27xx_adc_read() error: uninitialized symbol 'value'.
vim +/value +461 drivers/iio/adc/sc27xx_adc.c
5df362a6cf49ca Freeman Liu 2018-06-21 364 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
5df362a6cf49ca Freeman Liu 2018-06-21 365 int scale, int *val)
5df362a6cf49ca Freeman Liu 2018-06-21 366 {
5df362a6cf49ca Freeman Liu 2018-06-21 367 int ret;
8de877d2bba5c3 Freeman Liu 2019-07-25 368 u32 tmp, value, status;
5df362a6cf49ca Freeman Liu 2018-06-21 369
5df362a6cf49ca Freeman Liu 2018-06-21 370 ret = hwspin_lock_timeout_raw(data->hwlock, SC27XX_ADC_HWLOCK_TIMEOUT);
5df362a6cf49ca Freeman Liu 2018-06-21 371 if (ret) {
5df362a6cf49ca Freeman Liu 2018-06-21 372 dev_err(data->dev, "timeout to get the hwspinlock\n");
5df362a6cf49ca Freeman Liu 2018-06-21 373 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 374 }
5df362a6cf49ca Freeman Liu 2018-06-21 375
cd6ab0edd81be2 Cixi Geng 2022-01-06 376 /*
cd6ab0edd81be2 Cixi Geng 2022-01-06 377 * According to the sc2721 chip data sheet, the reference voltage of
cd6ab0edd81be2 Cixi Geng 2022-01-06 378 * specific channel 30 and channel 31 in ADC module needs to be set from
cd6ab0edd81be2 Cixi Geng 2022-01-06 379 * the default 2.8v to 3.5v.
cd6ab0edd81be2 Cixi Geng 2022-01-06 380 */
cd6ab0edd81be2 Cixi Geng 2022-01-06 381 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 382 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 383 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 384 SC27XX_ADC_REFVOL_VDD35,
cd6ab0edd81be2 Cixi Geng 2022-01-06 385 SC27XX_ADC_REFVOL_VDD35);
cd6ab0edd81be2 Cixi Geng 2022-01-06 386 if (ret) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 387 dev_err(data->dev, "failed to set the volref 3.5V\n");
cd6ab0edd81be2 Cixi Geng 2022-01-06 388 hwspin_unlock_raw(data->hwlock);
cd6ab0edd81be2 Cixi Geng 2022-01-06 389 return ret;
cd6ab0edd81be2 Cixi Geng 2022-01-06 390 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 391 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 392 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 393
5df362a6cf49ca Freeman Liu 2018-06-21 394 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 395 SC27XX_ADC_EN, SC27XX_ADC_EN);
5df362a6cf49ca Freeman Liu 2018-06-21 396 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 397 goto unlock_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 398
8de877d2bba5c3 Freeman Liu 2019-07-25 399 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
8de877d2bba5c3 Freeman Liu 2019-07-25 400 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
8de877d2bba5c3 Freeman Liu 2019-07-25 401 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 402 goto disable_adc;
Assume we hit a goto.
8de877d2bba5c3 Freeman Liu 2019-07-25 403
5df362a6cf49ca Freeman Liu 2018-06-21 404 /* Configure the channel id and scale */
b39db3bcbc9a96 Cixi Geng 2022-01-06 405 tmp = (scale << data->var_data->scale_shift) & data->var_data->scale_mask;
5df362a6cf49ca Freeman Liu 2018-06-21 406 tmp |= channel & SC27XX_ADC_CHN_ID_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 407 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CH_CFG,
b39db3bcbc9a96 Cixi Geng 2022-01-06 408 SC27XX_ADC_CHN_ID_MASK |
b39db3bcbc9a96 Cixi Geng 2022-01-06 409 data->var_data->scale_mask,
5df362a6cf49ca Freeman Liu 2018-06-21 410 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 411 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 412 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 413
5df362a6cf49ca Freeman Liu 2018-06-21 414 /* Select 12bit conversion mode, and only sample 1 time */
5df362a6cf49ca Freeman Liu 2018-06-21 415 tmp = SC27XX_ADC_12BIT_MODE;
5df362a6cf49ca Freeman Liu 2018-06-21 416 tmp |= (0 << SC27XX_ADC_RUN_NUM_SHIFT) & SC27XX_ADC_RUN_NUM_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 417 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 418 SC27XX_ADC_RUN_NUM_MASK | SC27XX_ADC_12BIT_MODE,
5df362a6cf49ca Freeman Liu 2018-06-21 419 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 420 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 421 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 422
5df362a6cf49ca Freeman Liu 2018-06-21 423 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 424 SC27XX_ADC_CHN_RUN, SC27XX_ADC_CHN_RUN);
5df362a6cf49ca Freeman Liu 2018-06-21 425 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 426 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 427
8de877d2bba5c3 Freeman Liu 2019-07-25 428 ret = regmap_read_poll_timeout(data->regmap,
8de877d2bba5c3 Freeman Liu 2019-07-25 429 data->base + SC27XX_ADC_INT_RAW,
8de877d2bba5c3 Freeman Liu 2019-07-25 430 status, (status & SC27XX_ADC_IRQ_RAW),
8de877d2bba5c3 Freeman Liu 2019-07-25 431 SC27XX_ADC_POLL_RAW_STATUS,
8de877d2bba5c3 Freeman Liu 2019-07-25 432 SC27XX_ADC_RDY_TIMEOUT);
8de877d2bba5c3 Freeman Liu 2019-07-25 433 if (ret) {
8de877d2bba5c3 Freeman Liu 2019-07-25 434 dev_err(data->dev, "read adc timeout, status = 0x%x\n", status);
8de877d2bba5c3 Freeman Liu 2019-07-25 435 goto disable_adc;
750ac07eb2c856 Freeman Liu 2018-11-09 436 }
5df362a6cf49ca Freeman Liu 2018-06-21 437
8de877d2bba5c3 Freeman Liu 2019-07-25 438 ret = regmap_read(data->regmap, data->base + SC27XX_ADC_DATA, &value);
value is initialized here.
8de877d2bba5c3 Freeman Liu 2019-07-25 439 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 440 goto disable_adc;
8de877d2bba5c3 Freeman Liu 2019-07-25 441
8de877d2bba5c3 Freeman Liu 2019-07-25 442 value &= SC27XX_ADC_DATA_MASK;
8de877d2bba5c3 Freeman Liu 2019-07-25 443
5df362a6cf49ca Freeman Liu 2018-06-21 444 disable_adc:
5df362a6cf49ca Freeman Liu 2018-06-21 445 regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 446 SC27XX_ADC_EN, 0);
5df362a6cf49ca Freeman Liu 2018-06-21 447 unlock_adc:
cd6ab0edd81be2 Cixi Geng 2022-01-06 448 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 449 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 450 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 451 SC27XX_ADC_REFVOL_VDD28,
cd6ab0edd81be2 Cixi Geng 2022-01-06 452 SC27XX_ADC_REFVOL_VDD28);
cd6ab0edd81be2 Cixi Geng 2022-01-06 453 if (ret)
cd6ab0edd81be2 Cixi Geng 2022-01-06 454 dev_err(data->dev, "failed to set the volref 2.8V\n");
ret is reset here.
cd6ab0edd81be2 Cixi Geng 2022-01-06 455 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 456 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 457
5df362a6cf49ca Freeman Liu 2018-06-21 458 hwspin_unlock_raw(data->hwlock);
5df362a6cf49ca Freeman Liu 2018-06-21 459
5df362a6cf49ca Freeman Liu 2018-06-21 460 if (!ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 @461 *val = value;
Since ret is reset, it is no long connected to value.
5df362a6cf49ca Freeman Liu 2018-06-21 462
5df362a6cf49ca Freeman Liu 2018-06-21 463 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 464 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721
@ 2022-01-10 5:17 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: Dan Carpenter @ 2022-01-10 5:17 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 8375 bytes --]
Hi Cixi,
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080030.L51zYw0G-lkp(a)intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/iio/adc/sc27xx_adc.c:461 sc27xx_adc_read() error: uninitialized symbol 'value'.
vim +/value +461 drivers/iio/adc/sc27xx_adc.c
5df362a6cf49ca Freeman Liu 2018-06-21 364 static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
5df362a6cf49ca Freeman Liu 2018-06-21 365 int scale, int *val)
5df362a6cf49ca Freeman Liu 2018-06-21 366 {
5df362a6cf49ca Freeman Liu 2018-06-21 367 int ret;
8de877d2bba5c3 Freeman Liu 2019-07-25 368 u32 tmp, value, status;
5df362a6cf49ca Freeman Liu 2018-06-21 369
5df362a6cf49ca Freeman Liu 2018-06-21 370 ret = hwspin_lock_timeout_raw(data->hwlock, SC27XX_ADC_HWLOCK_TIMEOUT);
5df362a6cf49ca Freeman Liu 2018-06-21 371 if (ret) {
5df362a6cf49ca Freeman Liu 2018-06-21 372 dev_err(data->dev, "timeout to get the hwspinlock\n");
5df362a6cf49ca Freeman Liu 2018-06-21 373 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 374 }
5df362a6cf49ca Freeman Liu 2018-06-21 375
cd6ab0edd81be2 Cixi Geng 2022-01-06 376 /*
cd6ab0edd81be2 Cixi Geng 2022-01-06 377 * According to the sc2721 chip data sheet, the reference voltage of
cd6ab0edd81be2 Cixi Geng 2022-01-06 378 * specific channel 30 and channel 31 in ADC module needs to be set from
cd6ab0edd81be2 Cixi Geng 2022-01-06 379 * the default 2.8v to 3.5v.
cd6ab0edd81be2 Cixi Geng 2022-01-06 380 */
cd6ab0edd81be2 Cixi Geng 2022-01-06 381 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 382 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 383 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 384 SC27XX_ADC_REFVOL_VDD35,
cd6ab0edd81be2 Cixi Geng 2022-01-06 385 SC27XX_ADC_REFVOL_VDD35);
cd6ab0edd81be2 Cixi Geng 2022-01-06 386 if (ret) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 387 dev_err(data->dev, "failed to set the volref 3.5V\n");
cd6ab0edd81be2 Cixi Geng 2022-01-06 388 hwspin_unlock_raw(data->hwlock);
cd6ab0edd81be2 Cixi Geng 2022-01-06 389 return ret;
cd6ab0edd81be2 Cixi Geng 2022-01-06 390 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 391 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 392 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 393
5df362a6cf49ca Freeman Liu 2018-06-21 394 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 395 SC27XX_ADC_EN, SC27XX_ADC_EN);
5df362a6cf49ca Freeman Liu 2018-06-21 396 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 397 goto unlock_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 398
8de877d2bba5c3 Freeman Liu 2019-07-25 399 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
8de877d2bba5c3 Freeman Liu 2019-07-25 400 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
8de877d2bba5c3 Freeman Liu 2019-07-25 401 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 402 goto disable_adc;
Assume we hit a goto.
8de877d2bba5c3 Freeman Liu 2019-07-25 403
5df362a6cf49ca Freeman Liu 2018-06-21 404 /* Configure the channel id and scale */
b39db3bcbc9a96 Cixi Geng 2022-01-06 405 tmp = (scale << data->var_data->scale_shift) & data->var_data->scale_mask;
5df362a6cf49ca Freeman Liu 2018-06-21 406 tmp |= channel & SC27XX_ADC_CHN_ID_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 407 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CH_CFG,
b39db3bcbc9a96 Cixi Geng 2022-01-06 408 SC27XX_ADC_CHN_ID_MASK |
b39db3bcbc9a96 Cixi Geng 2022-01-06 409 data->var_data->scale_mask,
5df362a6cf49ca Freeman Liu 2018-06-21 410 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 411 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 412 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 413
5df362a6cf49ca Freeman Liu 2018-06-21 414 /* Select 12bit conversion mode, and only sample 1 time */
5df362a6cf49ca Freeman Liu 2018-06-21 415 tmp = SC27XX_ADC_12BIT_MODE;
5df362a6cf49ca Freeman Liu 2018-06-21 416 tmp |= (0 << SC27XX_ADC_RUN_NUM_SHIFT) & SC27XX_ADC_RUN_NUM_MASK;
5df362a6cf49ca Freeman Liu 2018-06-21 417 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 418 SC27XX_ADC_RUN_NUM_MASK | SC27XX_ADC_12BIT_MODE,
5df362a6cf49ca Freeman Liu 2018-06-21 419 tmp);
5df362a6cf49ca Freeman Liu 2018-06-21 420 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 421 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 422
5df362a6cf49ca Freeman Liu 2018-06-21 423 ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 424 SC27XX_ADC_CHN_RUN, SC27XX_ADC_CHN_RUN);
5df362a6cf49ca Freeman Liu 2018-06-21 425 if (ret)
5df362a6cf49ca Freeman Liu 2018-06-21 426 goto disable_adc;
5df362a6cf49ca Freeman Liu 2018-06-21 427
8de877d2bba5c3 Freeman Liu 2019-07-25 428 ret = regmap_read_poll_timeout(data->regmap,
8de877d2bba5c3 Freeman Liu 2019-07-25 429 data->base + SC27XX_ADC_INT_RAW,
8de877d2bba5c3 Freeman Liu 2019-07-25 430 status, (status & SC27XX_ADC_IRQ_RAW),
8de877d2bba5c3 Freeman Liu 2019-07-25 431 SC27XX_ADC_POLL_RAW_STATUS,
8de877d2bba5c3 Freeman Liu 2019-07-25 432 SC27XX_ADC_RDY_TIMEOUT);
8de877d2bba5c3 Freeman Liu 2019-07-25 433 if (ret) {
8de877d2bba5c3 Freeman Liu 2019-07-25 434 dev_err(data->dev, "read adc timeout, status = 0x%x\n", status);
8de877d2bba5c3 Freeman Liu 2019-07-25 435 goto disable_adc;
750ac07eb2c856 Freeman Liu 2018-11-09 436 }
5df362a6cf49ca Freeman Liu 2018-06-21 437
8de877d2bba5c3 Freeman Liu 2019-07-25 438 ret = regmap_read(data->regmap, data->base + SC27XX_ADC_DATA, &value);
value is initialized here.
8de877d2bba5c3 Freeman Liu 2019-07-25 439 if (ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 440 goto disable_adc;
8de877d2bba5c3 Freeman Liu 2019-07-25 441
8de877d2bba5c3 Freeman Liu 2019-07-25 442 value &= SC27XX_ADC_DATA_MASK;
8de877d2bba5c3 Freeman Liu 2019-07-25 443
5df362a6cf49ca Freeman Liu 2018-06-21 444 disable_adc:
5df362a6cf49ca Freeman Liu 2018-06-21 445 regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
5df362a6cf49ca Freeman Liu 2018-06-21 446 SC27XX_ADC_EN, 0);
5df362a6cf49ca Freeman Liu 2018-06-21 447 unlock_adc:
cd6ab0edd81be2 Cixi Geng 2022-01-06 448 if (data->var_data->pmic_type == SC2721_ADC) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 449 if ((channel == 30) || (channel == 31)) {
cd6ab0edd81be2 Cixi Geng 2022-01-06 450 ret = regulator_set_voltage(data->volref,
cd6ab0edd81be2 Cixi Geng 2022-01-06 451 SC27XX_ADC_REFVOL_VDD28,
cd6ab0edd81be2 Cixi Geng 2022-01-06 452 SC27XX_ADC_REFVOL_VDD28);
cd6ab0edd81be2 Cixi Geng 2022-01-06 453 if (ret)
cd6ab0edd81be2 Cixi Geng 2022-01-06 454 dev_err(data->dev, "failed to set the volref 2.8V\n");
ret is reset here.
cd6ab0edd81be2 Cixi Geng 2022-01-06 455 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 456 }
cd6ab0edd81be2 Cixi Geng 2022-01-06 457
5df362a6cf49ca Freeman Liu 2018-06-21 458 hwspin_unlock_raw(data->hwlock);
5df362a6cf49ca Freeman Liu 2018-06-21 459
5df362a6cf49ca Freeman Liu 2018-06-21 460 if (!ret)
8de877d2bba5c3 Freeman Liu 2019-07-25 @461 *val = value;
Since ret is reset, it is no long connected to value.
5df362a6cf49ca Freeman Liu 2018-06-21 462
5df362a6cf49ca Freeman Liu 2018-06-21 463 return ret;
5df362a6cf49ca Freeman Liu 2018-06-21 464 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620
@ 2022-01-10 6:30 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: Dan Carpenter @ 2022-01-10 6:30 UTC (permalink / raw)
To: kbuild, Cixi Geng, orsonzhai, baolin.wang7, zhang.lyra, jic23,
lars, robh+dt, lgirdwood, broonie
Cc: lkp, kbuild-all, yuming.zhu1, linux-iio
Hi Cixi,
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080313.70ZrhufR-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/iio/adc/sc27xx_adc.c:196 adc_nvmem_cell_calib_data() warn: passing zero to 'PTR_ERR'
vim +/PTR_ERR +196 drivers/iio/adc/sc27xx_adc.c
87da8dcc76b4aa Cixi Geng 2022-01-06 184 static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
87da8dcc76b4aa Cixi Geng 2022-01-06 185 {
87da8dcc76b4aa Cixi Geng 2022-01-06 186 struct nvmem_cell *cell;
87da8dcc76b4aa Cixi Geng 2022-01-06 187 void *buf;
87da8dcc76b4aa Cixi Geng 2022-01-06 188 u32 calib_data = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 189 size_t len = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 190
87da8dcc76b4aa Cixi Geng 2022-01-06 191 if (!data)
87da8dcc76b4aa Cixi Geng 2022-01-06 192 return -EINVAL;
87da8dcc76b4aa Cixi Geng 2022-01-06 193
87da8dcc76b4aa Cixi Geng 2022-01-06 194 cell = nvmem_cell_get(data->dev, cell_name);
87da8dcc76b4aa Cixi Geng 2022-01-06 195 if (IS_ERR_OR_NULL(cell))
87da8dcc76b4aa Cixi Geng 2022-01-06 @196 return PTR_ERR(cell);
When functions return a both error pointers and NULL, then the NULL
return means that the feature has been deliberately disabled by the
admin.
nvmem_cell_get() cannot be disabled and never returns NULL so this code
should just be:
if (IS_ERR(cell))
return PTR_ERR(cell);
Otherwise we have to create the whole infrastructure to disable it. We
can't just create random sprinklings of dead code to half way support
disabling the feature.
87da8dcc76b4aa Cixi Geng 2022-01-06 197
87da8dcc76b4aa Cixi Geng 2022-01-06 198 buf = nvmem_cell_read(cell, &len);
87da8dcc76b4aa Cixi Geng 2022-01-06 199 if (IS_ERR_OR_NULL(buf)) {
87da8dcc76b4aa Cixi Geng 2022-01-06 200 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 201 return PTR_ERR(buf);
Same.
87da8dcc76b4aa Cixi Geng 2022-01-06 202 }
87da8dcc76b4aa Cixi Geng 2022-01-06 203
87da8dcc76b4aa Cixi Geng 2022-01-06 204 memcpy(&calib_data, buf, min(len, sizeof(u32)));
87da8dcc76b4aa Cixi Geng 2022-01-06 205
87da8dcc76b4aa Cixi Geng 2022-01-06 206 kfree(buf);
87da8dcc76b4aa Cixi Geng 2022-01-06 207 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 208 return calib_data;
87da8dcc76b4aa Cixi Geng 2022-01-06 209 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620
@ 2022-01-10 6:30 ` Dan Carpenter
0 siblings, 0 replies; 30+ messages in thread
From: Dan Carpenter @ 2022-01-10 6:30 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3020 bytes --]
Hi Cixi,
url: https://github.com/0day-ci/linux/commits/Cixi-Geng/iio-adc-sc27xx-adjust-structure-and-add-PMIC-s-support/20220106-210151
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: openrisc-randconfig-m031-20220106 (https://download.01.org/0day-ci/archive/20220108/202201080313.70ZrhufR-lkp(a)intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/iio/adc/sc27xx_adc.c:196 adc_nvmem_cell_calib_data() warn: passing zero to 'PTR_ERR'
vim +/PTR_ERR +196 drivers/iio/adc/sc27xx_adc.c
87da8dcc76b4aa Cixi Geng 2022-01-06 184 static int adc_nvmem_cell_calib_data(struct sc27xx_adc_data *data, const char *cell_name)
87da8dcc76b4aa Cixi Geng 2022-01-06 185 {
87da8dcc76b4aa Cixi Geng 2022-01-06 186 struct nvmem_cell *cell;
87da8dcc76b4aa Cixi Geng 2022-01-06 187 void *buf;
87da8dcc76b4aa Cixi Geng 2022-01-06 188 u32 calib_data = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 189 size_t len = 0;
87da8dcc76b4aa Cixi Geng 2022-01-06 190
87da8dcc76b4aa Cixi Geng 2022-01-06 191 if (!data)
87da8dcc76b4aa Cixi Geng 2022-01-06 192 return -EINVAL;
87da8dcc76b4aa Cixi Geng 2022-01-06 193
87da8dcc76b4aa Cixi Geng 2022-01-06 194 cell = nvmem_cell_get(data->dev, cell_name);
87da8dcc76b4aa Cixi Geng 2022-01-06 195 if (IS_ERR_OR_NULL(cell))
87da8dcc76b4aa Cixi Geng 2022-01-06 @196 return PTR_ERR(cell);
When functions return a both error pointers and NULL, then the NULL
return means that the feature has been deliberately disabled by the
admin.
nvmem_cell_get() cannot be disabled and never returns NULL so this code
should just be:
if (IS_ERR(cell))
return PTR_ERR(cell);
Otherwise we have to create the whole infrastructure to disable it. We
can't just create random sprinklings of dead code to half way support
disabling the feature.
87da8dcc76b4aa Cixi Geng 2022-01-06 197
87da8dcc76b4aa Cixi Geng 2022-01-06 198 buf = nvmem_cell_read(cell, &len);
87da8dcc76b4aa Cixi Geng 2022-01-06 199 if (IS_ERR_OR_NULL(buf)) {
87da8dcc76b4aa Cixi Geng 2022-01-06 200 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 201 return PTR_ERR(buf);
Same.
87da8dcc76b4aa Cixi Geng 2022-01-06 202 }
87da8dcc76b4aa Cixi Geng 2022-01-06 203
87da8dcc76b4aa Cixi Geng 2022-01-06 204 memcpy(&calib_data, buf, min(len, sizeof(u32)));
87da8dcc76b4aa Cixi Geng 2022-01-06 205
87da8dcc76b4aa Cixi Geng 2022-01-06 206 kfree(buf);
87da8dcc76b4aa Cixi Geng 2022-01-06 207 nvmem_cell_put(cell);
87da8dcc76b4aa Cixi Geng 2022-01-06 208 return calib_data;
87da8dcc76b4aa Cixi Geng 2022-01-06 209 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-07 7:04 ` Baolin Wang
@ 2022-01-13 1:53 ` Cixi Geng
2022-01-17 6:16 ` Baolin Wang
0 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-13 1:53 UTC (permalink / raw)
To: Baolin Wang
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
>
> On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > 1 file changed, 79 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > 100, 341,
> > };
> >
> > +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,
> > +};
>
> The original big_scale_graph_calib and small_scale_graph_calib are for
> SC2731 PMIC, why add new structure definition for SC2731?
>
> > +
> > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > 4200, 856,
> > 3600, 733,
> > @@ -130,11 +157,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 +187,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 +212,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;
> > + }
> > +}
>
> This is unnecessary I think, please see sc27xx_adc_write_raw() that
> can set the channel scale.
Did you mean that all the PMIC's scale_init function should put into
the sc27xx_adc_write_raw?
but the scale_init is all different by each PMIC, if implemented in
the write_raw, will add a lot of
if or switch_case branch
>
> > +
> > static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
> > int scale, int *val)
> > {
> > @@ -208,10 +250,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 +305,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 +476,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 +500,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 +514,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 +582,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 +610,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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-13 1:53 ` Cixi Geng
@ 2022-01-17 6:16 ` Baolin Wang
2022-01-24 8:06 ` Cixi Geng
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-01-17 6:16 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
>
> Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> >
> > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > 100, 341,
> > > };
> > >
> > > +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,
> > > +};
> >
> > The original big_scale_graph_calib and small_scale_graph_calib are for
> > SC2731 PMIC, why add new structure definition for SC2731?
> >
> > > +
> > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > 4200, 856,
> > > 3600, 733,
> > > @@ -130,11 +157,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 +187,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 +212,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;
> > > + }
> > > +}
> >
> > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > can set the channel scale.
> Did you mean that all the PMIC's scale_init function should put into
> the sc27xx_adc_write_raw?
No.
> but the scale_init is all different by each PMIC, if implemented in
> the write_raw, will add a lot of
> if or switch_case branch
What I mean is we should follow the original method to set the channel
scale by iio_info. Please also refer to other drivers how ot handle
the channel scale.
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-17 6:16 ` Baolin Wang
@ 2022-01-24 8:06 ` Cixi Geng
2022-02-10 8:08 ` Baolin Wang
0 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-01-24 8:06 UTC (permalink / raw)
To: Baolin Wang
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
Baolin Wang <baolin.wang7@gmail.com> 于2022年1月17日周一 14:15写道：
>
> On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
> >
> > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> > >
> > > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > > 100, 341,
> > > > };
> > > >
> > > > +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,
> > > > +};
> > >
> > > The original big_scale_graph_calib and small_scale_graph_calib are for
> > > SC2731 PMIC, why add new structure definition for SC2731?
> > >
> > > > +
> > > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > > 4200, 856,
> > > > 3600, 733,
> > > > @@ -130,11 +157,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 +187,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 +212,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;
> > > > + }
> > > > +}
> > >
> > > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > > can set the channel scale.
> > Did you mean that all the PMIC's scale_init function should put into
> > the sc27xx_adc_write_raw?
>
> No.
>
> > but the scale_init is all different by each PMIC, if implemented in
> > the write_raw, will add a lot of
> > if or switch_case branch
>
> What I mean is we should follow the original method to set the channel
> scale by iio_info. Please also refer to other drivers how ot handle
> the channel scale.
Hi Baolin, I understand the adc_write_raw() function is the method to set
channal scale for the userspace, we can change the channel scale by write
a value on a user code. did i understand right?
out scale_init is to set scale value when the driver probe stage, and I also
did not found other adc driver use the adc_write_raw() during the driver
initialization phase.
>
> --
> Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-01-24 8:06 ` Cixi Geng
@ 2022-02-10 8:08 ` Baolin Wang
2022-02-23 12:46 ` Cixi Geng
0 siblings, 1 reply; 30+ messages in thread
From: Baolin Wang @ 2022-02-10 8:08 UTC (permalink / raw)
To: Cixi Geng
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
On Mon, Jan 24, 2022 at 4:07 PM Cixi Geng <gengcixi@gmail.com> wrote:
>
> Baolin Wang <baolin.wang7@gmail.com> 于2022年1月17日周一 14:15写道：
> >
> > On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
> > >
> > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> > > >
> > > > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > > > 100, 341,
> > > > > };
> > > > >
> > > > > +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,
> > > > > +};
> > > >
> > > > The original big_scale_graph_calib and small_scale_graph_calib are for
> > > > SC2731 PMIC, why add new structure definition for SC2731?
> > > >
> > > > > +
> > > > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > > > 4200, 856,
> > > > > 3600, 733,
> > > > > @@ -130,11 +157,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 +187,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 +212,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;
> > > > > + }
> > > > > +}
> > > >
> > > > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > > > can set the channel scale.
> > > Did you mean that all the PMIC's scale_init function should put into
> > > the sc27xx_adc_write_raw?
> >
> > No.
> >
> > > but the scale_init is all different by each PMIC, if implemented in
> > > the write_raw, will add a lot of
> > > if or switch_case branch
> >
> > What I mean is we should follow the original method to set the channel
> > scale by iio_info. Please also refer to other drivers how ot handle
> > the channel scale.
> Hi Baolin, I understand the adc_write_raw() function is the method to set
> channal scale for the userspace, we can change the channel scale by write
> a value on a user code. did i understand right?
> out scale_init is to set scale value when the driver probe stage, and I also
> did not found other adc driver use the adc_write_raw() during the driver
> initialization phase.
Hi Jonathan,
How do you think about the method in this patch to set the channel
scale? Thanks.
--
Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-02-10 8:08 ` Baolin Wang
@ 2022-02-23 12:46 ` Cixi Geng
2022-02-25 10:19 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Cixi Geng @ 2022-02-23 12:46 UTC (permalink / raw)
To: Baolin Wang
Cc: Orson Zhai, Chunyan Zhang, jic23, Lars-Peter Clausen,
Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
Baolin Wang <baolin.wang7@gmail.com> 于2022年2月10日周四 16:07写道：
>
> On Mon, Jan 24, 2022 at 4:07 PM Cixi Geng <gengcixi@gmail.com> wrote:
> >
> > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月17日周一 14:15写道：
> > >
> > > On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
> > > >
> > > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> > > > >
> > > > > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > > > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > > > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > > > > 100, 341,
> > > > > > };
> > > > > >
> > > > > > +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,
> > > > > > +};
> > > > >
> > > > > The original big_scale_graph_calib and small_scale_graph_calib are for
> > > > > SC2731 PMIC, why add new structure definition for SC2731?
> > > > >
> > > > > > +
> > > > > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > > > > 4200, 856,
> > > > > > 3600, 733,
> > > > > > @@ -130,11 +157,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 +187,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 +212,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;
> > > > > > + }
> > > > > > +}
> > > > >
> > > > > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > > > > can set the channel scale.
> > > > Did you mean that all the PMIC's scale_init function should put into
> > > > the sc27xx_adc_write_raw?
> > >
> > > No.
> > >
> > > > but the scale_init is all different by each PMIC, if implemented in
> > > > the write_raw, will add a lot of
> > > > if or switch_case branch
> > >
> > > What I mean is we should follow the original method to set the channel
> > > scale by iio_info. Please also refer to other drivers how ot handle
> > > the channel scale.
> > Hi Baolin, I understand the adc_write_raw() function is the method to set
> > channal scale for the userspace, we can change the channel scale by write
> > a value on a user code. did i understand right?
> > out scale_init is to set scale value when the driver probe stage, and I also
> > did not found other adc driver use the adc_write_raw() during the driver
> > initialization phase.
>
> Hi Jonathan,
>
> How do you think about the method in this patch to set the channel
> scale? Thanks.
>
Hi Jonathan,
Could you have a loot at this patch ,and give some advice about the
method to set the channel scale? Thanks very much.
> --
> Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-02-23 12:46 ` Cixi Geng
@ 2022-02-25 10:19 ` Jonathan Cameron
2022-03-01 6:27 ` Cixi Geng
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2022-02-25 10:19 UTC (permalink / raw)
To: Cixi Geng
Cc: Baolin Wang, Orson Zhai, Chunyan Zhang, jic23,
Lars-Peter Clausen, Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457) ,
linux-iio, Devicetree List, LKML
On Wed, 23 Feb 2022 20:46:08 +0800
Cixi Geng <gengcixi@gmail.com> wrote:
> Baolin Wang <baolin.wang7@gmail.com> 于2022年2月10日周四 16:07写道：
> >
> > On Mon, Jan 24, 2022 at 4:07 PM Cixi Geng <gengcixi@gmail.com> wrote:
> > >
> > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月17日周一 14:15写道：
> > > >
> > > > On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
> > > > >
> > > > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> > > > > >
> > > > > > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > > > > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > > > > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > > > > > 100, 341,
> > > > > > > };
> > > > > > >
> > > > > > > +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,
> > > > > > > +};
> > > > > >
> > > > > > The original big_scale_graph_calib and small_scale_graph_calib are for
> > > > > > SC2731 PMIC, why add new structure definition for SC2731?
> > > > > >
> > > > > > > +
> > > > > > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > > > > > 4200, 856,
> > > > > > > 3600, 733,
> > > > > > > @@ -130,11 +157,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 +187,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 +212,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;
> > > > > > > + }
> > > > > > > +}
> > > > > >
> > > > > > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > > > > > can set the channel scale.
> > > > > Did you mean that all the PMIC's scale_init function should put into
> > > > > the sc27xx_adc_write_raw?
> > > >
> > > > No.
> > > >
> > > > > but the scale_init is all different by each PMIC, if implemented in
> > > > > the write_raw, will add a lot of
> > > > > if or switch_case branch
> > > >
> > > > What I mean is we should follow the original method to set the channel
> > > > scale by iio_info. Please also refer to other drivers how ot handle
> > > > the channel scale.
> > > Hi Baolin, I understand the adc_write_raw() function is the method to set
> > > channal scale for the userspace, we can change the channel scale by write
> > > a value on a user code. did i understand right?
> > > out scale_init is to set scale value when the driver probe stage, and I also
> > > did not found other adc driver use the adc_write_raw() during the driver
> > > initialization phase.
> >
> > Hi Jonathan,
> >
> > How do you think about the method in this patch to set the channel
> > scale? Thanks.
> >
> Hi Jonathan,
> Could you have a loot at this patch ,and give some advice about the
> method to set the channel scale? Thanks very much.
Hi, thanks for poking me on this - I'd missed the question buried deep in the thread!
Anyhow, I don't quite follow the discussion but think it could be focused
on one of 2 questions...
1) Does setting an initial default make sense?
Yes, this is an acceptable thing to do if there is a particular set of defaults
and there is no risk of regressions (i.e. the device wasn't previously supported
with different defaults).
2) Should you use the write_raw callback to actually do the setting?
Probably not as it has a set of parameters that don't make as much sense from within
the driver. It 'might' make sense to have a common _set() function for this
feature which is called both in this initialization case and from the write_raw()
function however as that could do bounds checking etc in one common place.
However, it is very simple here, so perhaps not necessary.
Jonathan
> > --
> > Baolin Wang
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
2022-02-25 10:19 ` Jonathan Cameron
@ 2022-03-01 6:27 ` Cixi Geng
0 siblings, 0 replies; 30+ messages in thread
From: Cixi Geng @ 2022-03-01 6:27 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Baolin Wang, Orson Zhai, Chunyan Zhang, jic23,
Lars-Peter Clausen, Rob Herring, lgirdwood, Mark Brown,
朱玉明 (Yuming Zhu/11457),
linux-iio, Devicetree List, LKML
Jonathan Cameron <Jonathan.Cameron@huawei.com> 于2022年2月25日周五 18:19写道：
>
> On Wed, 23 Feb 2022 20:46:08 +0800
> Cixi Geng <gengcixi@gmail.com> wrote:
>
> > Baolin Wang <baolin.wang7@gmail.com> 于2022年2月10日周四 16:07写道：
> > >
> > > On Mon, Jan 24, 2022 at 4:07 PM Cixi Geng <gengcixi@gmail.com> wrote:
> > > >
> > > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月17日周一 14:15写道：
> > > > >
> > > > > On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@gmail.com> wrote:
> > > > > >
> > > > > > Baolin Wang <baolin.wang7@gmail.com> 于2022年1月7日周五 15:03写道：
> > > > > > >
> > > > > > > On Thu, Jan 6, 2022 at 9:00 PM 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 | 94 ++++++++++++++++++++++++++++++------
> > > > > > > > 1 file changed, 79 insertions(+), 15 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > > > > > > index aee076c8e2b1..d2712e54ee79 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,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > > > > > > > 100, 341,
> > > > > > > > };
> > > > > > > >
> > > > > > > > +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,
> > > > > > > > +};
> > > > > > >
> > > > > > > The original big_scale_graph_calib and small_scale_graph_calib are for
> > > > > > > SC2731 PMIC, why add new structure definition for SC2731?
> > > > > > >
> > > > > > > > +
> > > > > > > > static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > > > > > > > 4200, 856,
> > > > > > > > 3600, 733,
> > > > > > > > @@ -130,11 +157,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 +187,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 +212,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;
> > > > > > > > + }
> > > > > > > > +}
> > > > > > >
> > > > > > > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > > > > > > can set the channel scale.
> > > > > > Did you mean that all the PMIC's scale_init function should put into
> > > > > > the sc27xx_adc_write_raw?
> > > > >
> > > > > No.
> > > > >
> > > > > > but the scale_init is all different by each PMIC, if implemented in
> > > > > > the write_raw, will add a lot of
> > > > > > if or switch_case branch
> > > > >
> > > > > What I mean is we should follow the original method to set the channel
> > > > > scale by iio_info. Please also refer to other drivers how ot handle
> > > > > the channel scale.
> > > > Hi Baolin, I understand the adc_write_raw() function is the method to set
> > > > channal scale for the userspace, we can change the channel scale by write
> > > > a value on a user code. did i understand right?
> > > > out scale_init is to set scale value when the driver probe stage, and I also
> > > > did not found other adc driver use the adc_write_raw() during the driver
> > > > initialization phase.
> > >
> > > Hi Jonathan,
> > >
> > > How do you think about the method in this patch to set the channel
> > > scale? Thanks.
> > >
> > Hi Jonathan,
> > Could you have a loot at this patch ,and give some advice about the
> > method to set the channel scale? Thanks very much.
>
> Hi, thanks for poking me on this - I'd missed the question buried deep in the thread!
>
> Anyhow, I don't quite follow the discussion but think it could be focused
> on one of 2 questions...
>
> 1) Does setting an initial default make sense?
> Yes, this is an acceptable thing to do if there is a particular set of defaults
> and there is no risk of regressions (i.e. the device wasn't previously supported
> with different defaults).
> 2) Should you use the write_raw callback to actually do the setting?
> Probably not as it has a set of parameters that don't make as much sense from within
> the driver. It 'might' make sense to have a common _set() function for this
> feature which is called both in this initialization case and from the write_raw()
> function however as that could do bounds checking etc in one common place.
> However, it is very simple here, so perhaps not necessary.
>
> Jonathan
>
Hi Jonathan, thanks for your comment !
And Baolin, I will send a new verision for the patches tto keep the
scale_init and
fix other issues . thanks!
> > > --
> > > Baolin Wang
>
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2022-03-01 6:28 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06 12:59 [PATCH 0/7] iio: adc: sc27xx: adjust structure and add PMIC's support Cixi Geng
2022-01-06 12:59 ` [PATCH 1/7] dt-bindings:iio:adc: add sprd,ump9620-adc dtbindings Cixi Geng
2022-01-06 17:39 ` Rob Herring
2022-01-06 12:59 ` [PATCH 2/7] iio: adc: sc27xx: fix read big scale voltage not right Cixi Geng
2022-01-07 6:55 ` Baolin Wang
2022-01-09 16:06 ` Jonathan Cameron
2022-01-06 12:59 ` [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization Cixi Geng
2022-01-07 7:04 ` Baolin Wang
2022-01-13 1:53 ` Cixi Geng
2022-01-17 6:16 ` Baolin Wang
2022-01-24 8:06 ` Cixi Geng
2022-02-10 8:08 ` Baolin Wang
2022-02-23 12:46 ` Cixi Geng
2022-02-25 10:19 ` Jonathan Cameron
2022-03-01 6:27 ` Cixi Geng
2022-01-06 12:59 ` [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 Cixi Geng
2022-01-07 7:16 ` Baolin Wang
2022-01-09 16:13 ` Jonathan Cameron
2022-01-06 12:59 ` [PATCH 5/7] iio: adc: sc27xx: add support for PMIC sc2730 Cixi Geng
2022-01-06 12:59 ` [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 Cixi Geng
2022-01-07 7:23 ` Baolin Wang
2022-01-06 12:59 ` [PATCH 7/7] iio: adc: sc27xx: add Ump9620 ADC suspend and resume pm support Cixi Geng
2022-01-07 7:34 ` Baolin Wang
2022-01-09 16:22 ` Jonathan Cameron
2022-01-07 16:21 [PATCH 4/7] iio: adc: sc27xx: add support for PMIC sc2720 and sc2721 kernel test robot
2022-01-10 5:17 ` Dan Carpenter
2022-01-10 5:17 ` Dan Carpenter
2022-01-07 19:17 [PATCH 6/7] iio: adc: sc27xx: add support for PMIC ump9620 kernel test robot
2022-01-10 6:30 ` Dan Carpenter
2022-01-10 6:30 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.