[PATCH v2 0/5] pinctrl: intel: Refactor register restoring on resume

[PATCH v2 0/5] pinctrl: intel: Refactor register restoring on resume

[Andy Shevchenko]

Refactor ->resume() path to be a bit smarter and less noisy.

In v2:
- add a patch to reduce a noise in kernel log
- drop warning to debug level for one message
- constify local pointers (Mika)

Andy Shevchenko (5):
  pinctrl: intel: Introduce intel_restore_padcfg() helper
  pinctrl: intel: Introduce intel_restore_hostown() helper
  pinctrl: intel: Introduce intel_restore_intmask() helper
  pinctrl: intel: Drop level from warning to debug in
    intel_restore_hostown()
  pinctrl: intel: Use helper to restore register values on ->resume()

 drivers/pinctrl/intel/pinctrl-intel.c | 115 ++++++++++++++------------
 1 file changed, 61 insertions(+), 54 deletions(-)

-- 
2.23.0

[PATCH v2 1/5] pinctrl: intel: Introduce intel_restore_padcfg() helper

[Andy Shevchenko]

Deduplicate restoring PADCFGx registers by using a common helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 51 +++++++++++++--------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 83981ad66a71..181e5c8c1855 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1607,6 +1607,27 @@ intel_gpio_update_pad_mode(void __iomem *hostown, u32 mask, u32 value)
 	return curr;
 }
 
+static void intel_restore_padcfg(struct intel_pinctrl *pctrl, unsigned int pin,
+				 unsigned int reg, u32 saved)
+{
+	u32 mask = (reg == PADCFG0) ? PADCFG0_GPIORXSTATE : 0;
+	unsigned int n = reg / sizeof(u32);
+	struct device *dev = pctrl->dev;
+	void __iomem *padcfg;
+	u32 value;
+
+	padcfg = intel_get_padcfg(pctrl, pin, reg);
+	if (!padcfg)
+		return;
+
+	value = readl(padcfg) & ~mask;
+	if (value == saved)
+		return;
+
+	writel(saved, padcfg);
+	dev_dbg(dev, "restored pin %u padcfg%u %#08x\n", pin, n, readl(padcfg));
+}
+
 int intel_pinctrl_resume_noirq(struct device *dev)
 {
 	struct intel_pinctrl *pctrl = dev_get_drvdata(dev);
@@ -1620,37 +1641,13 @@ int intel_pinctrl_resume_noirq(struct device *dev)
 	pads = pctrl->context.pads;
 	for (i = 0; i < pctrl->soc->npins; i++) {
 		const struct pinctrl_pin_desc *desc = &pctrl->soc->pins[i];
-		void __iomem *padcfg;
-		u32 val;
 
 		if (!intel_pinctrl_should_save(pctrl, desc->number))
 			continue;
 
-		padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG0);
-		val = readl(padcfg) & ~PADCFG0_GPIORXSTATE;
-		if (val != pads[i].padcfg0) {
-			writel(pads[i].padcfg0, padcfg);
-			dev_dbg(dev, "restored pin %u padcfg0 %#08x\n",
-				desc->number, readl(padcfg));
-		}
-
-		padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG1);
-		val = readl(padcfg);
-		if (val != pads[i].padcfg1) {
-			writel(pads[i].padcfg1, padcfg);
-			dev_dbg(dev, "restored pin %u padcfg1 %#08x\n",
-				desc->number, readl(padcfg));
-		}
-
-		padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG2);
-		if (padcfg) {
-			val = readl(padcfg);
-			if (val != pads[i].padcfg2) {
-				writel(pads[i].padcfg2, padcfg);
-				dev_dbg(dev, "restored pin %u padcfg2 %#08x\n",
-					desc->number, readl(padcfg));
-			}
-		}
+		intel_restore_padcfg(pctrl, desc->number, PADCFG0, pads[i].padcfg0);
+		intel_restore_padcfg(pctrl, desc->number, PADCFG1, pads[i].padcfg1);
+		intel_restore_padcfg(pctrl, desc->number, PADCFG2, pads[i].padcfg2);
 	}
 
 	communities = pctrl->context.communities;
-- 
2.23.0

Re: [PATCH v2 1/5] pinctrl: intel: Introduce intel_restore_padcfg() helper

[Linus Walleij]

On Tue, Oct 22, 2019 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Deduplicate restoring PADCFGx registers by using a common helper.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

These look good to me, expecting to get them through a pull
request.

Yours,
Linus Walleij

[PATCH v2 2/5] pinctrl: intel: Introduce intel_restore_hostown() helper

[Andy Shevchenko]

Refactor restoring HOSTSW_OWN registers by using an introduced helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 38 +++++++++++++++------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 181e5c8c1855..1bdc0365e1ad 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1607,6 +1607,25 @@ intel_gpio_update_pad_mode(void __iomem *hostown, u32 mask, u32 value)
 	return curr;
 }
 
+static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
+				  void __iomem *base, unsigned int gpp, u32 saved)
+{
+	const struct intel_community *community = &pctrl->communities[c];
+	const struct intel_padgroup *padgrp = &community->gpps[gpp];
+	struct device *dev = pctrl->dev;
+	u32 requested, value;
+
+	if (padgrp->gpio_base < 0)
+		return;
+
+	requested = intel_gpio_is_requested(&pctrl->chip, padgrp->gpio_base, padgrp->size);
+	value = intel_gpio_update_pad_mode(base + gpp * 4, requested, saved);
+	if (!((value ^ saved) & requested))
+		return;
+
+	dev_warn(dev, "restored hostown %u/%u %#8x->%#8x\n", c, gpp, value, saved);
+}
+
 static void intel_restore_padcfg(struct intel_pinctrl *pctrl, unsigned int pin,
 				 unsigned int reg, u32 saved)
 {
@@ -1664,23 +1683,8 @@ int intel_pinctrl_resume_noirq(struct device *dev)
 		}
 
 		base = community->regs + community->hostown_offset;
-		for (gpp = 0; gpp < community->ngpps; gpp++) {
-			const struct intel_padgroup *padgrp = &community->gpps[gpp];
-			u32 requested = 0, value = 0;
-			u32 saved = communities[i].hostown[gpp];
-
-			if (padgrp->gpio_base < 0)
-				continue;
-
-			requested = intel_gpio_is_requested(&pctrl->chip,
-					padgrp->gpio_base, padgrp->size);
-			value = intel_gpio_update_pad_mode(base + gpp * 4,
-					requested, saved);
-			if ((value ^ saved) & requested) {
-				dev_warn(dev, "restore hostown %d/%u %#8x->%#8x\n",
-					i, gpp, value, saved);
-			}
-		}
+		for (gpp = 0; gpp < community->ngpps; gpp++)
+			intel_restore_hostown(pctrl, i, base, gpp, communities[i].hostown[gpp]);
 	}
 
 	return 0;
-- 
2.23.0

[PATCH v2 3/5] pinctrl: intel: Introduce intel_restore_intmask() helper

[Andy Shevchenko]

Refactor restoring GPI_IE registers by using an introduced helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 1bdc0365e1ad..da34750a420f 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1626,6 +1626,15 @@ static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
 	dev_warn(dev, "restored hostown %u/%u %#8x->%#8x\n", c, gpp, value, saved);
 }
 
+static void intel_restore_intmask(struct intel_pinctrl *pctrl, unsigned int c,
+				  void __iomem *base, unsigned int gpp, u32 saved)
+{
+	struct device *dev = pctrl->dev;
+
+	writel(saved, base + gpp * 4);
+	dev_dbg(dev, "restored mask %u/%u %#08x\n", c, gpp, readl(base + gpp * 4));
+}
+
 static void intel_restore_padcfg(struct intel_pinctrl *pctrl, unsigned int pin,
 				 unsigned int reg, u32 saved)
 {
@@ -1676,11 +1685,8 @@ int intel_pinctrl_resume_noirq(struct device *dev)
 		unsigned int gpp;
 
 		base = community->regs + community->ie_offset;
-		for (gpp = 0; gpp < community->ngpps; gpp++) {
-			writel(communities[i].intmask[gpp], base + gpp * 4);
-			dev_dbg(dev, "restored mask %d/%u %#08x\n", i, gpp,
-				readl(base + gpp * 4));
-		}
+		for (gpp = 0; gpp < community->ngpps; gpp++)
+			intel_restore_intmask(pctrl, i, base, gpp, communities[i].intmask[gpp]);
 
 		base = community->regs + community->hostown_offset;
 		for (gpp = 0; gpp < community->ngpps; gpp++)
-- 
2.23.0

[PATCH v2 4/5] pinctrl: intel: Drop level from warning to debug in intel_restore_hostown()

[Andy Shevchenko]

Since we didn't get any new reports from users about wrong settings
of pad ownership, there is no point to spam kernel log with it. Thus,
drop level from warning to debug.

Also, modify format to be in align with the rest restore helpers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index da34750a420f..54a5eb33c9fa 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1623,7 +1623,7 @@ static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
 	if (!((value ^ saved) & requested))
 		return;
 
-	dev_warn(dev, "restored hostown %u/%u %#8x->%#8x\n", c, gpp, value, saved);
+	dev_dbg(dev, "restored hostown %u/%u %#08x\n", c, gpp, readl(base + gpp * 4));
 }
 
 static void intel_restore_intmask(struct intel_pinctrl *pctrl, unsigned int c,
-- 
2.23.0

[PATCH v2 5/5] pinctrl: intel: Use helper to restore register values on ->resume()

[Andy Shevchenko]

We can restore only values that had been changed and do not spam kernel log
with unnecessary messages. Convert intel_gpio_update_pad_mode() to a helper
function that will be used across few callers.

Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 54a5eb33c9fa..b54b27228ad9 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1595,16 +1595,18 @@ intel_gpio_is_requested(struct gpio_chip *chip, int base, unsigned int size)
 	return requested;
 }
 
