> -----Original Message----- > From: Mark Brown [mailto:broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx] > Sent: Thursday, February 18, 2010 3:18 PM > To: Nayak, Rajendra > Cc: linux-omap@xxxxxxxxxxxxxxx; Liam Girdwood; Samuel Ortiz > Subject: Re: [PATCH 1/2] twl6030: regulator: Fix vsel > calculations in set/get voltage apis > > On Thu, Feb 18, 2010 at 12:02:49PM +0530, Nayak, Rajendra wrote: > > > > This looks wrong - this code is inside a loop over vsel->voltage > > > mappings, if that mapping is wrong and needs to be > ignored (which is > > > what this code is doing) then we shouldn't be looking at > it at all. > > > Not sure if I understood your comment completely, but > here's how I think > > it will work. > > > The for loop runs through all the valid voltages supported > by the regulator > > and and when it finds the first in-range value programs the > VOLTAGE register. > > For twl4030 the VOLTAGE register can be programmed with > just the index > > of the table since the mapping was something like > > > 1800mV = 0x0 > > 2800mV = 0x1 > > 2900mV = 0x2 > > 3000mV = 0x3 > > > Now for twl6030 I just have added an additional vsel > calulation part after > > the in-range value is found, since using the table index > directly as vsel > > would not work. > > Right, but this means that the code is looking at one table > of values to > find a voltage it likes and then picking the actual selector from > another set of values. This means that depending on the actual > differences it may either pick a value which isn't valid for > the TWL6030 > or omit values which the TWL6030 can actually support. > Either way from > a code inspection point of view the code doesn't look good due to the > mismatch between the two sets of selectors. > > The TWL6030 case should be doing something more like other regulator > drivers with straightforward mappings between selector and > voltage (most > of them) and not using the TWL4030 value table. The code is'nt looking into twl4030 tables for finding the valid voltage for twl6030. There are seperate tables for twl4030 and twl6030 regulators with all the valid supported voltages. The difference is, in case of twl4030 once you find a valid voltage from the right table you could just use the table index to program the register on twl4030. In case of twl6030, once you find a valid voltage, again from the right table (meant for twl6030) you still need to derive what value needs to be programmed in the register, so twl6030 understands it. Using the table index does not help. Example: In case of TWL4030 static const u16 VAUX1_VSEL_table[] = { 2500, 2800, 3000, 3000, 3000, 3000, }; If the request is to set 2800mv, look up the table, once found program the VOLTAGE register with the table index. In this case 0x1. So the mapping is VOLTAGE_REG=0x0, VAUX1=2500mv VOLTAGE_REG=0x1, VAUX1=2800mv VOLTAGE_REG=0x2, VAUX1=3000mv In case of TWL6030 static const u16 VAUX1_6030_VSEL_table[] = { 1000, 1300, 1800, 2500, 2800, 2900, 3000, 3000, }; if the request is to set 2800mv, look up the table, once found calulate the value to be programmed value = (2800 - 1000)/100 + 1 = 0x13 So here the mapping is VOLTAGE_REG=0x10, VAUX1=2500mv VOLTAGE_REG=0x13, VAUX1=2800mv VOLTAGE_REG=0x15, VAUX1=3000mv Without this patch the code goes ahead and programs the table index (In this case 0x4) which is wrong > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
