linux-kernel - Re: [PATCH] pinctrl: mvebu: armada-38x: add suspend/resume support

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 19 Oct 2015 09:23:20 +0200
From:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:	Marcin Wojtas <mw@...ihalf.com>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Andrew Lunn <andrew@...n.ch>,
	Jason Cooper <jason@...edaemon.net>,
	Tawfik Bayouk <tawfik@...vell.com>,
	Grzegorz Jaszczyk <jaz@...ihalf.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel@...r.kernel.org, nadavh@...vell.com,
	linux-gpio@...r.kernel.org, Lior Amsalem <alior@...vell.com>,
	Gregory Clément 
	<gregory.clement@...e-electrons.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH] pinctrl: mvebu: armada-38x: add suspend/resume support

Hello,

On Mon, 19 Oct 2015 08:04:49 +0200, Marcin Wojtas wrote:

> > I don't like this. The mvebu_pinctrl_soc_info structure is meant to be
> > a read-only structure that only describes static information giving
> > SoC-specific details for pin-muxing. The idea is that in the event
> > where you had multiple pinctrl in the same system, you would still have
> > only one instance of mvebu_pinctrl_soc_info.
> 
> Ok, understood. What with current static globals, like mpp_base? This
> is a problem when we consider hypothetical multi-pintrl system...

The current driver is indeed not designed for multiple instances of the
same pinctrl controller. But that's exactly what Russell is asking for.

> In genereal, I think storing additional global data is not
> starightforward, as dev->platform_data and dev->driver_data are
> currently occupied by mvebu_pinctrl and mvebu_pinctrl_soc_info. I
> propose the following:
>
> 
> 1. Create a new structure:
> struct mvebu_pinctrl_pm_info {

This definitely shouldn't be called "pm_info", because 'base' is not PM
related. It should be mvebu_pinctrl_state or something like that.

>     void __iomem *base;
>     static u32 *mpp_saved_regs;
>     int nregs;
> }
> 
> 2. Add new field to struct mvebu_pinctrl:
> struct mvebu_pinctrl_pm_info *pm_info;

Does not work because "mvebu_pinctrl_pm_info" cannot be a generic
structure, it has to be a per-SoC driver structure, since the set of
registers to save for PM reasons is different from one SoC to the
other. Also, some SoC have only one "base" pointer, some others (like
Dove) have multiple.

So it should be the other way around: the SoC-specific driver create a
structure, and this structure points back to the mvebu_pinctrl
structure.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/