On Thu, Jun 23, 2022 at 2:00 PM ChiaEn Wu peterwu.pub@gmail.com wrote:
From: ChiaEn Wu chiaen_wu@richtek.com
Add Mediatek MT6370 ADC support.
...
+config MEDIATEK_MT6370_ADC
tristate "Mediatek MT6370 ADC driver"depends on MFD_MT6370helpSay yes here to enable Mediatek MT6370 ADC support.This ADC driver provides 9 channels for system monitoring (chargercurrent, voltage, and temperature).
What will be the module name?
...
+#include <dt-bindings/iio/adc/mediatek,mt6370_adc.h>
Usually this goes after linux/* asm/* as it's not so generic.
+#include <linux/bits.h> +#include <linux/bitfield.h> +#include <linux/iio/iio.h> +#include <linux/kernel.h> +#include <linux/module.h>
+#include <linux/mod_devicetable.h>
I believe the order should be otherwise, this is first followed by module.h.
+#include <linux/mutex.h> +#include <linux/platform_device.h> +#include <linux/regmap.h>
...
+#define ADC_CONV_POLLING_TIME 1000
If it's time, add a unit suffix, if it's a counter, make it clear.
...
msleep(ADC_CONV_TIME_US / 1000);
Why define microseconds if milliseconds are in use?
...
ret = regmap_read_poll_timeout(priv->regmap,MT6370_REG_CHG_ADC, reg_val,!(reg_val & MT6370_ADC_START_MASK),ADC_CONV_POLLING_TIME,ADC_CONV_TIME_US * 3);if (ret) {if (ret == -ETIMEDOUT)dev_err(priv->dev, "Failed to wait ADC conversion\n");
wait for
elsedev_err(priv->dev,"Failed to read ADC register (%d)\n", ret);
Do you really need to differentiate the errors here? I believe the latter one covers all cases.
goto adc_unlock;}
...
+#define MT6370_ADC_CHAN(_idx, _type, _addr, _extra_info) { \
.type = _type, \.channel = MT6370_CHAN_##_idx, \.address = _addr, \.scan_index = MT6370_CHAN_##_idx, \.indexed = 1, \.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \BIT(IIO_CHAN_INFO_SCALE) | \_extra_info \
Leave a comma after the last member as well.
+}
...
regmap = dev_get_regmap(pdev->dev.parent, NULL);if (!regmap) {
dev_err(&pdev->dev, "Failed to get regmap\n");return -ENODEV;
return dev_err_probe(...);
}
...
ret = regmap_write(priv->regmap, MT6370_REG_CHG_ADC, 0);if (ret) {dev_err(&pdev->dev, "Failed to reset ADC\n");return ret;}
Ditto.