RE: [PATCH 2/2] regulator: tps65217: Fix voltage boundary checkingin tps65217_pmic_map_voltage

From: AnilKumar, Chimata
Date: Tue Jul 03 2012 - 02:48:44 EST


On Tue, Jul 03, 2012 at 10:56:08, Axel Lin wrote:
>
> > Case 1:-
> > ------
> >
> > tps->info[rid]->min_uV = 600000;
> > tps->info[rid]->max_uV = 1100000;
> >
> > If we do regulator_set_voltage(reg, 550000, 1100000);
> >
> > This results into error condition and how can we handle with this?
>
> In original code, it returns error. ( I think which is wrong.)
>
> What we want is to set the smallest voltage within the specified voltage
> range.
> With this patch, calling regulator_set_voltage(reg, 550000, 1100000);
> is the same as calling regulator_set_voltage(reg, 600000, 1100000);
> It will set the voltage to 600000 uV.

Suppose
rdev->constraints->min_uV = 600000; /* wrong data entered by user as DT/plat
data */
rdev->constraints->max_uV = 1500000;

And tps limits (little modification from my previous mail "actual values")

tps->info[rid]->min_uV = 900000;
tps->info[rid]->max_uV = 1800000;

regulator_set_voltage(reg, 850000, 1100000);

In side tps65217_pmic_map_voltage() we have

tps->info[rid]->uv_to_vsel(850000, &sel) nothing but
tps65217_uv_to_vsel1(850000, &sel);

sel = (((850000 - 900000) + (25000) - 1) / (25000));
sel = ((-25001)/(25000));
sel = -1; /* Which is not expected */

Ideally this would be the change

if (min_uV < tps->info[rid]->min_uV)
min_uV = tps->info[rid]->min_uV;

if (max_uV > tps->info[rid]->max_uV)
max_uV = tps->info[rid]->max_uV;

if (max_uV < tps->info[rid]->min_uV ||
min_uV > tps->info[rid]->max_uV)
return -EINVAL;

If this is not the case then I am completely in a wrong direction.

>
> >
> > Case 2:-
> > ------
> >
> > I think the current code handles this case as well.
> >
> > There might be a case where board/DT data is false like
> >
> > tps->info[rid]->min_uV = 1100000;
> > tps->info[rid]->max_uV = 600000;
> >
> I don't get it.
> You mean the case min_uV is greater than max_uV?
>
> My understanding of current implementation is that the
> tps->info[rid]->min_uV and tps->info[rid]->max_uV are not controlled by
> board/DT data. They are defined in tps65217_pmic_regs[].

My point here is rdev->constraints->min_uV and rdev->constraints->max_uV
values set from DT/board file. Sorry about the confusion.
This case is taken care by regulator core.

Regards
AnilKumar--
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/