diff mbox series

[v1,1/2] pinctrl: cherryview: Avoid duplicated I/O

Message ID 20231006094033.3082468-1-andriy.shevchenko@linux.intel.com
State Accepted
Commit d3386552155c6e9128e49fff86d7acfe4b13f949
Headers show
Series [v1,1/2] pinctrl: cherryview: Avoid duplicated I/O | expand

Commit Message

Andy Shevchenko Oct. 6, 2023, 9:40 a.m. UTC
In some cases we already read the value from the register followed
by a reading of it again for other purposes, but the both reads
are under the lock and bits we are insterested in are not going
to change (they are not volatile from HW perspective). Hence, no
need to read the same registeer twice.

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

Comments

Mika Westerberg Oct. 6, 2023, 10:26 a.m. UTC | #1
On Fri, Oct 06, 2023 at 12:40:33PM +0300, Andy Shevchenko wrote:
> @@ -1119,28 +1099,26 @@ static struct pinctrl_desc chv_pinctrl_desc = {
>  static int chv_gpio_get(struct gpio_chip *chip, unsigned int offset)
>  {
>  	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
> -	unsigned long flags;
>  	u32 ctrl0, cfg;
>  
> -	raw_spin_lock_irqsave(&chv_lock, flags);
> -	ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
> -	raw_spin_unlock_irqrestore(&chv_lock, flags);
> +	scoped_guard(raw_spinlock_irqsave, &chv_lock)
> +		ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
>  
>  	cfg = ctrl0 & CHV_PADCTRL0_GPIOCFG_MASK;
>  	cfg >>= CHV_PADCTRL0_GPIOCFG_SHIFT;
>  
>  	if (cfg == CHV_PADCTRL0_GPIOCFG_GPO)
>  		return !!(ctrl0 & CHV_PADCTRL0_GPIOTXSTATE);
> +

Unrelated whitespace change.

Otherwise looks good,

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Andy Shevchenko Oct. 6, 2023, 10:32 a.m. UTC | #2
On Fri, Oct 06, 2023 at 01:26:41PM +0300, Mika Westerberg wrote:
> On Fri, Oct 06, 2023 at 12:40:33PM +0300, Andy Shevchenko wrote:

...

> >  	if (cfg == CHV_PADCTRL0_GPIOCFG_GPO)
> >  		return !!(ctrl0 & CHV_PADCTRL0_GPIOTXSTATE);
> > +
> 
> Unrelated whitespace change.

Will drop it.

> Otherwise looks good,
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!
Andy Shevchenko Oct. 6, 2023, 10:38 a.m. UTC | #3
On Fri, Oct 06, 2023 at 01:24:12PM +0300, Mika Westerberg wrote:
> On Fri, Oct 06, 2023 at 12:40:32PM +0300, Andy Shevchenko wrote:

...

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

Pushed to my review and testing queue, thanks!
diff mbox series

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index f047f7bf4afb..9b9f18f50c1d 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -612,9 +612,14 @@  static void chv_writel(struct intel_pinctrl *pctrl, unsigned int pin, unsigned i
 }
 
 /* When Pad Cfg is locked, driver can only change GPIOTXState or GPIORXState */
+static bool chv_pad_is_locked(u32 ctrl1)
+{
+	return ctrl1 & CHV_PADCTRL1_CFGLOCK;
+}
+
 static bool chv_pad_locked(struct intel_pinctrl *pctrl, unsigned int offset)
 {
-	return chv_readl(pctrl, offset, CHV_PADCTRL1) & CHV_PADCTRL1_CFGLOCK;
+	return chv_pad_is_locked(chv_readl(pctrl, offset, CHV_PADCTRL1));
 }
 
 static void chv_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
@@ -623,13 +628,11 @@  static void chv_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	unsigned long flags;
 	u32 ctrl0, ctrl1;
-	bool locked;
 
 	raw_spin_lock_irqsave(&chv_lock, flags);
 
 	ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
 	ctrl1 = chv_readl(pctrl, offset, CHV_PADCTRL1);
-	locked = chv_pad_locked(pctrl, offset);
 
 	raw_spin_unlock_irqrestore(&chv_lock, flags);
 
@@ -646,7 +649,7 @@  static void chv_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 
 	seq_printf(s, "0x%08x 0x%08x", ctrl0, ctrl1);
 
-	if (locked)
+	if (chv_pad_is_locked(ctrl1))
 		seq_puts(s, " [LOCKED]");
 }