-static u32
-intel_gpio_update_pad_mode(void __iomem *hostown, u32 mask, u32 value)
+static bool intel_gpio_update_reg(void __iomem *reg, u32 mask, u32 value)
 {
 	u32 curr, updated;
 
-	curr = readl(hostown);
+	curr = readl(reg);
+
 	updated = (curr & ~mask) | (value & mask);
-	writel(updated, hostown);
+	if (curr == updated)
+		return false;
 
-	return curr;
+	writel(updated, reg);
+	return true;
 }
 
 static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
@@ -1613,14 +1615,13 @@ static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
 	const struct intel_community *community = &pctrl->communities[c];
 	const struct intel_padgroup *padgrp = &community->gpps[gpp];
 	struct device *dev = pctrl->dev;
-	u32 requested, value;
+	u32 requested;
 
 	if (padgrp->gpio_base < 0)
 		return;
 
 	requested = intel_gpio_is_requested(&pctrl->chip, padgrp->gpio_base, padgrp->size);
-	value = intel_gpio_update_pad_mode(base + gpp * 4, requested, saved);
-	if (!((value ^ saved) & requested))
+	if (!intel_gpio_update_reg(base + gpp * 4, requested, saved))
 		return;
 
 	dev_dbg(dev, "restored hostown %u/%u %#08x\n", c, gpp, readl(base + gpp * 4));
@@ -1631,7 +1632,9 @@ static void intel_restore_intmask(struct intel_pinctrl *pctrl, unsigned int c,
 {
 	struct device *dev = pctrl->dev;
 
-	writel(saved, base + gpp * 4);
+	if (!intel_gpio_update_reg(base + gpp * 4, ~0U, saved))
+		return;
+
 	dev_dbg(dev, "restored mask %u/%u %#08x\n", c, gpp, readl(base + gpp * 4));
 }
 
@@ -1642,17 +1645,14 @@ static void intel_restore_padcfg(struct intel_pinctrl *pctrl, unsigned int pin,
 	unsigned int n = reg / sizeof(u32);
 	struct device *dev = pctrl->dev;
 	void __iomem *padcfg;
-	u32 value;
 
 	padcfg = intel_get_padcfg(pctrl, pin, reg);
 	if (!padcfg)
 		return;
 
-	value = readl(padcfg) & ~mask;
-	if (value == saved)
+	if (!intel_gpio_update_reg(padcfg, ~mask, saved))
 		return;
 
-	writel(saved, padcfg);
 	dev_dbg(dev, "restored pin %u padcfg%u %#08x\n", pin, n, readl(padcfg));
 }
 
-- 
2.23.0

Re: [PATCH v2 0/5] pinctrl: intel: Refactor register restoring on resume

[Mika Westerberg]

On Tue, Oct 22, 2019 at 12:59:59PM +0300, Andy Shevchenko wrote:
> Refactor ->resume() path to be a bit smarter and less noisy.
> 
> In v2:
> - add a patch to reduce a noise in kernel log
> - drop warning to debug level for one message
> - constify local pointers (Mika)
> 
> Andy Shevchenko (5):
>   pinctrl: intel: Introduce intel_restore_padcfg() helper
>   pinctrl: intel: Introduce intel_restore_hostown() helper
>   pinctrl: intel: Introduce intel_restore_intmask() helper
>   pinctrl: intel: Drop level from warning to debug in
>     intel_restore_hostown()
>   pinctrl: intel: Use helper to restore register values on ->resume()

For the whole series,

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v2 0/5] pinctrl: intel: Refactor register restoring on resume

[Andy Shevchenko]

On Wed, Oct 23, 2019 at 10:30:33AM +0300, Mika Westerberg wrote:
> On Tue, Oct 22, 2019 at 12:59:59PM +0300, Andy Shevchenko wrote:
> > Refactor ->resume() path to be a bit smarter and less noisy.
> > 
> > In v2:
> > - add a patch to reduce a noise in kernel log
> > - drop warning to debug level for one message
> > - constify local pointers (Mika)
> > 
> > Andy Shevchenko (5):
> >   pinctrl: intel: Introduce intel_restore_padcfg() helper
> >   pinctrl: intel: Introduce intel_restore_hostown() helper
> >   pinctrl: intel: Introduce intel_restore_intmask() helper
> >   pinctrl: intel: Drop level from warning to debug in
> >     intel_restore_hostown()
> >   pinctrl: intel: Use helper to restore register values on ->resume()
> 
> For the whole series,
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko