> >
> > > +
> > > +#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
