Messages in this thread | | | Date | Tue, 29 May 2012 11:10:32 +0900 | From | jonghwa3.lee@samsung ... | Subject | Re: [PATCH v4] regulator: MAX77686: Add Maxim 77686 regulator driver |
| |
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(®ulators[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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |