> Date: Thu, 06 Jul 2023 07:31:18 +0900 > From: SASANO Takayoshi <u...@mx5.nisiq.net> > > Hello, > > Adding axp305 support to axppmic(4), I found following bugs: > > - only voltage range defined base and delta working, > base2 and delta2 not working > - even if base2 and delta2 range working, voltage setting/status > is not correct > - vmask for AXP209's dcdc3 is incorrect > - proper vmask is required for AXP209's ldo2/ldo5 settings > > Especially in AXP809(dcdc4), voltage range is non-contiguous such as > 0.6-1.54V/20mV step + 1.8-2.6V/100mV step. > > To define voltage range correctly, I introduced step (and step2) to > struct axppmic_regdata. > > I attach the diff, but it is huge due to modifying axppmic_data table. > > Any comments are welcomed.
Can you use "nsteps" instead of "step" in the variable names? The variable stores the number of steps and that name makes this more clear. One other comment below... > -- > SASANO Takayoshi (JG1UAA) <u...@mx5.nisiq.net> > > Index: axppmic.c > =================================================================== > RCS file: /cvs/src/sys/dev/fdt/axppmic.c,v > retrieving revision 1.16 > diff -u -p -r1.16 axppmic.c > --- axppmic.c 3 Sep 2022 18:05:10 -0000 1.16 > +++ axppmic.c 5 Jul 2023 22:24:37 -0000 > @@ -65,180 +65,180 @@ struct axppmic_regdata { > const char *name; > uint8_t ereg, emask, eval, dval; > uint8_t vreg, vmask; > - uint32_t base, delta; > - uint32_t base2, delta2; > + uint32_t base, delta, step; > + uint32_t base2, delta2, step2; > }; > > const struct axppmic_regdata axp209_regdata[] = { > { "dcdc2", 0x12, (1 << 4), (1 << 4), (0 << 4), > - 0x23, 0x3f, 700000, 25000 }, > + 0x23, 0x3f, 700000, 25000, 64 }, > { "dcdc3", 0x12, (1 << 1), (1 << 1), (0 << 1), > - 0x27, 0x3f, 700000, 25000 }, > + 0x27, 0x7f, 700000, 25000, 113 }, > /* LDO1 can't be controlled */ > { "ldo2", 0x12, (1 << 2), (1 << 2), (0 << 2), > - 0x28, 0xf0, 1800000, (100000 >> 4) }, > + 0x28, 0xf0, 1800000, (100000 >> 4), (16 << 4) }, > { "ldo3", 0x12, (1 << 6), (1 << 6), (0 << 6), > - 0x29, 0x7f, 700000, 25000 }, > + 0x29, 0x7f, 700000, 25000, 113 }, > /* LDO4 voltage levels are complicated */ > { "ldo5", 0x90, 0x07, 0x03, 0x07, > - 0x91, 0xf0, 1800000, (100000 >> 4) }, > + 0x91, 0xf0, 1800000, (100000 >> 4), (16 << 4) }, > { NULL } > }; > > const struct axppmic_regdata axp221_regdata[] = { > { "dcdc1", 0x10, (1 << 1), (1 << 1), (0 << 1), > - 0x21, 0x1f, 1600000, 100000 }, > + 0x21, 0x1f, 1600000, 100000, 19 }, > { "dcdc2", 0x10, (1 << 2), (1 << 2), (0 << 2), > - 0x22, 0x3f, 600000, 20000 }, > + 0x22, 0x3f, 600000, 20000, 48 }, > { "dcdc3", 0x10, (1 << 3), (1 << 3), (0 << 3), > - 0x23, 0x3f, 600000, 20000 }, > + 0x23, 0x3f, 600000, 20000, 64 }, > { "dcdc4", 0x10, (1 << 4), (1 << 4), (0 << 4), > - 0x24, 0x3f, 600000, 20000 }, > + 0x24, 0x3f, 600000, 20000, 48 }, > { "dcdc5", 0x10, (1 << 5), (1 << 5), (0 << 5), > - 0x25, 0x1f, 1000000, 50000 }, > + 0x25, 0x1f, 1000000, 50000, 32 }, > { "dc1sw", 0x12, (1 << 7), (1 << 7), (0 << 7) }, > { "dc5ldo", 0x10, (1 << 0), (1 << 0), (0 << 0), > - 0x1c, 0x07, 700000, 100000 }, > + 0x1c, 0x07, 700000, 100000, 8 }, > { "aldo1", 0x10, (1 << 6), (1 << 6), (0 << 6), > - 0x28, 0x1f, 700000, 100000 }, > + 0x28, 0x1f, 700000, 100000, 27 }, > { "aldo2", 0x10, (1 << 7), (1 << 7), (0 << 7), > - 0x29, 0x1f, 700000, 100000 }, > + 0x29, 0x1f, 700000, 100000, 27 }, > { "aldo3", 0x13, (1 << 7), (1 << 7), (0 << 7), > - 0x2a, 0x1f, 700000, 100000 }, > + 0x2a, 0x1f, 700000, 100000, 27 }, > { "dldo1", 0x12, (1 << 3), (1 << 3), (0 << 3), > - 0x15, 0x1f, 700000, 100000 }, > + 0x15, 0x1f, 700000, 100000, 27 }, > { "dldo2", 0x12, (1 << 4), (1 << 4), (0 << 4), > - 0x16, 0x1f, 700000, 100000 }, > + 0x16, 0x1f, 700000, 100000, 27 }, > { "dldo3", 0x12, (1 << 5), (1 << 5), (0 << 5), > - 0x17, 0x1f, 700000, 100000 }, > + 0x17, 0x1f, 700000, 100000, 27 }, > { "dldo4", 0x12, (1 << 6), (1 << 6), (0 << 6), > - 0x18, 0x1f, 700000, 100000 }, > + 0x18, 0x1f, 700000, 100000, 27 }, > { "eldo1", 0x12, (1 << 0), (1 << 0), (0 << 0), > - 0x19, 0x1f, 700000, 100000 }, > + 0x19, 0x1f, 700000, 100000, 27 }, > { "eldo2", 0x12, (1 << 1), (1 << 1), (0 << 1), > - 0x1a, 0x1f, 700000, 100000 }, > + 0x1a, 0x1f, 700000, 100000, 27 }, > { "eldo3", 0x12, (1 << 2), (1 << 2), (0 << 2), > - 0x1b, 0x1f, 700000, 100000 }, > + 0x1b, 0x1f, 700000, 100000, 27 }, > { "ldo_io0", 0x90, 0x07, 0x03, 0x04, > - 0x91, 0x1f, 700000, 100000 }, > + 0x91, 0x1f, 700000, 100000, 27 }, > { "ldo_io1", 0x92, 0x07, 0x03, 0x04, > - 0x93, 0x1f, 700000, 100000 }, > + 0x93, 0x1f, 700000, 100000, 27 }, > { NULL } > }; > > const struct axppmic_regdata axp803_regdata[] = { > { "dcdc1", 0x10, (1 << 0), (1 << 0), (0 << 0), > - 0x20, 0x1f, 1600000, 100000 }, > + 0x20, 0x1f, 1600000, 100000, 19 }, > { "dcdc2", 0x10, (1 << 1), (1 << 1), (0 << 1), > - 0x21, 0x7f, 500000, 10000, 1220000, 20000 }, > + 0x21, 0x7f, 500000, 10000, 71, 1220000, 20000, 5 }, > { "dcdc3", 0x10, (1 << 2), (1 << 2), (0 << 2), > - 0x22, 0x7f, 500000, 10000, 1220000, 20000 }, > + 0x22, 0x7f, 500000, 10000, 71, 1220000, 20000, 5 }, > { "dcdc4", 0x10, (1 << 3), (1 << 3), (0 << 3), > - 0x23, 0x7f, 500000, 10000, 1220000, 20000 }, > + 0x23, 0x7f, 500000, 10000, 71, 1220000, 20000, 5 }, > { "dcdc5", 0x10, (1 << 4), (1 << 4), (0 << 4), > - 0x24, 0x7f, 800000, 10000, 1140000, 20000 }, > + 0x24, 0x7f, 800000, 10000, 33, 1140000, 20000, 36 }, > { "dcdc6", 0x10, (1 << 5), (1 << 5), (0 << 5), > - 0x25, 0x7f, 600000, 10000, 1120000, 20000 }, > + 0x25, 0x7f, 600000, 10000, 51, 1120000, 20000, 21 }, > { "dc1sw", 0x12, (1 << 7), (1 << 7), (0 << 7) }, > { "aldo1", 0x13, (1 << 5), (1 << 5), (0 << 5), > - 0x28, 0x1f, 700000, 100000 }, > + 0x28, 0x1f, 700000, 100000, 27 }, > { "aldo2", 0x13, (1 << 6), (1 << 6), (0 << 6), > - 0x29, 0x1f, 700000, 100000 }, > + 0x29, 0x1f, 700000, 100000, 27 }, > { "aldo3", 0x13, (1 << 7), (1 << 7), (0 << 7), > - 0x2a, 0x1f, 700000, 100000 }, > + 0x2a, 0x1f, 700000, 100000, 27 }, > { "dldo1", 0x12, (1 << 3), (1 << 3), (0 << 3), > - 0x15, 0x1f, 700000, 100000 }, > + 0x15, 0x1f, 700000, 100000, 27 }, > { "dldo2", 0x12, (1 << 4), (1 << 4), (0 << 4), > - 0x16, 0x1f, 700000, 100000, 3400000, 200000 }, > + 0x16, 0x1f, 700000, 100000, 27, 3400000, 200000, 5 }, > { "dldo3", 0x12, (1 << 5), (1 << 5), (0 << 5), > - 0x17, 0x1f, 700000, 100000 }, > + 0x17, 0x1f, 700000, 100000, 27 }, > { "dldo4", 0x12, (1 << 6), (1 << 6), (0 << 6), > - 0x18, 0x1f, 700000, 100000 }, > + 0x18, 0x1f, 700000, 100000, 27 }, > { "eldo1", 0x12, (1 << 0), (1 << 0), (0 << 0), > - 0x19, 0x1f, 700000, 50000 }, > + 0x19, 0x1f, 700000, 50000, 25 }, > { "eldo2", 0x12, (1 << 1), (1 << 1), (0 << 1), > - 0x1a, 0x1f, 700000, 50000 }, > + 0x1a, 0x1f, 700000, 50000, 25 }, > { "eldo3", 0x12, (1 << 2), (1 << 2), (0 << 2), > - 0x1b, 0x1f, 700000, 50000 }, > + 0x1b, 0x1f, 700000, 50000, 25 }, > { "fldo1", 0x13, (1 << 2), (1 << 2), (0 << 2), > - 0x1c, 0x0f, 700000, 50000 }, > + 0x1c, 0x0f, 700000, 50000, 16 }, > { "fldo2", 0x13, (1 << 3), (1 << 3), (0 << 3), > - 0x1d, 0x0f, 700000, 50000 }, > + 0x1d, 0x0f, 700000, 50000, 16 }, > { "ldo-io0", 0x90, 0x07, 0x03, 0x04, > - 0x91, 0x1f, 700000, 100000 }, > + 0x91, 0x1f, 700000, 100000, 27 }, > { "ldo-io1", 0x92, 0x07, 0x03, 0x04, > - 0x93, 0x1f, 700000, 100000 }, > + 0x93, 0x1f, 700000, 100000, 27 }, > { NULL } > }; > > const struct axppmic_regdata axp806_regdata[] = { > { "dcdca", 0x10, (1 << 0), (1 << 0), (0 << 0), > - 0x12, 0x7f, 600000, 10000, 1120000, 20000 }, > + 0x12, 0x7f, 600000, 10000, 51, 1120000, 20000, 21 }, > { "dcdcb", 0x10, (1 << 1), (1 << 1), (0 << 1), > - 0x13, 0x1f, 1000000, 50000 }, > + 0x13, 0x1f, 1000000, 50000, 32 }, > { "dcdcc", 0x10, (1 << 2), (1 << 2), (0 << 2), > - 0x14, 0x7f, 600000, 10000, 1120000, 20000 }, > + 0x14, 0x7f, 600000, 10000, 51, 1120000, 20000, 21 }, > { "dcdcd", 0x10, (1 << 3), (1 << 3), (0 << 3), > - 0x15, 0x3f, 600000, 20000, 1600000, 100000 }, > + 0x15, 0x3f, 600000, 20000, 46, 1600000, 100000, 18 }, > { "dcdce", 0x10, (1 << 4), (1 << 4), (0 << 4), > - 0x16, 0x1f, 1100000, 100000 }, > + 0x16, 0x1f, 1100000, 100000, 24 }, > { "aldo1", 0x10, (1 << 5), (1 << 5), (0 << 5), > - 0x17, 0x1f, 700000, 100000 }, > + 0x17, 0x1f, 700000, 100000, 27 }, > { "aldo2", 0x10, (1 << 6), (1 << 6), (0 << 6), > - 0x18, 0x1f, 700000, 100000 }, > + 0x18, 0x1f, 700000, 100000, 27 }, > { "aldo3", 0x10, (1 << 7), (1 << 7), (0 << 7), > - 0x19, 0x1f, 700000, 100000 }, > + 0x19, 0x1f, 700000, 100000, 27 }, > { "bldo1", 0x11, (1 << 0), (1 << 0), (0 << 0), > - 0x20, 0x0f, 700000, 100000 }, > + 0x20, 0x0f, 700000, 100000, 13 }, > { "bldo2", 0x11, (1 << 1), (1 << 1), (0 << 1), > - 0x21, 0x0f, 700000, 100000 }, > + 0x21, 0x0f, 700000, 100000, 13 }, > { "bldo3", 0x11, (1 << 2), (1 << 2), (0 << 2), > - 0x22, 0x0f, 700000, 100000 }, > + 0x22, 0x0f, 700000, 100000, 13 }, > { "bldo4", 0x11, (1 << 3), (1 << 3), (0 << 3), > - 0x23, 0x0f, 700000, 100000 }, > + 0x23, 0x0f, 700000, 100000, 13 }, > { "cldo1", 0x11, (1 << 4), (1 << 4), (0 << 4), > - 0x24, 0x1f, 700000, 100000 }, > + 0x24, 0x1f, 700000, 100000 , 27}, > { "cldo2", 0x11, (1 << 5), (1 << 5), (0 << 5), > - 0x25, 0x1f, 700000, 100000, 3600000, 200000 }, > + 0x25, 0x1f, 700000, 100000, 28, 3600000, 200000, 4 }, > { "cldo3", 0x11, (1 << 6), (1 << 6), (0 << 6), > - 0x26, 0x1f, 700000, 100000 }, > + 0x26, 0x1f, 700000, 100000, 27 }, > { "sw", 0x11, (1 << 7), (1 << 7), (0 << 7) }, > { NULL } > }; > > const struct axppmic_regdata axp809_regdata[] = { > { "dcdc1", 0x10, (1 << 1), (1 << 1), (0 << 1), > - 0x21, 0x1f, 1600000, 100000 }, > + 0x21, 0x1f, 1600000, 100000, 19 }, > { "dcdc2", 0x10, (1 << 2), (1 << 2), (0 << 2), > - 0x22, 0x3f, 600000, 20000 }, > + 0x22, 0x3f, 600000, 20000, 48 }, > { "dcdc3", 0x10, (1 << 3), (1 << 3), (0 << 3), > - 0x23, 0x3f, 600000, 20000 }, > + 0x23, 0x3f, 600000, 20000, 64 }, > { "dcdc4", 0x10, (1 << 4), (1 << 4), (0 << 4), > - 0x24, 0x3f, 600000, 20000, 1800000, 100000 }, > + 0x24, 0x3f, 600000, 20000, 48, 1800000, 100000, 9 }, > { "dcdc5", 0x10, (1 << 5), (1 << 5), (0 << 5), > - 0x25, 0x1f, 1000000, 50000 }, > + 0x25, 0x1f, 1000000, 50000, 32 }, > { "dc5ldo", 0x10, (1 << 0), (1 << 0), (0 << 0), > - 0x1c, 0x07, 700000, 100000 }, > + 0x1c, 0x07, 700000, 100000, 8 }, > { "aldo1", 0x10, (1 << 6), (1 << 6), (0 << 6), > - 0x28, 0x1f, 700000, 100000 }, > + 0x28, 0x1f, 700000, 100000, 27 }, > { "aldo2", 0x10, (1 << 7), (1 << 7), (0 << 7), > - 0x29, 0x1f, 700000, 100000 }, > + 0x29, 0x1f, 700000, 100000, 27 }, > { "aldo3", 0x12, (1 << 5), (1 << 5), (0 << 5), > - 0x2a, 0x1f, 700000, 100000 }, > + 0x2a, 0x1f, 700000, 100000, 27 }, > { "dldo1", 0x12, (1 << 3), (1 << 3), (0 << 3), > - 0x15, 0x1f, 700000, 100000 }, > + 0x15, 0x1f, 700000, 100000, 27, 3400000, 200000, 5 }, > { "dldo2", 0x12, (1 << 4), (1 << 4), (0 << 4), > - 0x16, 0x1f, 700000, 100000 }, > + 0x16, 0x1f, 700000, 100000, 27 }, > { "eldo1", 0x12, (1 << 0), (1 << 0), (0 << 0), > - 0x19, 0x1f, 700000, 100000 }, > + 0x19, 0x1f, 700000, 100000, 27 }, > { "eldo2", 0x12, (1 << 1), (1 << 1), (0 << 1), > - 0x1a, 0x1f, 700000, 100000 }, > + 0x1a, 0x1f, 700000, 100000, 27 }, > { "eldo3", 0x12, (1 << 2), (1 << 2), (0 << 2), > - 0x1b, 0x1f, 700000, 100000 }, > + 0x1b, 0x1f, 700000, 100000, 27 }, > { "ldo_io0", 0x90, 0x07, 0x03, 0x04, > - 0x91, 0x1f, 700000, 100000 }, > + 0x91, 0x1f, 700000, 100000, 27 }, > { "ldo_io1", 0x92, 0x07, 0x03, 0x04, > - 0x93, 0x1f, 700000, 100000 }, > + 0x93, 0x1f, 700000, 100000, 27 }, > { NULL } > }; > > @@ -306,6 +306,7 @@ const struct axppmic_device axppmic_devi > { "x-powers,axp209", "AXP209", axp209_regdata, axp209_sensdata }, > { "x-powers,axp221", "AXP221", axp221_regdata, axp221_sensdata }, > { "x-powers,axp223", "AXP223", axp221_regdata, axp221_sensdata }, > + { "x-powers,axp305", "AXP305", axp806_regdata }, > { "x-powers,axp803", "AXP803", axp803_regdata, axp803_sensdata }, > { "x-powers,axp805", "AXP805", axp806_regdata }, > { "x-powers,axp806", "AXP806", axp806_regdata }, > @@ -511,7 +512,8 @@ axppmic_attach_common(struct axppmic_sof > sc->sc_sensdata = device->sensdata; > > /* Switch AXP806 into master or slave mode. */ > - if (strcmp(name, "x-powers,axp805") == 0 || > + if (strcmp(name, "x-powers,axp305") == 0 || > + strcmp(name, "x-powers,axp805") == 0 || > strcmp(name, "x-powers,axp806") == 0) { > if (OF_getproplen(node, "x-powers,master-mode") == 0 || > OF_getproplen(node, "x-powers,self-working-mode") == 0) { > @@ -626,8 +628,8 @@ struct axppmic_regulator { > uint8_t ar_eval, ar_dval; > > uint8_t ar_vreg, ar_vmask; > - uint32_t ar_base, ar_delta; > - uint32_t ar_base2, ar_delta2; > + uint32_t ar_base, ar_delta, ar_step; > + uint32_t ar_base2, ar_delta2, ar_step2; > > struct regulator_device ar_rd; > }; > @@ -676,6 +678,10 @@ axppmic_attach_regulator(struct axppmic_ > ar->ar_vmask = sc->sc_regdata[i].vmask; > ar->ar_base = sc->sc_regdata[i].base; > ar->ar_delta = sc->sc_regdata[i].delta; > + ar->ar_step = sc->sc_regdata[i].step; > + ar->ar_base2 = sc->sc_regdata[i].base2; > + ar->ar_delta2 = sc->sc_regdata[i].delta2; > + ar->ar_step2 = sc->sc_regdata[i].step2; > > ar->ar_rd.rd_node = node; > ar->ar_rd.rd_cookie = ar; > @@ -694,11 +700,10 @@ axppmic_get_voltage(void *cookie) > > value = axppmic_read_reg(ar->ar_sc, ar->ar_vreg); > value &= ar->ar_vmask; > - voltage = ar->ar_base + value * ar->ar_delta; > - if (ar->ar_base2 > 0 && voltage > ar->ar_base2) { > - value -= (ar->ar_base2 - ar->ar_base) / ar->ar_delta; > - voltage = ar->ar_base2 + value * ar->ar_delta2; > - } > + if (ar->ar_base2 > 0 && value >= ar->ar_step) > + voltage = ar->ar_base2 + (value - ar->ar_step) * ar->ar_delta2; > + else > + voltage = ar->ar_base + value * ar->ar_delta; > return voltage; > } > > @@ -710,17 +715,22 @@ axppmic_set_voltage(void *cookie, uint32 > > if (voltage < ar->ar_base) > return EINVAL; > - value = (voltage - ar->ar_base) / ar->ar_delta; > - if (ar->ar_base2 > 0 && voltage > ar->ar_base2) { > - value = (ar->ar_base2 - ar->ar_base) / ar->ar_delta; > - value += (voltage - ar->ar_base2) / ar->ar_delta2; > + if (ar->ar_base2 > 0 && voltage >= ar->ar_base2) { > + value = (voltage - ar->ar_base2) / ar->ar_delta2; > + if (value >= ar->ar_step2) > + return EINVAL; > + value += ar->ar_step; > + } else { > + value = (voltage - ar->ar_base) / ar->ar_delta; > + if (value >= ar->ar_step) > + return EINVAL; > } > if (value > ar->ar_vmask) > return EINVAL; > > reg = axppmic_read_reg(ar->ar_sc, ar->ar_vreg); > - reg &= ~ar->ar_vmask; > - axppmic_write_reg(ar->ar_sc, ar->ar_vreg, reg | value); > + axppmic_write_reg(ar->ar_sc, ar->ar_vreg, > + (reg & ~ar->ar_vmask) | (value & ar->ar_vmask)); The indentation is wrong here (should be just 4 spaces). > return 0; > }