| Message ID | 1679667167-16261-1-git-send-email-chiaen_wu@richtek.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips | expand |
Hi ChiaEn,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.3-rc3 next-20230324]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ChiaEn-Wu/iio-adc-mt6370-Fix-ibus-and-ibat-scaling-value-of-some-specific-vendor-ID-chips/20230324-221545
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/1679667167-16261-1-git-send-email-chiaen_wu%40richtek.com
patch subject: [PATCH] iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips
config: riscv-randconfig-r034-20230324 (https://download.01.org/0day-ci/archive/20230325/202303250050.V7JLIADZ-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/9822359d5de2dba531d882cfee6949864a2d6170
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review ChiaEn-Wu/iio-adc-mt6370-Fix-ibus-and-ibat-scaling-value-of-some-specific-vendor-ID-chips/20230324-221545
git checkout 9822359d5de2dba531d882cfee6949864a2d6170
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/iio/adc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303250050.V7JLIADZ-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/iio/adc/mt6370-adc.c:306:31: warning: variable 'priv' is uninitialized when used here [-Wuninitialized]
ret = mt6370_get_vendor_info(priv);
^~~~
drivers/iio/adc/mt6370-adc.c:297:30: note: initialize the variable 'priv' to silence this warning
struct mt6370_adc_data *priv;
^
= NULL
1 warning generated.
vim +/priv +306 drivers/iio/adc/mt6370-adc.c
293
294 static int mt6370_adc_probe(struct platform_device *pdev)
295 {
296 struct device *dev = &pdev->dev;
297 struct mt6370_adc_data *priv;
298 struct iio_dev *indio_dev;
299 struct regmap *regmap;
300 int ret;
301
302 regmap = dev_get_regmap(pdev->dev.parent, NULL);
303 if (!regmap)
304 return dev_err_probe(dev, -ENODEV, "Failed to get regmap\n");
305
> 306 ret = mt6370_get_vendor_info(priv);
307 if (ret)
308 return dev_err_probe(dev, ret, "Failed to get vid\n");
309
310 indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
311 if (!indio_dev)
312 return -ENOMEM;
313
314 priv = iio_priv(indio_dev);
315 priv->dev = dev;
316 priv->regmap = regmap;
317 mutex_init(&priv->adc_lock);
318
319 ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, 0);
320 if (ret)
321 return dev_err_probe(dev, ret, "Failed to reset ADC\n");
322
323 indio_dev->name = "mt6370-adc";
324 indio_dev->info = &mt6370_adc_iio_info;
325 indio_dev->modes = INDIO_DIRECT_MODE;
326 indio_dev->channels = mt6370_adc_channels;
327 indio_dev->num_channels = ARRAY_SIZE(mt6370_adc_channels);
328
329 return devm_iio_device_register(dev, indio_dev);
330 }
331
On Fri, 24 Mar 2023 22:12:47 +0800 ChiaEn Wu <chiaen_wu@richtek.com> 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@richtek.com> Hi. Only significant issue here is the one the build bot found. A few other trivial formatting suggestions inline. Thanks, Jonathan > > static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan, > @@ -98,6 +105,26 @@ 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; > + else I'd drop the else. We are special casing the matches above, so it makes sense for them to be out of line. The 'normal' case doesn't need to be indented. > + 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; > + else > + return 3870; > +} > + > static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, > int chan, int *val1, int *val2) > { > @@ -123,7 +150,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 +177,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 +278,19 @@ 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); Blank line preferred before a simple return like this. Makes the code a tiny bit more readable. > + return 0; > +} > + > static int mt6370_adc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -263,6 +303,10 @@ static int mt6370_adc_probe(struct platform_device *pdev) > if (!regmap) > return dev_err_probe(dev, -ENODEV, "Failed to get regmap\n"); > > + ret = mt6370_get_vendor_info(priv); The build bot spotted this one. Can't use priv yet as it doesn't exist for a few more lines. > + if (ret) > + return dev_err_probe(dev, ret, "Failed to get vid\n"); > + > indio_dev = devm_iio_device_alloc(dev, sizeof(*priv)); > if (!indio_dev) > return -ENOMEM;
Hi ChiaEn,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ChiaEn-Wu/iio-adc-mt6370-Fix-ibus-and-ibat-scaling-value-of-some-specific-vendor-ID-chips/20230324-221545
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/1679667167-16261-1-git-send-email-chiaen_wu%40richtek.com
patch subject: [PATCH] iio: adc: mt6370: Fix ibus and ibat scaling value of some specific vendor ID chips
config: m68k-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230330/202303302231.gNdPKrtD-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202303302231.gNdPKrtD-lkp@intel.com/
smatch warnings:
drivers/iio/adc/mt6370-adc.c:306 mt6370_adc_probe() error: uninitialized symbol 'priv'.
vim +/priv +306 drivers/iio/adc/mt6370-adc.c
c1404d1b659fe3d ChiaEn Wu 2022-10-11 294 static int mt6370_adc_probe(struct platform_device *pdev)
c1404d1b659fe3d ChiaEn Wu 2022-10-11 295 {
c1404d1b659fe3d ChiaEn Wu 2022-10-11 296 struct device *dev = &pdev->dev;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 297 struct mt6370_adc_data *priv;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 298 struct iio_dev *indio_dev;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 299 struct regmap *regmap;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 300 int ret;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 301
c1404d1b659fe3d ChiaEn Wu 2022-10-11 302 regmap = dev_get_regmap(pdev->dev.parent, NULL);
c1404d1b659fe3d ChiaEn Wu 2022-10-11 303 if (!regmap)
c1404d1b659fe3d ChiaEn Wu 2022-10-11 304 return dev_err_probe(dev, -ENODEV, "Failed to get regmap\n");
c1404d1b659fe3d ChiaEn Wu 2022-10-11 305
9822359d5de2dba ChiaEn Wu 2023-03-24 @306 ret = mt6370_get_vendor_info(priv);
"priv" is uninitialized.
9822359d5de2dba ChiaEn Wu 2023-03-24 307 if (ret)
9822359d5de2dba ChiaEn Wu 2023-03-24 308 return dev_err_probe(dev, ret, "Failed to get vid\n");
9822359d5de2dba ChiaEn Wu 2023-03-24 309
c1404d1b659fe3d ChiaEn Wu 2022-10-11 310 indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
c1404d1b659fe3d ChiaEn Wu 2022-10-11 311 if (!indio_dev)
c1404d1b659fe3d ChiaEn Wu 2022-10-11 312 return -ENOMEM;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 313
c1404d1b659fe3d ChiaEn Wu 2022-10-11 314 priv = iio_priv(indio_dev);
^^^^^^^^^^^^^^^^^^^^^^^^^^
c1404d1b659fe3d ChiaEn Wu 2022-10-11 315 priv->dev = dev;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 316 priv->regmap = regmap;
c1404d1b659fe3d ChiaEn Wu 2022-10-11 317 mutex_init(&priv->adc_lock);
diff --git a/drivers/iio/adc/mt6370-adc.c b/drivers/iio/adc/mt6370-adc.c index bc62e5a..eea7223 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; + int vid; }; static int mt6370_adc_read_channel(struct mt6370_adc_data *priv, int chan, @@ -98,6 +105,26 @@ 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; + else + 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; + else + return 3870; +} + static int mt6370_adc_read_scale(struct mt6370_adc_data *priv, int chan, int *val1, int *val2) { @@ -123,7 +150,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 +177,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 +278,19 @@ 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; @@ -263,6 +303,10 @@ static int mt6370_adc_probe(struct platform_device *pdev) if (!regmap) return dev_err_probe(dev, -ENODEV, "Failed to get regmap\n"); + ret = mt6370_get_vendor_info(priv); + if (ret) + return dev_err_probe(dev, ret, "Failed to get vid\n"); + indio_dev = devm_iio_device_alloc(dev, sizeof(*priv)); if (!indio_dev) return -ENOMEM;
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@richtek.com> --- drivers/iio/adc/mt6370-adc.c | 48 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)