Hi,
* Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> [170316 23:44]:
> > Signed-off-by: Tony Lindgrne <tony@xxxxxxxxxxx>
>
> Lindgren
Hmm yeah there are slight variations to my signature it seems :)
...
> > +/* Register CPCAP_REG_ADCC1 bits */
> > +#define CPCAP_BIT_ADEN_AUTO_CLR BIT(15) /* Currently unused */
> > +#define CPCAP_BIT_CAL_MODE BIT(14) /* Set with BIT_RAND0 */
> > +#define CPCAP_BIT_ADC_CLK_SEL1 BIT(13) /* Currently unused */
> > +#define CPCAP_BIT_ADC_CLK_SEL0 BIT(12) /* Currently unused */
> > +#define CPCAP_BIT_ATOX BIT(11) /* in/out ps factor */
>
> wondering what a 'ps' factor is...
I guess I have no idea at this point with no documentation. I'll
just drop the comments for now for the mystery registers.
> > +#define CPCAP_CHAN(_type, _index, _address, _datasheet_name) { \
> > + .type = (_type), \
> > + .address = (_address), \
> > + .indexed = 1, \
> > + .channel = (_index), \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > + BIT(IIO_CHAN_INFO_SCALE), \
> > + .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> > + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
>
> I don't see SAMP_FREQ and _OVERSAMPLING handled?
OK will drop for now.
> > + dev_info(ddata->dev,
> > + "cpcap_adc_probe: CHRGI Cal complete!\n");
>
> maybe calibration instead of Cal
Yup thanks, also can use __func__ or just driver name.
> > + break;
> > + }
> > +
> > + msleep(5);
>
> I guess this will trigger a checkpatch warning
>
> > + i++;
> > + } while (i > 5);
>
> should be < 5????
> why not just use a for look?
Heheh yeah that looks really weird :) I guess I was too chicken to mess
with the calibration for the battery ADCs earlier so I tried to keep it
as it was in the Motorola driver. But now things are working and I can
compare the results so it should not be a problem.
> can't this code duplication be avoided?
Yeah so it seems, I'll take a look.
> > +/* Looks up temperatures in a table and calculates averages if needed */
> > +static unsigned short cpcap_adc_table_to_millicelcius(unsigned short value)
>
> why unsigned if it returns millicelcius, shouldn't this be int to cover
> the temperature rate?
Thanks yes. It was Kelvins in the Motorola driver.
Will fix up the rest of your comments and repost. Thanks for looking
and thanks for the good comments.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
