On Thu, 6 Apr 2023 11:08:51 +0800
ChiaEn Wu <chiaen_wu@xxxxxxxxxxx> 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 <chiaen_wu@xxxxxxxxxxx>
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");
