From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Chiu Subject: Re: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume Date: Wed, 3 Apr 2019 15:06:43 +0800 Message-ID: References: <20190327172940.GR3622@lahna.fi.intel.com> <20190328091729.GV9224@smile.fi.intel.com> <20190328123444.GX3622@lahna.fi.intel.com> <20190401074953.GQ3622@lahna.fi.intel.com> <20190401122256.GF9224@smile.fi.intel.com> <20190402115836.GL9224@smile.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20190402115836.GL9224@smile.fi.intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Andy Shevchenko Cc: Mika Westerberg , Daniel Drake , Heikki Krogerus , Linus Walleij , "open list:PIN CONTROL SUBSYSTEM" , Linux Kernel , Linux Upstreaming Team List-Id: linux-gpio@vger.kernel.org On Tue, Apr 2, 2019 at 7:58 PM Andy Shevchenko wrote: > > + 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 > > Thanks for your great comment. I remove the useless hostown save function and make the following change in ->resume() to detect and restore the abnormal HOSTSW_OWN. Please help comment if there're still problems. Thanks. @@ -1588,6 +1600,29 @@ 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++) { + const struct intel_padgroup *padgrp = &community->gpps[i]; + unsigned int requested = 0; + int j; + + if (padgrp->gpio_base < 0) + continue; + + for (j = 0; j < padgrp->size; j++) + if (gpiochip_is_requested(&pctrl->chip, padgrp->gpio_base + j)) + requested |= BIT(j); + + if (requested) { + if (communities[i].hostown[gpp] != readl(base + gpp * 4)) { + writel(communities[i].hostown[gpp], base + gpp * 4); + dev_warn(dev, "hostown been changed during resume\n"); + dev_dbg(dev, "restored hostown %d/%u %#08x\n", i, gpp, + readl(base + gpp * 4)); + } + } + } } return 0;