[v1,3/3] pinctrl: intel: Introduce intel_restore_intmask() helper
diff mbox series

Message ID 20191014084348.42489-3-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • [v1,1/3] pinctrl: intel: Introduce intel_restore_padcfg() helper
Related show

Commit Message

Andy Shevchenko Oct. 14, 2019, 8:43 a.m. UTC
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(-)

Comments

Mika Westerberg Oct. 21, 2019, 12:27 p.m. UTC | #1
On Mon, Oct 14, 2019 at 11:43:48AM +0300, Andy Shevchenko wrote:
> 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 e59ac31921e7..b9df243e19cf 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1607,6 +1607,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;

const?

Also should we do the same here as we do with others and check first
whether we need to update the mask at all?
Andy Shevchenko Oct. 21, 2019, 1:19 p.m. UTC | #2
On Mon, Oct 21, 2019 at 03:27:20PM +0300, Mika Westerberg wrote:
> On Mon, Oct 14, 2019 at 11:43:48AM +0300, Andy Shevchenko wrote:
> > Refactor restoring GPI_IE registers by using an introduced helper.

> > +	struct device *dev = pctrl->dev;
> 
> const?

It's only about dozen occurrences in 8 drivers altogether in the kernel
to have const with struct device.

So, I consider this pattern is unusual.

> Also should we do the same here as we do with others and check first
> whether we need to update the mask at all?

I will look at it.
I think we may do it as a separate change (this doesn't change any
functionality).

Patch
diff mbox series

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index e59ac31921e7..b9df243e19cf 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1607,6 +1607,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 value)
 {
@@ -1657,11 +1666,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++)