linux-kernel - Re: [RFC PATCH 3/3] dt: Document: Add optional MAX77686 operating mode bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 10 Dec 2012 10:30:42 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Abhilash Kesavan <a.kesavan@...sung.com>
Cc:	lrg@...com, broonie@...nsource.wolfsonmicro.com,
	linux-kernel@...r.kernel.org, olof@...om.net,
	grant.likely@...retlab.ca, thomas.abraham@...aro.org,
	kgene.kim@...sung.com, sameo@...ux.intel.com
Subject: Re: [RFC PATCH 3/3] dt: Document: Add optional MAX77686 operating
 mode bindings

Abhilash,

Thanks for posting up these patches.  Just going to do my commenting
directly on the documentation patch since they are high-level
comments.

On Sun, Dec 9, 2012 at 10:26 PM, Abhilash Kesavan <a.kesavan@...sung.com> wrote:
> Add documenatation for various operating mode capabilities of
> the MAX77686 PMIC.
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@...sung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max77686.txt |    6 ++++++

This should probably be squashed with PATCH 2/3.  It seems to be best
to introduce documentation in the same patch as first usage.

>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> index c6a3469..f2867a9 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -13,6 +13,12 @@ Required properties:
>  - interrupts : This i2c device has an IRQ line connected to the main SoC.
>  - interrupt-parent : The parent interrupt controller.
>
> +optional properties:
> +- max77686-opmode : The regulators of max77686 have various operating mode
> +  capabilities such as low power mode, standby mode (controlled by PWRREQ
> +  signal) etc. Check the regulator CTRL register for the bits setting these
> +  modes.

I have a feeling we're going to want something a little more generic
than just whacking a register value straight into the device tree.

I would propose the following for LDOs to match the generic regulator
define, but someone with more device tree experience may have a better
idea.  The idea is to map to the generic set_suspend_mode() /
set_mode():
  regulator-suspend-mode = <REGULATOR_MODE_XXX constant>
  regulator-mode = <REGULATOR_MODE_XXX constant>

The max77686 appears to support NORMAL and IDLE mode for most LDOs, so
the only reasonable values for these two would be 2 and 4.  It appears
that it would also be nonsensical (impossible to map to a register
value) to have REGULATOR_MODE_NORMAL for suspend and
REGULATOR_MODE_IDLE for the normal running mode.

For bucks 1-3, you'd just have a boolean property mapping to
set_suspend_disable().  Perhaps:
  regulator-suspend-en;

That wouldn't allow access to the "Forced low power mode" of bucks
1-3, but I don't think there's any code to support that now so nobody
must be using it.

Given that all of the above maps nicely into regulator constants and
regulator functions, I'd imagine that support could be added straight
into the generic regulator code and you won't have to touch max77686
at all (unless you wanted to add support for set_mode(), which doesn't
seem to be there).

All of the above would set _defaults_ for regulators.  If device-tree
people think it's wise, we could add "-default" into the names.

> +
>  Optional node:
>  - voltage-regulators : The regulators of max77686 have to be instantiated
>    under subnode named "voltage-regulators" using the following format.
> --
> 1.7.8.6
>

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/