From: Baolin Wang <baolin.wang7@gmail.com>
To: Cixi Geng <gengcixi@gmail.com>
Cc: Orson Zhai <orsonzhai@gmail.com>,
Chunyan Zhang <zhang.lyra@gmail.com>,
jic23@kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
Rob Herring <robh+dt@kernel.org>,
lgirdwood@gmail.com, Mark Brown <broonie@kernel.org>,
yuming.zhu1@unisoc.com, linux-iio@vger.kernel.org,
Devicetree List <devicetree@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization
Date: Fri, 7 Jan 2022 15:04:35 +0800 [thread overview]
Message-ID: <CADBw62pBCdrbRspTV9Yck4DP8DE=ECGmEtD74NOtm1YRT3DM8w@mail.gmail.com> (raw)
In-Reply-To: <20220106125947.139523-4-gengcixi@gmail.com>
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
next prev parent reply other threads:[~2022-01-07 7:03 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CADBw62pBCdrbRspTV9Yck4DP8DE=ECGmEtD74NOtm1YRT3DM8w@mail.gmail.com' \
--to=baolin.wang7@gmail.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gengcixi@gmail.com \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=orsonzhai@gmail.com \
--cc=robh+dt@kernel.org \
--cc=yuming.zhu1@unisoc.com \
--cc=zhang.lyra@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.