lkml.org 
[lkml]   [2019]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume
On Tue, Apr 02, 2019 at 02:16:19PM +0800, Chris Chiu wrote:
> On Mon, Apr 1, 2019 at 8:23 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Mon, Apr 01, 2019 at 06:41:57PM +0800, Chris Chiu wrote:

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

> struct intel_community_context {
> u32 *intmask;
> + u32 *hostown;

This is okay.

> };

> +#ifdef CONFIG_PM_SLEEP
> +static void intel_save_hostown(struct intel_pinctrl *pctrl, unsigned int pin);
> +#endif
> +

No need for this...

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

...and for this.
Just save all of them at ->suspend()

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

This is good.

> }

> +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;

Useless.

> +}

This is too complicated. Just add

base = community->regs + community->hostown_offset;
for (gpp = 0; gpp < community->ngpps; gpp++)
communities[i].hostown[gpp] = readl(base + gpp * 4);

into ->suspend() loop.

> + 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));
> + }
> + }

Instead you may need to loop over each pin in the part of the group related to
one 32-bit HOSTSW_OWN register (i.e. 8, see PADOWN_*() macros in the driver),
check if it's requested and break a loop. If loop index is off-by-one a limit,
nothing to do, otherwise restore hostown register.

More pedantic approach is to collect the mask inside the loop and apply it.

The check function name is gpiochip_is_requested().

(One of Intel's drivers which is using that at ->resume() is
drivers/gpio/gpio-lynxpoint.c)

P.S. I prefer pedantic approach. The simplification one is showed in order to
give you an idea.

--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2019-04-02 13:59    [W:0.071 / U:34.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site