Re: [PATCH v3 07/10] ARM: sunxi: dt: Add x-powers-axp209.dtsi file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 28, 2014 at 01:34:38PM +0000, Mark Brown wrote:
> On Thu, Mar 27, 2014 at 10:29:21PM +0100, Carlo Caione wrote:
> > This dtsi describes the axp209 PMIC, and is to be included from inside
> > the i2c controller node to which the axp209 is connected.
> 
> >  arch/arm/boot/dts/x-powers-axp209.dtsi | 54 ++++++++++++++++++++++++++++++++++
> 
> Something is wrong here.  Either the changelog is inaccurate or this is
> broken, either way this should be cleaned up because this looks like
> very bad practice.  If this is a common include file for boards derived
> from some reference design then the changelog is misleading and the DT
> should be clearer about the board tie ins.  Otherwise the .dtsi is
> defining what should be board specific parameters.
> 
> The fact that the DT names everything after the regulator on the PMIC
> and not after the supply names on the board is especially suspicious and
> glancing at a couple of the regulators it looks like the constraints
> here are the maximum ranges the PMIC supports rather than anything to do
> with any board.
> 
> Please take a step back and think about what the regulator constraints
> are intended to do - they're about matching the regulator capabilities
> to the board since it is rare for boards to be able to do everything the
> regulator can support.  Duplicating the basic device capabilities into
> the DT would be a pointless waste of time.
> 
> > +	axp_dcdc2: dcdc2 {
> > +		regulator-min-microvolt = <700000>;
> > +		regulator-max-microvolt = <2275000>;
> > +		regulator-always-on;
> > +	};
> 
> What is this configuration actually for - what guarantee do we have that
> the above is safe for a given board using this regulator?  
> 
> In general I'd expect to see anything specifying variability for a
> regulator to also include the relevant consumer node.

Ok, you are completely right. These values in the DTSI are wrongly the
maximum and minimum ranges for the PMIC. Even worst I didn't specify the
board specific parameters for the regulators.

Thank you for reviewing and noticing it,

-- 
Carlo Caione
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux