All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andy@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Raag Jadav <raag.jadav@intel.com>
Subject: [PATCH v1 2/2] pinctrl: cherryview: Simplify code with cleanup helpers
Date: Fri,  6 Oct 2023 12:40:33 +0300	[thread overview]
Message-ID: <20231006094033.3082468-2-andriy.shevchenko@linux.intel.com> (raw)
In-Reply-To: <20231006094033.3082468-1-andriy.shevchenko@linux.intel.com>

Use macros defined in linux/cleanup.h to automate resource lifetime
control in the driver.

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

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 9b9f18f50c1d..2ffeccc83ccd 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -12,6 +12,7 @@
 
 #include <linux/acpi.h>
 #include <linux/array_size.h>
+#include <linux/cleanup.h>
 #include <linux/dmi.h>
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
@@ -626,15 +627,12 @@ static void chv_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 			     unsigned int offset)
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
-	unsigned long flags;
 	u32 ctrl0, ctrl1;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
-
-	ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
-	ctrl1 = chv_readl(pctrl, offset, CHV_PADCTRL1);
-
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
+	scoped_guard(raw_spinlock_irqsave, &chv_lock) {
+		ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
+		ctrl1 = chv_readl(pctrl, offset, CHV_PADCTRL1);
+	}
 
 	if (ctrl0 & CHV_PADCTRL0_GPIOEN) {
 		seq_puts(s, "GPIO ");
@@ -666,17 +664,15 @@ static int chv_pinmux_set_mux(struct pinctrl_dev *pctldev,
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	struct device *dev = pctrl->dev;
 	const struct intel_pingroup *grp;
-	unsigned long flags;
 	int i;
 
 	grp = &pctrl->soc->groups[group];
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	/* Check first that the pad is not locked */
 	for (i = 0; i < grp->grp.npins; i++) {
 		if (chv_pad_locked(pctrl, grp->grp.pins[i])) {
-			raw_spin_unlock_irqrestore(&chv_lock, flags);
 			dev_warn(dev, "unable to set mode for locked pin %u\n", grp->grp.pins[i]);
 			return -EBUSY;
 		}
@@ -716,8 +712,6 @@ static int chv_pinmux_set_mux(struct pinctrl_dev *pctldev,
 			invert_oe ? "" : "not ");
 	}
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
@@ -748,16 +742,14 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev,
 				   unsigned int offset)
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
-	unsigned long flags;
 	u32 value;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	if (chv_pad_locked(pctrl, offset)) {
 		value = chv_readl(pctrl, offset, CHV_PADCTRL0);
 		if (!(value & CHV_PADCTRL0_GPIOEN)) {
 			/* Locked so cannot enable */
-			raw_spin_unlock_irqrestore(&chv_lock, flags);
 			return -EBUSY;
 		}
 	} else {
@@ -792,8 +784,6 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev,
 		chv_writel(pctrl, offset, CHV_PADCTRL0, value);
 	}
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
@@ -802,14 +792,13 @@ static void chv_gpio_disable_free(struct pinctrl_dev *pctldev,
 				  unsigned int offset)
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
-	if (!chv_pad_locked(pctrl, offset))
-		chv_gpio_clear_triggering(pctrl, offset);
+	if (chv_pad_locked(pctrl, offset))
+		return;
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
+	chv_gpio_clear_triggering(pctrl, offset);
 }
 
 static int chv_gpio_set_direction(struct pinctrl_dev *pctldev,
@@ -817,10 +806,9 @@ static int chv_gpio_set_direction(struct pinctrl_dev *pctldev,
 				  unsigned int offset, bool input)
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
-	unsigned long flags;
 	u32 ctrl0;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0) & ~CHV_PADCTRL0_GPIOCFG_MASK;
 	if (input)
