+#define MT6370_AICR_400MA 0x6 +#define MT6370_ICHG_500MA 0x4 +#define MT6370_ICHG_900MA 0x8
+#define ADC_CONV_TIME_US 35000 +#define ADC_CONV_POLLING_TIME 1000
+struct mt6370_adc_data {
struct device *dev;struct regmap *regmap;struct mutex lock;All locks need documentation. What is the scope of the lock? Looks like it protects device state when doing setup, wait for read, read cycles.
This mutex lock is for preventing the different adc channel from being read at the same time. So, if I just change its name to adc_chan_lock or adc_lock and add the comment for this mutex lock, does this change meet your requirement
Yes
+};
+static int mt6370_adc_read_scale(struct mt6370_adc_data *priv,
int chan, int *val1, int *val2)+{
unsigned int reg_val;int ret;switch (chan) {case MT6370_CHAN_VBAT:case MT6370_CHAN_VSYS:case MT6370_CHAN_CHG_VDDP:*val1 = 5000;This seems very large. Voltages are in millivolts as per Documentation/ABI/testing/sysfs-bus-iio and this means each step is 5 volts.
So value in mv is currently 5 * _raw
OK, I got it. Also, I will add the ABI file in the next version. Thanks!
Only add ABI documentation for anything non-standard.
The documentation scripts really don't like repeats!
+static const char * const mt6370_channel_labels[MT6370_CHAN_MAX] = {
Perhaps define an enum with which to index this and the chan spec and hence ensure they end up matching. [vbusdiv5] = "vbusdiv5", etc
Do you mean that I can refine this const char array to the following array??
static const char * const mt6370_channel_labels[MT6370_CHAN_MAX] = { [MT6370_CHAN_VBUSDIV5] = "vbusdiv5", [MT6370_CHAN_VBUSDIV2] = "vbusdiv2", ... ... [MT6370_CHAN_TEMP_JC] = "temp_jc", };
Yes
thanks,
Jonathan