Re: [PATCH 1/3] ARM: dts: imx6: phycore-som: fix arm and soc minimum voltage

["Stefan Riedmüller" <s.riedmueller@phytec.de>]

Hi Lucas, hi Marco,

On 03.12.19 12:44, Lucas Stach wrote:
> On Di, 2019-12-03 at 09:33 +0100, Marco Felsch wrote:
> [...]
>>>> Please check the following structs:
>>>>
>>>>    - static const struct da9062_regulator_info local_da9061_regulator_info[]
>>>>    - static const struct da9062_regulator_info local_da9062_regulator_info[]
>>>>
>>>> There you have the min_uV, uV_step, n_voltages so the core can validate
>>>> if the dt-value is within the range.
>>>
>>> Thanks, that makes more sense.
>>>
>>>>> The regulator bindings state:
>>>>> - regulator-min-microvolt: smallest voltage consumers may set
>>>>>
>>>>> - regulator-max-microvolt: largest voltage consumers may set
>>>>
>>>> Yes and according the datasheet I mentionied the current values aren't
>>>> correct.
>>>>
>>>>> For me that is device depended and not design depended.
>>>>>
>>>>> What is the scenario you're thinking about which would cause the SOC, as a
>>>>> consumer, to request a lower voltage as it needs?
>>>>
>>>> The thing is that the DT abstracts the HW and these values are not
>>>> correct. As mentioned in my commit message the values should meet
>>>> the datasheet restrictions and this isn't the case yet.
>>>
>>> I don't agree. The datasheet you mention is the i.MX 6 datasheet and thus
>>> the limitation should reside in the i.MX 6 regulators and not in the PMIC's.
>>> This limitation is not just valid in combination with that PMIC but for all
>>> i.MX 6.
>>
>> The datasheet tells you which voltage should be applied to the imx6 and
>> so you have to set this here. What happens if the internally ldo
>> request a voltage value below 0.9V? Then the value will be applied
>> because we specified 0.73V and the system don't work anymore or did you
>> verified that case?
> 
> The DT constraints are supposed to reflect absolute maximum ratings of
> the board design. The regulator driver already knows the limits of the
> PMIC chip, so there is no point in duplicating this information in the
> DT.
> 
> The DT constraints are there to make sure the regulator core can
> constrain the voltage setting to something safe for the specific
> design. A consumer driver bug must never be able to set a voltage that
> is outside of the absolute maximum ratings of _all_ consumers of this
> specific power rail. I know that a lot of DTs get this detail wrong,
> but we shouldn't block patches to fix this. :)

Thanks for the clarification and the example. I got it now and will keep it 
in mind for the future.

> 
>>> If I have this wrong and the maintainers agree with you could you please
>>> make sure to account for the bypass mode as well since these values from the
>>> datasheet are valid too?
>>
>> As I said, the bypass mode isn't supported by the driver and all imx6
>> based devicetrees follow that case. So we don't have to take that into
>> account. Also we can't meet both contraints with one dt and futhermore
>> the bypass mode decrease your imx6 lifetime due the the increased ripple
>> on the arm-core supply. So I think no one wants this setup in the near
>> future.
> 
> As a violation of the minimum voltage setting is very unlikely to cause
> any permanent damage to the design (expect if you got reverse voltage
> flows somewhere) I think it is safe to include the LDO bypass supply
> limits as the lower bound in the DT constraints, even if this mode
> isn't currently used anywhere.

Sounds good to me.

Regards,
Stefan

> 
> Regards,
> Lucas
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel