mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ray Jui <ray.jui@broadcom.com>
To: Julia Cartwright <julia@ni.com>, Ray Jui <rjui@broadcom.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com
Cc: linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH 07/19] gpio: bcm-kona: make use of raw_spinlock variants
Date: Fri, 10 Mar 2017 09:28:08 -0800	[thread overview]
Message-ID: <78b657c5-0f6a-6481-a0cf-e241c0632c7f@broadcom.com> (raw)
In-Reply-To: <77d635877e765a6b4b0935debfe5836e554572c0.1489015238.git.julia@ni.com>

Hi Julia/Linus,

On 3/9/2017 8:21 AM, Julia Cartwright wrote:
> The bcm-kona gpio driver currently implements an irq_chip for handling
> interrupts; due to how irq_chip handling is done, it's necessary for the
> irq_chip methods to be invoked from hardirq context, even on a a
> real-time kernel.  Because the spinlock_t type becomes a "sleeping"
> spinlock w/ RT kernels, it is not suitable to be used with irq_chips.
> 
> A quick audit of the operations under the lock reveal that they do only
> minimal, bounded work, and are therefore safe to do under a raw spinlock.

This is new to me. But it seems like, for the vast majority cases, user
can still continue to use spin_lock as it is without needing to worry
about the underlying difference between standard or RT kernels. But in
certain cases, e.g., irq_chips, extra care needs to be done, i.e.,
switching to use raw spin lock to make sure that it is not blocking in
the case of RT.

Is such API use change well accepted by the open source community already?

Thanks,

Ray

> 
> Signed-off-by: Julia Cartwright <julia@ni.com>
> ---
>  drivers/gpio/gpio-bcm-kona.c | 48 ++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> index 41d0ac142580..dfcf56ee3c61 100644
> --- a/drivers/gpio/gpio-bcm-kona.c
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -67,7 +67,7 @@
>  struct bcm_kona_gpio {
>  	void __iomem *reg_base;
>  	int num_bank;
> -	spinlock_t lock;
> +	raw_spinlock_t lock;
>  	struct gpio_chip gpio_chip;
>  	struct irq_domain *irq_domain;
>  	struct bcm_kona_gpio_bank *banks;
> @@ -95,13 +95,13 @@ static void bcm_kona_gpio_lock_gpio(struct bcm_kona_gpio *kona_gpio,
>  	unsigned long flags;
>  	int bank_id = GPIO_BANK(gpio);
>  
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(kona_gpio->reg_base + GPIO_PWD_STATUS(bank_id));
>  	val |= BIT(gpio);
>  	bcm_kona_gpio_write_lock_regs(kona_gpio->reg_base, bank_id, val);
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
> @@ -111,13 +111,13 @@ static void bcm_kona_gpio_unlock_gpio(struct bcm_kona_gpio *kona_gpio,
>  	unsigned long flags;
>  	int bank_id = GPIO_BANK(gpio);
>  
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(kona_gpio->reg_base + GPIO_PWD_STATUS(bank_id));
>  	val &= ~BIT(gpio);
>  	bcm_kona_gpio_write_lock_regs(kona_gpio->reg_base, bank_id, val);
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static int bcm_kona_gpio_get_dir(struct gpio_chip *chip, unsigned gpio)
> @@ -141,7 +141,7 @@ static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
>  
>  	kona_gpio = gpiochip_get_data(chip);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	/* this function only applies to output pin */
>  	if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
> @@ -154,7 +154,7 @@ static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, int value)
>  	writel(val, reg_base + reg_offset);
>  
>  out:
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
> @@ -168,7 +168,7 @@ static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
>  
>  	kona_gpio = gpiochip_get_data(chip);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	if (bcm_kona_gpio_get_dir(chip, gpio) == GPIOF_DIR_IN)
>  		reg_offset = GPIO_IN_STATUS(bank_id);
> @@ -178,7 +178,7 @@ static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
>  	/* read the GPIO bank status */
>  	val = readl(reg_base + reg_offset);
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  
>  	/* return the specified bit status */
>  	return !!(val & BIT(bit));
> @@ -208,14 +208,14 @@ static int bcm_kona_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
>  
>  	kona_gpio = gpiochip_get_data(chip);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_CONTROL(gpio));
>  	val &= ~GPIO_GPCTR0_IOTR_MASK;
>  	val |= GPIO_GPCTR0_IOTR_CMD_INPUT;
>  	writel(val, reg_base + GPIO_CONTROL(gpio));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  
>  	return 0;
>  }
> @@ -232,7 +232,7 @@ static int bcm_kona_gpio_direction_output(struct gpio_chip *chip,
>  
>  	kona_gpio = gpiochip_get_data(chip);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_CONTROL(gpio));
>  	val &= ~GPIO_GPCTR0_IOTR_MASK;
> @@ -244,7 +244,7 @@ static int bcm_kona_gpio_direction_output(struct gpio_chip *chip,
>  	val |= BIT(bit);
>  	writel(val, reg_base + reg_offset);
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  
>  	return 0;
>  }
> @@ -288,7 +288,7 @@ static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio,
>  	}
>  
>  	/* spin lock for read-modify-write of the GPIO register */
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_CONTROL(gpio));
>  	val &= ~GPIO_GPCTR0_DBR_MASK;
> @@ -303,7 +303,7 @@ static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio,
>  
>  	writel(val, reg_base + GPIO_CONTROL(gpio));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  
>  	return 0;
>  }
> @@ -347,13 +347,13 @@ static void bcm_kona_gpio_irq_ack(struct irq_data *d)
>  
>  	kona_gpio = irq_data_get_irq_chip_data(d);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_INT_STATUS(bank_id));
>  	val |= BIT(bit);
>  	writel(val, reg_base + GPIO_INT_STATUS(bank_id));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static void bcm_kona_gpio_irq_mask(struct irq_data *d)
> @@ -368,13 +368,13 @@ static void bcm_kona_gpio_irq_mask(struct irq_data *d)
>  
>  	kona_gpio = irq_data_get_irq_chip_data(d);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_INT_MASK(bank_id));
>  	val |= BIT(bit);
>  	writel(val, reg_base + GPIO_INT_MASK(bank_id));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static void bcm_kona_gpio_irq_unmask(struct irq_data *d)
> @@ -389,13 +389,13 @@ static void bcm_kona_gpio_irq_unmask(struct irq_data *d)
>  
>  	kona_gpio = irq_data_get_irq_chip_data(d);
>  	reg_base = kona_gpio->reg_base;
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_INT_MSKCLR(bank_id));
>  	val |= BIT(bit);
>  	writel(val, reg_base + GPIO_INT_MSKCLR(bank_id));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  }
>  
>  static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> @@ -431,14 +431,14 @@ static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>  		return -EINVAL;
>  	}
>  
> -	spin_lock_irqsave(&kona_gpio->lock, flags);
> +	raw_spin_lock_irqsave(&kona_gpio->lock, flags);
>  
>  	val = readl(reg_base + GPIO_CONTROL(gpio));
>  	val &= ~GPIO_GPCTR0_ITR_MASK;
>  	val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT;
>  	writel(val, reg_base + GPIO_CONTROL(gpio));
>  
> -	spin_unlock_irqrestore(&kona_gpio->lock, flags);
> +	raw_spin_unlock_irqrestore(&kona_gpio->lock, flags);
>  
>  	return 0;
>  }
> @@ -655,7 +655,7 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev)
>  						 bank);
>  	}
>  
> -	spin_lock_init(&kona_gpio->lock);
> +	raw_spin_lock_init(&kona_gpio->lock);
>  
>  	return 0;
>  
> 

  reply	other threads:[~2017-03-10 17:28 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-09 16:21 [PATCH 00/19] fixup usage of non-raw spinlocks in irqchips Julia Cartwright
2017-03-09 16:21 ` [PATCH 01/19] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations Julia Cartwright
2017-03-09 20:15   ` Julia Lawall
2017-03-09 21:37     ` Julia Cartwright
2017-03-09 16:21 ` [PATCH 02/19] gpio: altera: make use of raw_spinlock variants Julia Cartwright
2017-03-15  9:53   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 03/19] alpha: marvel: " Julia Cartwright
2017-03-12  2:58   ` kbuild test robot
2017-03-09 16:21 ` [PATCH 04/19] powerpc: mpc52xx_gpt: " Julia Cartwright
2017-03-09 16:21 ` [PATCH 05/19] gpio: 104-dio-48e: " Julia Cartwright
2017-03-09 18:35   ` William Breathitt Gray
2017-03-15  9:54   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 06/19] gpio: ath79: " Julia Cartwright
2017-03-13 19:51   ` Alban
2017-03-15  9:59   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 07/19] gpio: bcm-kona: " Julia Cartwright
2017-03-10 17:28   ` Ray Jui [this message]
2017-03-10 19:35     ` Julia Cartwright
2017-03-15  9:57     ` Linus Walleij
2017-03-15  9:56   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 08/19] gpio: etraxfs: " Julia Cartwright
2017-03-15 10:00   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 09/19] gpio: pl061: " Julia Cartwright
2017-03-15 10:01   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 10/19] gpio: ws16c48: " Julia Cartwright
2017-03-09 18:36   ` William Breathitt Gray
2017-03-15 10:02   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 11/19] gpio: zx: " Julia Cartwright
2017-03-15 10:03   ` Linus Walleij
2017-03-09 16:21 ` [PATCH 12/19] i2c: mux: pca954x: " Julia Cartwright
2017-03-16 19:38   ` Peter Rosin
2017-03-09 16:22 ` [PATCH 13/19] mfd: asic3: " Julia Cartwright
2017-03-09 16:22 ` [PATCH 14/19] mfd: t7l66xb: " Julia Cartwright
2017-03-15 11:17   ` Lee Jones
2017-03-15 19:07     ` Julia Cartwright
2017-03-16  9:22       ` Lee Jones
2017-03-09 16:22 ` [PATCH 15/19] mfd: tc6393xb: " Julia Cartwright
2017-03-09 16:22 ` [PATCH 16/19] pinctrl: bcm: " Julia Cartwright
2017-03-15 10:11   ` Linus Walleij
2017-03-09 16:22 ` [PATCH 17/19] pinctrl: amd: " Julia Cartwright
2017-03-15 10:12   ` Linus Walleij
2017-03-09 16:22 ` [PATCH 18/19] pinctrl: sirf: atlas7: " Julia Cartwright
2017-03-15 10:13   ` Linus Walleij
2017-03-09 16:22 ` [PATCH 19/19] pinctrl: sunxi: " Julia Cartwright
2017-03-15 10:14   ` Linus Walleij
2017-03-15  9:51 ` [PATCH 00/19] fixup usage of non-raw spinlocks in irqchips Linus Walleij

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=78b657c5-0f6a-6481-a0cf-e241c0632c7f@broadcom.com \
    --to=ray.jui@broadcom.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=f.fainelli@gmail.com \
    --cc=gnurou@gmail.com \
    --cc=julia@ni.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjui@broadcom.com \
    --cc=sbranden@broadcom.com \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome