[PATCH 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for range and scale
Roberta Dobrescu
roberta.dobrescu at gmail.com
Thu Feb 19 12:30:46 EET 2015
On 19.02.2015 11:54, Daniel Baluta wrote:
> Some more comments.
>
> <snip>
>
>> -static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range,
>> - unsigned int *new_range)
>> +static int isl29018_set_range(struct isl29018_chip *chip, unsigned long range)
>> {
>> - static const unsigned long supp_ranges[] = {1000, 4000, 16000, 64000};
>> - int i;
>> + int i, ret;
>>
>> - for (i = 0; i < ARRAY_SIZE(supp_ranges); ++i) {
>> - if (range <= supp_ranges[i]) {
>> - *new_range = (unsigned int)supp_ranges[i];
>> + for (i = 0; i < ARRAY_SIZE(isl29018_ranges); ++i) {
>> + if (range == isl29018_ranges[i]) {
>> + chip->range = i;
>> break;
>> }
>> }
> We need to use some local variables to keep track of range and scale.
You're right about this. I'll introduce local variables.
>>
>> - if (i >= ARRAY_SIZE(supp_ranges))
>> + if (i >= ARRAY_SIZE(isl29018_ranges))
>> return -EINVAL;
>>
>> + chip->scale = isl29018_scales[chip->range][0];
>> +
>> + ret = regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
>> + COMMANDII_RESOLUTION_MASK, 0);
>> + if (ret < 0)
>> + return ret;
>> +
>> return regmap_update_bits(chip->regmap, ISL29018_REG_ADD_COMMANDII,
>> COMMANDII_RANGE_MASK, i << COMMANDII_RANGE_SHIFT);
>
> .. after regmap_update_bits returns true then we can update the chip
> state with range and scale.
>
> Otherwise, in case an error we will have an inconsistent view of that
> chips state/scale.
So local variables here too.
> Also, perhaps we could merge these 2 regmap_update_bits calls since they update
> the same register.
Will do.
>> }
>>
>> -static int isl29018_set_resolution(struct isl29018_chip *chip,
>> - unsigned long adcbit, unsigned int *conf_adc_bit)
>> +static int isl29018_set_scale(struct isl29018_chip *chip, int scale, int uscale)
>> {
>> - static const unsigned long supp_adcbit[] = {16, 12, 8, 4};
>> int i;
>>
>> - for (i = 0; i < ARRAY_SIZE(supp_adcbit); ++i) {
>> - if (adcbit >= supp_adcbit[i]) {
>> - *conf_adc_bit = (unsigned int)supp_adcbit[i];
>> + for (i = 0; i < ARRAY_SIZE(isl29018_scales[chip->range]); ++i) {
>> + if (scale == isl29018_scales[chip->range][i].scale &&
>> + uscale == isl29018_scales[chip->range][i].uscale) {
>> + chip->scale = isl29018_scales[chip->range][i];
> The same problem here. We update state of chip before knowing that
> the hardware operation succeeded.
Thanks for finding this problems. I'll solve them asap.
More information about the firefly
mailing list