linux-kernel - RE: [PATCH v2] pinctrl: add support for group and pinmux states

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]

Message-ID: <74CDBE0F657A3D45AFBB94109FB122FF176BE92C1B@HQMAIL01.nvidia.com>
Date:	Mon, 19 Dec 2011 16:24:50 -0800
From:	Stephen Warren <swarren@...dia.com>
To:	Linus Walleij <linus.walleij@...ricsson.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Grant Likely <grant.likely@...retlab.ca>,
	Barry Song <21cnbao@...il.com>,
	Shawn Guo <shawn.guo@...escale.com>,
	Thomas Abraham <thomas.abraham@...aro.org>,
	Dong Aisheng <dong.aisheng@...aro.org>,
	Rajendra Nayak <rajendra.nayak@...aro.org>,
	Haojian Zhuang <haojian.zhuang@...vell.com>,
	Linus Walleij <linus.walleij@...aro.org>
Subject: RE: [PATCH v2] pinctrl: add support for group and pinmux states

Linus Walleij wrote at Sunday, December 18, 2011 4:07 PM:
> This makes it possible for pin groups and pin muxes to go into
> different states, which is especially useful for power management,
> both runtime and across suspend/resume.

> diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt

> +Pin group states
> +================
> +
> +To simplify handling of specific group states, such as when a group of
> +pins need to be deactivated or put in a specific sleep state, the pin controller
> +may implement a .pin_group_set_state() callback. This will for example be
> +called automatically to set a certain pin group in active state when the groups
> +are selected for use in a certain pin multiplexing configuration (see below).
> +This is a good opportunity to set up any default pin configuration (see below)
> +for the pins in a group.

I don't think it's possible for a pinctrl driver to implement such a
callback; the appropriate state for a pin may well be board-specific,
and the pinctrl driver itself can't possibly know what that state is.

For example, for some random pin that a board happens to be using as a
GPIO - how does the pinctrl driver know whether that GPIO actually does
something while in suspend; if the GPIO isn't used, the SoC can probably
just turn off all drivers and pull-up/downs on the pin, but if the GPIO
is used to control some aspect of a PMIC, it probably needs to be in a
specific state during suspend. This kind of thing is entirely board-
specific.

Similarly, the sequencing of reprogramming of the various pins when
entering/leaving suspend is most likely board specific, e.g. there may
be a need to assert a "speaker mute" GPIO before de-asserting a GPIO
that enables power to speaker amplifiers etc.

> diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h

> +enum pin_group_state {
> +	PIN_GROUP_STATE_DISABLED,
> +	PIN_GROUP_STATE_ACTIVE,
> +	PIN_GROUP_STATE_IDLE,
> +	PIN_GROUP_STATE_SLEEP,
> +};

I'm unsure that having the pinctrl core explicitly define the set of
legal states is the right approach. This prevents domain-specific states
(e.g. PCIe power states say) from being defined without revising the
core. That said, I'm less adamant about this than about my first point
above.

I'd like to suggest an alternative:

a) The mapping table should be enhanced to cover all configuration that
   pinctrl can program:
   * Mux function selection
   * Whether a pin a used as a GPIO, if gpiolib doesn't interact with
     pinctrl to set this up.
   * Any pin config parameters that should be set.
   * Define all the above for each state that a driver can be in.
   * Name legal states as strings in the mapping table, so they're just
     data and any given driver can define the names of states it expects
     to have available.
   * The table can have a defined execution ordering this allowing board-
     specific sequencing requirements to be defined. Perhaps the mapping
     table can define the set of parameters to program for each legal
     to/from state transition combination, rather than target
     configuration for each state, so all sequencing is fully under
     control of the mapping table.
b) Add a pinctrl API that transitions a given pmx (from pinmux_get())
   between states as defined in the mapping table.

That should allow this to be fully generic.

For standard states (e.g. fully active, system suspend) we could define
(de-facto?) standardized names that drivers can use if appropriate.
That'd prevent too many custom state strings, yet allow extensibility
when needed.

IIRC, some of my previous emails have touched on this idea, and included
example mapping tables with multiple states per device.

-- 
nvpublic

--
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/