| 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
| ||
|
Date: Tue, 2 Apr 2019 14:16:19 +0800
From: Chris Chiu <chiu@...lessm.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Mika Westerberg <mika.westerberg@...ux.intel.com>,
Daniel Drake <drake@...lessm.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
"open list:PIN CONTROL SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel <linux-kernel@...r.kernel.org>,
Linux Upstreaming Team <linux@...lessm.com>
Subject: Re: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume
On Mon, Apr 1, 2019 at 8:23 PM Andy Shevchenko
<andriy.shevchenko@...el.com> wrote:
>
> On Mon, Apr 01, 2019 at 06:41:57PM +0800, Chris Chiu wrote:
>
> Thanks for the patch.
> My comments below.
>
> > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> > b/drivers/pinctrl/intel/pinctrl-intel.c
> > index 8cda7b535b02..d1cfa5adef9b 100644
> > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > @@ -77,6 +77,7 @@ struct intel_pad_context {
> > u32 padcfg0;
> > u32 padcfg1;
> > u32 padcfg2;
>
> > + u32 hostown;
>
> This is wrong. We have one register per entire (*) group of pins to keep host
> ownership. Basically it's a mask.
>
> *) if it's <= 32, otherwise there are more registers. But in any case it's 1
> bit per pin, and not 32 bits.
>
> > for (i = 0; i < pctrl->soc->npins; i++)
>
> Thus, the actual actions should mimic what we do for interrupt mask.
>
> --
> With Best Regards,
> Andy Shevchenko
>
Thanks for the comment. My first version did mimic the logic of the interrupt
mask restore but it was based on the DMI quirk. It saves HOSTSW_OWN
for each padgroup and restores them all after resume if DMI info matched.
What really confused me is how to do this specifically for a requested GPIO
pin. So here's my new proposed patch. Please suggests if there's any better
idea. Thanks.
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -81,6 +81,7 @@ struct intel_pad_context {
struct intel_community_context {
u32 *intmask;
+ u32 *hostown;
};
struct intel_pinctrl_context {
@@ -117,6 +118,10 @@ struct intel_pinctrl {
#define pin_to_padno(c, p) ((p) - (c)->pin_base)
#define padgroup_offset(g, p) ((p) - (g)->base)
+#ifdef CONFIG_PM_SLEEP
+static void intel_save_hostown(struct intel_pinctrl *pctrl, unsigned int pin);
+#endif
+
static struct intel_community *intel_get_community(struct intel_pinctrl *pctrl,
unsigned int pin)
{
@@ -456,7 +461,9 @@ static int intel_gpio_request_enable(struct
pinctrl_dev *pctldev,
intel_gpio_set_gpio_mode(padcfg0);
/* Disable TX buffer and enable RX (this will be input) */
__intel_gpio_set_direction(padcfg0, true);
+#ifdef CONFIG_PM_SLEEP
+ intel_save_hostown(pctrl, pin);
+#endif
raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return 0;
@@ -1284,7 +1291,7 @@ static int intel_pinctrl_pm_init(struct
intel_pinctrl *pctrl)
for (i = 0; i < pctrl->ncommunities; i++) {
struct intel_community *community = &pctrl->communities[i];
- u32 *intmask;
+ u32 *intmask, *hostown;
intmask = devm_kcalloc(pctrl->dev, community->ngpps,
sizeof(*intmask), GFP_KERNEL);
@@ -1292,6 +1299,13 @@ static int intel_pinctrl_pm_init(struct
intel_pinctrl *pctrl)
return -ENOMEM;
communities[i].intmask = intmask;
+
+ hostown = devm_kcalloc(pctrl->dev, community->ngpps,
+ sizeof(*hostown), GFP_KERNEL);
+ if (!hostown)
+ return -ENOMEM;
+
+ communities[i].hostown= hostown;
}
pctrl->context.pads = pads;
@@ -1447,6 +1461,43 @@ int intel_pinctrl_probe_by_uid(struct
platform_device *pdev)
EXPORT_SYMBOL_GPL(intel_pinctrl_probe_by_uid);
#ifdef CONFIG_PM_SLEEP
+static void intel_save_hostown(struct intel_pinctrl *pctrl, unsigned int pin)
+{
+ const struct intel_community *community;
+ const struct intel_padgroup *padgrp;
+ int i;
+
+ community = intel_get_community(pctrl, pin);
+ if (!community)
+ return;
+ if (!community->hostown_offset)
+ return;
+
+ padgrp = intel_community_get_padgroup(community, pin);
+ if (!padgrp)
+ return;
+
+ for (i = 0; i < pctrl->ncommunities; i++) {
+ const struct intel_community *comm = &pctrl->communities[i];
+ int j;
+
+ for (j = 0; j < comm->ngpps; j++) {
+ const struct intel_padgroup *pgrp = &comm->gpps[j];
+
+ if (padgrp == pgrp) {
+ struct intel_community_context *communities;
+ void __iomem *base;
+
+ communities = pctrl->context.communities;
+ base = community->regs +
community->hostown_offset;
+ communities[i].hostown[j] = readl(base + j * 4);
+ break;
+ }
+ }
+ }
+ return;
+}
+
static bool intel_pinctrl_should_save(struct intel_pinctrl *pctrl,
unsigned int pin)
{
const struct pin_desc *pd = pin_desc_get(pctrl->pctldev, pin);
@@ -1588,6 +1639,17 @@ int intel_pinctrl_resume(struct device *dev)
dev_dbg(dev, "restored mask %d/%u %#08x\n", i, gpp,
readl(base + gpp * 4));
}
+
+ base = community->regs + community->hostown_offset;
+ for (gpp = 0; gpp < community->ngpps; gpp++) {
+ if (communities[i].hostown[gpp] &&
+ communities[i].hostown[gpp] != readl(base
+ gpp * 4)) {
+ writel(communities[i].hostown[gpp],
base + gpp * 4);
+ dev_warn(dev, "hostown changed after resume\n");
+ dev_dbg(dev, "restored hostown %d/%u
%#08x\n", i, gpp,
+ readl(base + gpp * 4));
+ }
+ }
}
return 0;
Powered by blists - more mailing lists