Re: [PATCH v4] regulator: MAX77686: Add Maxim 77686 regulator driver

[jonghwa3 . lee]

Hi, Yadwinder,

On 2012ë 05ì 25ì 21:27, Yadwinder Singh Brar wrote:

> Hi Jonghwa,
> 
>> +
>> +       if (pdata)
>> +               max77686->opmode_data = pdata->opmode_data;
> 
> I think this is unused(unwanted) now.
> 


Sorry, i missed to remove it out. I'll remove it.

>> +
>> +       for (i = 0; i < MAX77686_REGULATORS; i++) {
>> +               if (pdata)
>> +                       init_data[pdata->regulators[i].id] =
>> +                                                pdata->regulators[i].initdata;
> 
> I  think we can directly use  pdata->regulators[i].initdata instead of
> init_data[i].
> In case if pdata is not their we can use same instance of
> init_data(default)  for all regulators.
> 


This if for some situation that pdata's initdata doensn't line up. When
user sets only initdata considered it being used, there may be
regulators not having initdata, also its order is not clear. So for
those state, i think just using temporary array which satisfies
regulator's id order is fine while it can't use pdata's initdata directly.

>> +
>> +               config.init_data = init_data[i];
>> +               rdev[i] = regulator_register(&regulators[i], &config);
>> +
> 
>> +
>> +       for (i = 0; i < 8; i++) {
>> +               if (pdata->buck2_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck2_voltage[i],
>> +                                       pdata->buck2_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck2_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck2_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck2_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK2DVS1 + i, max77686->buck2_vol[i]);
>> +
>> +               if (pdata->buck3_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck3_voltage[i],
>> +                                       pdata->buck3_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck3_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck3_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck3_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK3DVS1 + i, max77686->buck3_vol[i]);
>> +
>> +               if (pdata->buck4_voltage[i] > 0)
>> +                       ret = regulator_map_voltage_linear(
>> +                                       rdev[MAX77686_LDOS+1],
>> +                                       pdata->buck4_voltage[i],
>> +                                       pdata->buck4_voltage[i]
>> +                                       + MAX77686_DVS_UVSTEP);
>> +               /* 1.1V as default for safety */
>> +               if (pdata->buck4_voltage[i] <= 0 || ret < 0)
>> +                       max77686->buck4_vol[i] = 0x28;
>> +               else
>> +                       max77686->buck4_vol[i] = ret;
>> +               regmap_write(max77686->iodev->regmap,
>> +                        MAX77686_REG_BUCK4DVS1 + i, max77686->buck4_vol[i]);
>> +       }
> 
> Why do we need to initialize the 8 voltage registers of BUCK2/3/4  ?
> 


Yes you're right. It is useless because we don't support DVS mode at
this moment. I'll remove it either.

> 
> Regards,
> Yadwinder.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


Thanks ,
Best Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/