Hi Mark,
Sorry for the late reply as I was sick and still recovering.
On 11 December 2013 00:24, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Tue, Dec 10, 2013 at 11:59:18AM +0530, Sachin Kamat wrote:
>
>> Added PMIC node to Arndale-Octa board.
>
> Is there a bootloader posted for this anywhere yet?
One version available here from Insignal:
git://git.insignal.co.kr/insignal/arndale_octa-jb_mr1.1/u-boot
>
>> + regulators {
>> + ldo1_reg: LDO1 {
>> + regulator-name = "vdd_ldo1";
>
> So, there is a label on the schematic for PVDD_LDO1 but it's not
> entirely obvious why that's there as throughout the rest of the
> schematic it's got the rather more constructive name PVDD_ALIVE_1V0.
> We should really be using the more readable versions of the names
> in the DT since the whole point in overriding the driver default
> names is to make it easier to associate the kernel diagnostics with
> the schematic, what the DT is doing here serves no purpose.
>
> The same comment applies to all the LDO supplies.
I will update with the appropriate labels.
>
>> + buck2_reg: BUCK2 {
>> + regulator-name = "vdd_arm";
>> + regulator-min-microvolt = <800000>;
>> + regulator-max-microvolt = <1500000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>
> Why are you specifying boot-on? It doesn't seem terribly plausible that
> the cores won't be powered on boot, or that we can't tell what the state
> is by reading the registers.
>
> Same for all the DCDCs.
Yes. Right. Will remove them.
>
>> + buck7_reg: BUCK7 {
>> + regulator-name = "vdd_1.0v_ldo";
>> + regulator-min-microvolt = <800000>;
>> + regulator-max-microvolt = <1500000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>
> This doesn't correspond to the schematic which says that BUCK7 is
> supplying VIN_LLDO_1V4 (the input for some of the LDOs). The voltage
> range specified there doesn't seem terribly sensible either, are you
> sure you haven't just got the underlying voltage range for the DCDC?
> I can't see why you'd want to raise the voltage over 1.4V (that's rather
> defeating the point) and while it'd be nice to have the core be able to
> vary supply to track the headroom needed by the LDOs none of the LDOs
> were configured with variable voltage so even if we implemented that
> it'd never be used.
That was definitely a mistake. It should be 1.4V Will fix it.
>
> Similar issues apply to the other bucks being used to drop the battery
> voltage for LDO supplies.
Will check and update others appropriately. Thanks for your review and
pointing out the errors.
--
With warm regards,
Sachin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
