Hi Przemyslaw, On 30 July 2015 at 02:22, Przemyslaw Marczak <p.marc...@samsung.com> wrote: > Hello Simon, > > > On 07/30/2015 04:05 AM, Simon Glass wrote: >> >> Hi Przemyslaw, >> >> On 10 July 2015 at 05:53, Przemyslaw Marczak <p.marc...@samsung.com> >> wrote: >>> >>> >>> Hello Simon, >>> >>> On 07/03/2015 02:16 AM, Simon Glass wrote: >>>> >>>> >>>> Add support for all BUCK regulators, now that the correct register is >>>> accessed for each. >>>> >>>> Signed-off-by: Simon Glass <s...@chromium.org> >>>> >>>> --- >>>> >>>> drivers/power/regulator/max77686.c | 10 +++------- >>>> 1 file changed, 3 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/power/regulator/max77686.c >>>> b/drivers/power/regulator/max77686.c >>>> index 21173fc..427b717 100644 >>>> --- a/drivers/power/regulator/max77686.c >>>> +++ b/drivers/power/regulator/max77686.c >>>> @@ -81,11 +81,7 @@ static int max77686_buck_volt2hex(int buck, int uV) >>>> /* hex = (uV - 600000) / 12500; */ >>>> hex = (uV - MAX77686_BUCK_UV_LMIN) / >>>> MAX77686_BUCK_UV_LSTEP; >>>> hex_max = MAX77686_BUCK234_VOLT_MAX_HEX; >>>> - /** >>>> - * Those use voltage scaller - temporary not implemented >>>> - * so return just 0 >>>> - */ >>>> - return -ENOSYS; >>>> + break; >>>> default: >>>> /* hex = (uV - 750000) / 50000; */ >>>> hex = (uV - MAX77686_BUCK_UV_HMIN) / >>>> MAX77686_BUCK_UV_HSTEP; >>>> @@ -379,11 +375,11 @@ static int max77686_buck_val(struct udevice *dev, >>>> int op, int *uV) >>>> case 2: >>>> case 3: >>>> case 4: >>>> - /* Those use voltage scallers - will support in the >>>> future */ >>>> mask = MAX77686_BUCK234_VOLT_MASK; >>>> - return -ENOSYS; >>>> + break; >>>> default: >>>> mask = MAX77686_BUCK_VOLT_MASK; >>>> + break; >>>> } >>>> >>>> ret = pmic_read(dev->parent, adr, &val, 1); >>>> >>> >>> The bucks 2,3,4 can work in DVS mode, which allows select one of eight >>> DVS regulators for each output. The default selection at power-on is DVS1 >>> for each output, and it corresponds to the currently defined output register >>> addresses. >>> >>> The selection can be done by six PMIC's GPIOs: >>> - DVS1/2/3 - output selection: 0x0=DVS1...0x7=DVS8 >>> - SELB2/3/4 - mode switch: 'DVS' or 'no DVS' >>> >>> Reading or writing the default registers is proper only in case: >>> - for the default PMIC's power-up setting - may conflict with bl1/bl2 >>> - when DVS1/2/3 GPIOs are set to LOW - DVS1 selected >>> - SELB2/3/4 - are set to LOW - no DVS mode >>> >>> The documentation is poor, but if I good understand, the SELB is used as >>> "latch" for the DVS selection. >>> >>> So the driver, could be unreliable for these outputs if it doesn't check >>> the PMIC's GPIOs. >>> >>> It's quite confusing, since the PMIC, doesn't provide registers to check >>> those GPIOs. It should be checked by the driver and can be delivered by >>> device-tree. >>> >>> This is also confusing, since it depends on board design, because the >>> PMIC's GPIOs can be connected to the SoCs GPIOs and also just pulled to >>> POWER/GND signals. >>> >>> The documentation says, that those GPIOs should be set accordingly, and >>> for example Odroid U3 has connected the SELB to VDD_IO(LDO3) power line, so >>> actually this state can not be changed or can be changed by accident when >>> changing the VDD_IO - which is HIGH at PMIC's power-up. >>> >>> The switching is impossible, since the VDD_IO line is shared with few >>> other peripherals. >>> >>> In this case, maybe you should add config to allow use of the BUCK234 >>> only for case in which DVS mode is disabled by board design (SELB2/3/4 set >>> to LOW). This may also work, when the GPIOs of Exynos stays in the reset >>> state. >>> >>> Then, using the default 'buck_out' registers: 0x14, 0x1e, 0x28 is >>> reasonable. >> >> >> I don't see anything in the binding. I have added a comment in the >> driver to explain this limitation. However I haven't actually seen >> hardware that makes use of it. Are you saying that Odroid U3 does use >> it, or just that it has the lines connected up incorrectly? >> > > Right, the binding for those GPIOs don't exists, since change of those > registers values was not supported by the driver before. > > I looked into the bl2 code from the Hardkernel's U-Boot (we are using it's > signed bl2 binary for U3). There is a code for smdk4212/smdk4412, which is > reused for Odroid U3 and the bl2 MAX77686 driver configures the DVS/SELB > gpios. Unfortunately the GPIO is invalid, probably it is valid for smdk. > > Code: > https://github.com/hardkernel/u-boot/blob/odroid-v2010.12/board/samsung/smdk4212/pmic.c#L707 > > > In the hardkernel's code we can also see the MAX77686 driver for smdk5250, > which also sets the DVS/SELB by GPD/GPX pins in a proper way. > > Code: > https://github.com/hardkernel/u-boot/blob/odroid-v2010.12/board/samsung/smdk5250/pmic.c#L546 >
That is a good collection of hackery! The question is, should I do anything here? Things seem to work OK in my testing with the patch as it is. > >>> >>> Did you tried measure the output voltage, after its change? >> >> >> No I have not measured it, only verified that the code looks correct. >> If you are able to do that I think it would be a good test. >> > > At present I can't measure it, but it should be verified in the future. > But if you can notice a stability change after setting those registers, then > we can assume, that it works. OK, sounds good. I can read the values back at least. I definitely had it die before I added the voltage stuff so it seems to be having an effect. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
