regulator: pcf50633: Don't write to reserved bits of AUTO output voltage select register

Convert pcf50633 to get_voltage_sel and then we can remove pcf50633_regulator_voltage_value function and move its implementation to pcf50633_regulator_list_voltage.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/pcf50633-regulator.c |   65 ++++++++++++++-----------------
 1 files changed, 29 insertions(+), 36 deletions(-)

drivers/regulator/pcf50633-regulator.c +6 -2

index 9ef7dff..b02cbb9 100644
@@ -151,23 +151,43 @@ static int pcf50633_regulator_set_voltage(struct regulator_dev *rdev,
 	return pcf50633_reg_write(pcf, regnr, volt_bits);
 }
 
-static int pcf50633_regulator_voltage_value(enum pcf50633_regulator_id id,
-						u8 bits)
+static int pcf50633_regulator_get_voltage_sel(struct regulator_dev *rdev)
 {
+	struct pcf50633 *pcf;
+	int regulator_id;
+	u8 volt_bits, regnr;
+
+	pcf = rdev_get_drvdata(rdev);

The datasheet says 00000000 to 00101110 are reserved, and the min value of the voltage setting is 1.8 V.
Thus don't write 0 to AUTO output voltage select register (address 1Ah).

Table 50. AUTOOUT - AUTO output voltage select register (address 1Ah) bit description[1] Bit Symbol Access Description
7:0 auto_out R/W VO(prog) = 0.625 + auto_out × 0.025 V
eg. 00000000 to 00101110: reserved
00101111: 1.8 V (min)
01010011: 2.7 V
01101010: 3.275 V
01101011: 3.300 V
01101100: 3.325 V
01111111 : 3.800 V (max)
..... .....
11111110 : 3.800 V
11111111 : 3.800 V

This patch also fixes a bug in pcf50633_regulator_list_voltage.
It is wrong to do "index += 0x2f" for PCF50633_REGULATOR_AUTO in pcf50633_regulator_list_voltage. The purpose of adding 0x2f to index is because current code return 0 in auto_voltage_bits when millivolts < 1800. For millivolts > 1800, adding 0x2f to index is wrong.

Quoted 18 lines

The datasheet says 00000000 to 00101110 are reserved, and the min value of the voltage setting is 1.8 V.
Thus don't write 0 to AUTO output voltage select register (address 1Ah).

Table 50. AUTOOUT - AUTO output voltage select register (address 1Ah) bit description[1] Bit Symbol Access Description
7:0 auto_out R/W VO(prog) = 0.625 + auto_out × 0.025 V
eg. 00000000 to 00101110: reserved
00101111: 1.8 V (min)
01010011: 2.7 V
01101010: 3.275 V
01101011: 3.300 V
01101100: 3.325 V
01111111 : 3.800 V (max)
..... .....
11111110 : 3.800 V
11111111 : 3.800 V

Quoted 6 lines

This patch also fixes a bug in pcf50633_regulator_list_voltage.
It is wrong to do "index += 0x2f" for PCF50633_REGULATOR_AUTO in pcf50633_regulator_list_voltage. The purpose of adding 0x2f to index is because current code return 0 in auto_voltage_bits when millivolts < 1800. For millivolts > 1800, adding 0x2f to index is wrong.

I think you misunderstood what the current code does. The first usable voltage is 1.8V which is equal to a index of of 0x2f. So the driver adds 0x2f to the index so that there is not a headroom of 0x2f unusable voltages. So a selector of 0 translates to 1.8V, a selector of 1 translates to 1.825V and so on.

I can see why you'd want to change it to simplify the code, but you also have to change the number of voltages for the AUTO regulator to 128.

2012/3/18 Lars-Peter Clausen <[email protected]>:

Quoted 26 lines · Axel Lin

Quoted 18 lines

The datasheet says 00000000 to 00101110 are reserved, and the min value of the voltage setting is 1.8 V.
Thus don't write 0 to AUTO output voltage select register (address 1Ah).

Table 50. AUTOOUT - AUTO output voltage select register (address 1Ah) bit description[1] Bit Symbol Access Description
7:0 auto_out R/W VO(prog) = 0.625 + auto_out × 0.025 V
eg. 00000000 to 00101110: reserved
00101111: 1.8 V (min)
01010011: 2.7 V
01101010: 3.275 V
01101011: 3.300 V
01101100: 3.325 V
01111111 : 3.800 V (max)
..... .....
11111110 : 3.800 V
11111111 : 3.800 V

Quoted 6 lines

This patch also fixes a bug in pcf50633_regulator_list_voltage.
It is wrong to do "index += 0x2f" for PCF50633_REGULATOR_AUTO in pcf50633_regulator_list_voltage. The purpose of adding 0x2f to index is because current code return 0 in auto_voltage_bits when millivolts < 1800. For millivolts > 1800, adding 0x2f to index is wrong.

Quoted 4 lines

I think you misunderstood what the current code does. The first usable voltage is 1.8V which is equal to a index of of 0x2f. So the driver adds 0x2f to the index so that there is not a headroom of 0x2f unusable voltages. So a selector of 0 translates to 1.8V, a selector of 1 translates to 1.825V and so on.

I think what I wry to explain is:
In regulator core _regulator_do_set_voltage function:

        if (rdev->desc->ops->set_voltage) {
                ret = rdev->desc->ops->set_voltage(rdev, min_uV, max_uV,
                                                   &selector);

                if (rdev->desc->ops->list_voltage)
                        selector = rdev->desc->ops->list_voltage(rdev,
                                                                 selector);
                else
                        selector = -1;

The list_voltage call here takes the selector got from set_voltage callback. Thus adding 0x2f to the index in pcf50633_regulator_list_voltage looks wrong to me.