@@ -829,8 +817,6 @@ static int chv_gpio_set_direction(struct pinctrl_dev *pctldev,
 		ctrl0 |= CHV_PADCTRL0_GPIOCFG_GPO << CHV_PADCTRL0_GPIOCFG_SHIFT;
 	chv_writel(pctrl, offset, CHV_PADCTRL0, ctrl0);
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
@@ -849,15 +835,14 @@ static int chv_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	enum pin_config_param param = pinconf_to_config_param(*config);
-	unsigned long flags;
 	u32 ctrl0, ctrl1;
 	u16 arg = 0;
 	u32 term;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
-	ctrl0 = chv_readl(pctrl, pin, CHV_PADCTRL0);
-	ctrl1 = chv_readl(pctrl, pin, CHV_PADCTRL1);
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
+	scoped_guard(raw_spinlock_irqsave, &chv_lock) {
+		ctrl0 = chv_readl(pctrl, pin, CHV_PADCTRL0);
+		ctrl1 = chv_readl(pctrl, pin, CHV_PADCTRL1);
+	}
 
 	term = (ctrl0 & CHV_PADCTRL0_TERM_MASK) >> CHV_PADCTRL0_TERM_SHIFT;
 
@@ -932,10 +917,10 @@ static int chv_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
 static int chv_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,
 			       enum pin_config_param param, u32 arg)
 {
-	unsigned long flags;
 	u32 ctrl0, pull;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
+
 	ctrl0 = chv_readl(pctrl, pin, CHV_PADCTRL0);
 
 	switch (param) {
@@ -958,7 +943,6 @@ static int chv_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,
 			pull = CHV_PADCTRL0_TERM_20K << CHV_PADCTRL0_TERM_SHIFT;
 			break;
 		default:
-			raw_spin_unlock_irqrestore(&chv_lock, flags);
 			return -EINVAL;
 		}
 
@@ -976,7 +960,6 @@ static int chv_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,
 			pull = CHV_PADCTRL0_TERM_20K << CHV_PADCTRL0_TERM_SHIFT;
 			break;
 		default:
-			raw_spin_unlock_irqrestore(&chv_lock, flags);
 			return -EINVAL;
 		}
 
@@ -984,12 +967,10 @@ static int chv_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,
 		break;
 
 	default:
-		raw_spin_unlock_irqrestore(&chv_lock, flags);
 		return -EINVAL;
 	}
 
 	chv_writel(pctrl, pin, CHV_PADCTRL0, ctrl0);
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
 
 	return 0;
 }
@@ -997,10 +978,10 @@ static int chv_config_set_pull(struct intel_pinctrl *pctrl, unsigned int pin,
 static int chv_config_set_oden(struct intel_pinctrl *pctrl, unsigned int pin,
 			       bool enable)
 {
-	unsigned long flags;
 	u32 ctrl1;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
+
 	ctrl1 = chv_readl(pctrl, pin, CHV_PADCTRL1);
 
 	if (enable)
@@ -1009,7 +990,6 @@ static int chv_config_set_oden(struct intel_pinctrl *pctrl, unsigned int pin,
 		ctrl1 &= ~CHV_PADCTRL1_ODEN;
 
 	chv_writel(pctrl, pin, CHV_PADCTRL1, ctrl1);
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
 
 	return 0;
 }
@@ -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);
+
 	return !!(ctrl0 & CHV_PADCTRL0_GPIORXSTATE);
 }
 
 static void chv_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
 {
 	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
-	unsigned long flags;
 	u32 ctrl0;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	ctrl0 = chv_readl(pctrl, offset, CHV_PADCTRL0);
 
@@ -1150,19 +1128,15 @@ static void chv_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
 		ctrl0 &= ~CHV_PADCTRL0_GPIOTXSTATE;
 
 	chv_writel(pctrl, offset, CHV_PADCTRL0, ctrl0);
-
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
 }
 
 static int chv_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
 	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
 	u32 ctrl0, direction;
-	unsigned long flags;
 
-	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);
 
 	direction = ctrl0 & CHV_PADCTRL0_GPIOCFG_MASK;
 	direction >>= CHV_PADCTRL0_GPIOCFG_SHIFT;
@@ -1203,23 +1177,20 @@ static void chv_gpio_irq_ack(struct irq_data *d)
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	u32 intr_line;
 
-	raw_spin_lock(&chv_lock);
+	guard(raw_spinlock)(&chv_lock);
 
 	intr_line = chv_readl(pctrl, hwirq, CHV_PADCTRL0);
 	intr_line &= CHV_PADCTRL0_INTSEL_MASK;
 	intr_line >>= CHV_PADCTRL0_INTSEL_SHIFT;
 	chv_pctrl_writel(pctrl, CHV_INTSTAT, BIT(intr_line));
-
-	raw_spin_unlock(&chv_lock);
 }
 
 static void chv_gpio_irq_mask_unmask(struct gpio_chip *gc, irq_hw_number_t hwirq, bool mask)
 {
 	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
 	u32 value, intr_line;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	intr_line = chv_readl(pctrl, hwirq, CHV_PADCTRL0);
 	intr_line &= CHV_PADCTRL0_INTSEL_MASK;
@@ -1231,8 +1202,6 @@ static void chv_gpio_irq_mask_unmask(struct gpio_chip *gc, irq_hw_number_t hwirq
 	else
 		value |= BIT(intr_line);
 	chv_pctrl_writel(pctrl, CHV_INTMASK, value);
-
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
 }
 
 static void chv_gpio_irq_mask(struct irq_data *d)
@@ -1257,7 +1226,15 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d)
 {
 	/*
 	 * Check if the interrupt has been requested with 0 as triggering
-	 * type. In that case it is assumed that the current values
+	 * type. If not, bail out, ...
+	 */
+	if (irqd_get_trigger_type(d) != IRQ_TYPE_NONE) {
+		chv_gpio_irq_unmask(d);
+		return 0;
+	}
+
+	/*
+	 * ...otherwise it is assumed that the current values
 	 * programmed to the hardware are used (e.g BIOS configured
 	 * defaults).
 	 *
@@ -1265,17 +1242,15 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d)
 	 * read back the values from hardware now, set correct flow handler
 	 * and update mappings before the interrupt is being used.
 	 */
-	if (irqd_get_trigger_type(d) == IRQ_TYPE_NONE) {
+	scoped_guard(raw_spinlock_irqsave, &chv_lock) {
 		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 		struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
 		struct device *dev = pctrl->dev;
 		struct intel_community_context *cctx = &pctrl->context.communities[0];
 		irq_hw_number_t hwirq = irqd_to_hwirq(d);
 		irq_flow_handler_t handler;
-		unsigned long flags;
 		u32 intsel, value;
 
-		raw_spin_lock_irqsave(&chv_lock, flags);
 		intsel = chv_readl(pctrl, hwirq, CHV_PADCTRL0);
 		intsel &= CHV_PADCTRL0_INTSEL_MASK;
 		intsel >>= CHV_PADCTRL0_INTSEL_SHIFT;
@@ -1292,7 +1267,6 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d)
 				intsel, hwirq);
 			cctx->intr_lines[intsel] = hwirq;
 		}
-		raw_spin_unlock_irqrestore(&chv_lock, flags);
 	}
 
 	chv_gpio_irq_unmask(d);
@@ -1357,17 +1331,14 @@ static int chv_gpio_irq_type(struct irq_data *d, unsigned int type)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
-	unsigned long flags;
 	u32 value;
 	int ret;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	ret = chv_gpio_set_intr_line(pctrl, hwirq);
-	if (ret) {
-		raw_spin_unlock_irqrestore(&chv_lock, flags);
+	if (ret)
 		return ret;
-	}
 
 	/*
 	 * Pins which can be used as shared interrupt are configured in
@@ -1408,8 +1379,6 @@ static int chv_gpio_irq_type(struct irq_data *d, unsigned int type)
 	else if (type & IRQ_TYPE_LEVEL_MASK)
 		irq_set_handler_locked(d, handle_level_irq);
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
@@ -1433,14 +1402,12 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
 	struct intel_community_context *cctx = &pctrl->context.communities[0];
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	unsigned long pending;
-	unsigned long flags;
 	u32 intr_line;
 
 	chained_irq_enter(chip, desc);
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
-	pending = chv_pctrl_readl(pctrl, CHV_INTSTAT);
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
+	scoped_guard(raw_spinlock_irqsave, &chv_lock)
+		pending = chv_pctrl_readl(pctrl, CHV_INTSTAT);
 
 	for_each_set_bit(intr_line, &pending, community->nirqs) {
 		unsigned int offset;
@@ -1629,21 +1596,17 @@ static acpi_status chv_pinctrl_mmio_access_handler(u32 function,
 	void *handler_context, void *region_context)
 {
 	struct intel_pinctrl *pctrl = region_context;
-	unsigned long flags;
-	acpi_status ret = AE_OK;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	if (function == ACPI_WRITE)
 		chv_pctrl_writel(pctrl, address, *value);
 	else if (function == ACPI_READ)
 		*value = chv_pctrl_readl(pctrl, address);
 	else
-		ret = AE_BAD_PARAMETER;
+		return AE_BAD_PARAMETER;
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
-	return ret;
+	return AE_OK;
 }
 
 static int chv_pinctrl_probe(struct platform_device *pdev)
@@ -1751,10 +1714,9 @@ static int chv_pinctrl_suspend_noirq(struct device *dev)
 {
 	struct intel_pinctrl *pctrl = dev_get_drvdata(dev);
 	struct intel_community_context *cctx = &pctrl->context.communities[0];
-	unsigned long flags;
 	int i;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	cctx->saved_intmask = chv_pctrl_readl(pctrl, CHV_INTMASK);
 
@@ -1772,8 +1734,6 @@ static int chv_pinctrl_suspend_noirq(struct device *dev)
 		ctx->padctrl1 = chv_readl(pctrl, desc->number, CHV_PADCTRL1);
 	}
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
@@ -1781,10 +1741,9 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
 {
 	struct intel_pinctrl *pctrl = dev_get_drvdata(dev);
 	struct intel_community_context *cctx = &pctrl->context.communities[0];
-	unsigned long flags;
 	int i;
 
-	raw_spin_lock_irqsave(&chv_lock, flags);
+	guard(raw_spinlock_irqsave)(&chv_lock);
 
 	/*
 	 * Mask all interrupts before restoring per-pin configuration
@@ -1826,8 +1785,6 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
 	chv_pctrl_writel(pctrl, CHV_INTSTAT, 0xffff);
 	chv_pctrl_writel(pctrl, CHV_INTMASK, cctx->saved_intmask);
 
-	raw_spin_unlock_irqrestore(&chv_lock, flags);
-
 	return 0;
 }
 
-- 
2.40.0.1.gaa8946217a0b


  reply	other threads:[~2023-10-06  9:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06  9:40 [PATCH v1 1/2] pinctrl: cherryview: Avoid duplicated I/O Andy Shevchenko
2023-10-06  9:40 ` Andy Shevchenko [this message]
2023-10-06 10:26   ` [PATCH v1 2/2] pinctrl: cherryview: Simplify code with cleanup helpers Mika Westerberg
2023-10-06 10:32     ` Andy Shevchenko
2023-10-06 10:37     ` Andy Shevchenko
2023-10-06 10:24 ` [PATCH v1 1/2] pinctrl: cherryview: Avoid duplicated I/O Mika Westerberg
2023-10-06 10:26   ` Andy Shevchenko
2023-10-06 10:38   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231006094033.3082468-2-andriy.shevchenko@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=raag.jadav@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.