2023-04-06 03:22:53

by ChiaEn Wu

[permalink] [raw]
Subject: [PATCH v2] iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips

The scale value of ibus and ibat on the datasheet is incorrect due to the
customer report after the experimentation with some specific vendor ID
chips.

Signed-off-by: ChiaEn Wu <[email protected]>
---
v2:
- Move 'get_vendor_info' after iio_priv allocation done
- For the special case, bypass indent, make it out-of-line
- For a simple return, add a blank line
- Drop else
---
drivers/iio/adc/mt6370-adc.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/mt6370-adc.c b/drivers/iio/adc/mt6370-adc.c
index bc62e5a..ae5cdff 100644
--- a/drivers/iio/adc/mt6370-adc.c
+++ b/drivers/iio/adc/mt6370-adc.c
@@ -19,6 +19,7 @@

#include <dt-bindings/iio/adc/mediatek,mt6370_adc.h>

+#define MT6370_REG_DEV_INFO 0x100
#define MT6370_REG_CHG_CTRL3 0x113
#define MT6370_REG_CHG_CTRL7 0x117
#define MT6370_REG_CHG_ADC 0x121
@@ -27,6 +28,7 @@
#define MT6370_ADC_START_MASK BIT(0)
#define MT6370_ADC_IN_SEL_MASK GENMASK(7, 4)
#define MT6370_AICR_ICHG_MASK GENMASK(7, 2)
+#define MT6370_VENID_MASK GENMASK(7, 4)

#define MT6370_AICR_100_mA 0x0
#define MT6370_AICR_150_mA 0x1
@@ -47,6 +49,10 @@
#define ADC_CONV_TIME_MS 35
#define ADC_CONV_POLLING_TIME_US 1000

+#define MT6370_VID_RT5081 0x8
+#define MT6370_VID_RT5081A 0xA
+#define MT6370_VID_MT6370 0xE
+
struct mt6370_adc_data {
struct device *dev;
struct regmap *regmap;
@@ -55,6 +61,7 @@ struct mt6370_adc_data {
* from being read at the same time.
*/
struct mutex adc_lock;
+ unsigned int vid;
};

static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan,
@@ -98,6 +105,22 @@ static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan,
return ret;
}

+static int mt6370_adc_get_ibus_scale(struct mt6370_adc_data *priv)
+{
+ if (priv->vid == MT6370_VID_RT5081 || priv->vid == MT6370_VID_RT5081A || priv->vid == MT6370_VID_MT6370)
+ return 3350;
+
+ return 3875;
+}
+
+static int mt6370_adc_get_ibat_scale(struct mt6370_adc_data *priv)
+{
+ if (priv->vid == MT6370_VID_RT5081 || priv->vid == MT6370_VID_RT5081A || priv->vid == MT6370_VID_MT6370)
+ return 2680;
+
+ return 3870;
+}
+
static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
int chan, int *val1, int *val2)
{
@@ -123,7 +146,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
case MT6370_AICR_250_mA:
case MT6370_AICR_300_mA:
case MT6370_AICR_350_mA:
- *val1 = 3350;
+ *val1 = mt6370_adc_get_ibus_scale(priv);
break;
default:
*val1 = 5000;
@@ -150,7 +173,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
case MT6370_ICHG_600_mA:
case MT6370_ICHG_700_mA:
case MT6370_ICHG_800_mA:
- *val1 = 2680;
+ *val1 = mt6370_adc_get_ibat_scale(priv);
break;
default:
*val1 = 5000;
@@ -251,6 +274,20 @@ static const struct iio_chan_spec mt6370_adc_channels[] = {
MT6370_ADC_CHAN(TEMP_JC, IIO_TEMP, 12, BIT(IIO_CHAN_INFO_OFFSET)),
};

+static int mt6370_get_vendor_info(struct mt6370_adc_data *priv)
+{
+ unsigned int dev_info;
+ int ret;
+
+ ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &dev_info);
+ if (ret)
+ return ret;
+
+ priv->vid = FIELD_GET(MT6370_VENID_MASK, dev_info);
+
+ return 0;
+}
+
static int mt6370_adc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -272,6 +309,10 @@ static int mt6370_adc_probe(struct platform_device *pdev)
priv->regmap = regmap;
mutex_init(&priv->adc_lock);

+ ret = mt6370_get_vendor_info(priv);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get vid\n");
+
ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, 0);
if (ret)
return dev_err_probe(dev, ret, "Failed to reset ADC\n");
--
2.7.4


2023-04-07 17:15:50

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2] iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips

On Thu, 6 Apr 2023 11:08:51 +0800
ChiaEn Wu <[email protected]> wrote:

> The scale value of ibus and ibat on the datasheet is incorrect due to the
> customer report after the experimentation with some specific vendor ID
> chips.
>
> Signed-off-by: ChiaEn Wu <[email protected]>

Hi,

Please give a fixes tag to aid backporting to right stable branches.

A minor formatting suggestion inline. Otherwise looks fine to me.

Thanks,

Jonathan


> ---
> v2:
> - Move 'get_vendor_info' after iio_priv allocation done
> - For the special case, bypass indent, make it out-of-line
> - For a simple return, add a blank line
> - Drop else
> ---
> drivers/iio/adc/mt6370-adc.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/mt6370-adc.c b/drivers/iio/adc/mt6370-adc.c
> index bc62e5a..ae5cdff 100644
> --- a/drivers/iio/adc/mt6370-adc.c
> +++ b/drivers/iio/adc/mt6370-adc.c
> @@ -19,6 +19,7 @@
>
> #include <dt-bindings/iio/adc/mediatek,mt6370_adc.h>
>
> +#define MT6370_REG_DEV_INFO 0x100
> #define MT6370_REG_CHG_CTRL3 0x113
> #define MT6370_REG_CHG_CTRL7 0x117
> #define MT6370_REG_CHG_ADC 0x121
> @@ -27,6 +28,7 @@
> #define MT6370_ADC_START_MASK BIT(0)
> #define MT6370_ADC_IN_SEL_MASK GENMASK(7, 4)
> #define MT6370_AICR_ICHG_MASK GENMASK(7, 2)
> +#define MT6370_VENID_MASK GENMASK(7, 4)
>
> #define MT6370_AICR_100_mA 0x0
> #define MT6370_AICR_150_mA 0x1
> @@ -47,6 +49,10 @@
> #define ADC_CONV_TIME_MS 35
> #define ADC_CONV_POLLING_TIME_US 1000
>
> +#define MT6370_VID_RT5081 0x8
> +#define MT6370_VID_RT5081A 0xA
> +#define MT6370_VID_MT6370 0xE
> +
> struct mt6370_adc_data {
> struct device *dev;
> struct regmap *regmap;
> @@ -55,6 +61,7 @@ struct mt6370_adc_data {
> * from being read at the same time.
> */
> struct mutex adc_lock;
> + unsigned int vid;
> };
>
> static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan,
> @@ -98,6 +105,22 @@ static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan,
> return ret;
> }
>
> +static int mt6370_adc_get_ibus_scale(struct mt6370_adc_data *priv)
> +{
> + if (priv->vid == MT6370_VID_RT5081 || priv->vid == MT6370_VID_RT5081A || priv->vid == MT6370_VID_MT6370)
> + return 3350;
> +
> + return 3875;
> +}
> +
> +static int mt6370_adc_get_ibat_scale(struct mt6370_adc_data *priv)
> +{
> + if (priv->vid == MT6370_VID_RT5081 || priv->vid == MT6370_VID_RT5081A || priv->vid == MT6370_VID_MT6370)
Line too long. You could wrap it, but perhaps even better to just make this a switch
statement as that will be easier to enhance if we have more cases in future without
a lot of churn.

switch (priv->vid) {
case MT6370_VID_RT5081:
case MT6370_VID_RT5081A:
case MT6370_VID_RT6370:
return 2680;
default:
return 3870;
}

Same for the other similar case above.

> + return 2680;
> +
> + return 3870;
> +}
> +
> static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
> int chan, int *val1, int *val2)
> {
> @@ -123,7 +146,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
> case MT6370_AICR_250_mA:
> case MT6370_AICR_300_mA:
> case MT6370_AICR_350_mA:
> - *val1 = 3350;
> + *val1 = mt6370_adc_get_ibus_scale(priv);
> break;
> default:
> *val1 = 5000;
> @@ -150,7 +173,7 @@ static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
> case MT6370_ICHG_600_mA:
> case MT6370_ICHG_700_mA:
> case MT6370_ICHG_800_mA:
> - *val1 = 2680;
> + *val1 = mt6370_adc_get_ibat_scale(priv);
> break;
> default:
> *val1 = 5000;
> @@ -251,6 +274,20 @@ static const struct iio_chan_spec mt6370_adc_channels[] = {
> MT6370_ADC_CHAN(TEMP_JC, IIO_TEMP, 12, BIT(IIO_CHAN_INFO_OFFSET)),
> };
>
> +static int mt6370_get_vendor_info(struct mt6370_adc_data *priv)
> +{
> + unsigned int dev_info;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &dev_info);
> + if (ret)
> + return ret;
> +
> + priv->vid = FIELD_GET(MT6370_VENID_MASK, dev_info);
> +
> + return 0;
> +}
> +
> static int mt6370_adc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -272,6 +309,10 @@ static int mt6370_adc_probe(struct platform_device *pdev)
> priv->regmap = regmap;
> mutex_init(&priv->adc_lock);
>
> + ret = mt6370_get_vendor_info(priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get vid\n");
> +
> ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, 0);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to reset ADC\n");