2021-12-04 17:11:02

by Winiarska, Iwona

[permalink] [raw]
Subject: [PATCH 1/2] gpio: aspeed: Convert aspeed_gpio.lock to raw_spinlock

The gpio-aspeed driver implements an irq_chip which need to be invoked
from hardirq context. Since spin_lock() can sleep with PREEMPT_RT, it is
no longer legal to invoke it while interrupts are disabled.
This also causes lockdep to complain about:
[ 0.649797] [ BUG: Invalid wait context ]
because aspeed_gpio.lock (spin_lock_t) is taken under irq_desc.lock
(raw_spinlock_t).
Let's use of raw_spinlock_t instead of spinlock_t.

Signed-off-by: Iwona Winiarska <[email protected]>
---
drivers/gpio/gpio-aspeed.c | 52 +++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 3c8f20c57695..318a7d95a1a8 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -53,7 +53,7 @@ struct aspeed_gpio_config {
struct aspeed_gpio {
struct gpio_chip chip;
struct irq_chip irqc;
- spinlock_t lock;
+ raw_spinlock_t lock;
void __iomem *base;
int irq;
const struct aspeed_gpio_config *config;
@@ -413,14 +413,14 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
unsigned long flags;
bool copro;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
copro = aspeed_gpio_copro_request(gpio, offset);

__aspeed_gpio_set(gc, offset, val);

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
@@ -435,7 +435,7 @@ static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
if (!have_input(gpio, offset))
return -ENOTSUPP;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

reg = ioread32(addr);
reg &= ~GPIO_BIT(offset);
@@ -445,7 +445,7 @@ static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
if (copro)
aspeed_gpio_copro_release(gpio, offset);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return 0;
}
@@ -463,7 +463,7 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
if (!have_output(gpio, offset))
return -ENOTSUPP;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

reg = ioread32(addr);
reg |= GPIO_BIT(offset);
@@ -474,7 +474,7 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return 0;
}
@@ -492,11 +492,11 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
if (!have_output(gpio, offset))
return GPIO_LINE_DIRECTION_IN;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
}
@@ -539,14 +539,14 @@ static void aspeed_gpio_irq_ack(struct irq_data *d)

status_addr = bank_reg(gpio, bank, reg_irq_status);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
copro = aspeed_gpio_copro_request(gpio, offset);

iowrite32(bit, status_addr);

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
@@ -565,7 +565,7 @@ static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)

addr = bank_reg(gpio, bank, reg_irq_enable);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
copro = aspeed_gpio_copro_request(gpio, offset);

reg = ioread32(addr);
@@ -577,7 +577,7 @@ static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static void aspeed_gpio_irq_mask(struct irq_data *d)
@@ -629,7 +629,7 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
return -EINVAL;
}

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
copro = aspeed_gpio_copro_request(gpio, offset);

addr = bank_reg(gpio, bank, reg_irq_type0);
@@ -649,7 +649,7 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

irq_set_handler_locked(d, handler);

@@ -716,7 +716,7 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,

treg = bank_reg(gpio, to_bank(offset), reg_tolerance);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
copro = aspeed_gpio_copro_request(gpio, offset);

val = readl(treg);
@@ -730,7 +730,7 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,

if (copro)
aspeed_gpio_copro_release(gpio, offset);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return 0;
}
@@ -856,7 +856,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
return rc;
}

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

if (timer_allocation_registered(gpio, offset)) {
rc = unregister_allocated_timer(gpio, offset);
@@ -916,7 +916,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
configure_timer(gpio, offset, i);

out:
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return rc;
}
@@ -927,13 +927,13 @@ static int disable_debounce(struct gpio_chip *chip, unsigned int offset)
unsigned long flags;
int rc;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

rc = unregister_allocated_timer(gpio, offset);
if (!rc)
configure_timer(gpio, offset, 0);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return rc;
}
@@ -1015,7 +1015,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
return -EINVAL;
bindex = offset >> 3;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

/* Sanity check, this shouldn't happen */
if (gpio->cf_copro_bankmap[bindex] == 0xff) {
@@ -1036,7 +1036,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
if (bit)
*bit = GPIO_OFFSET(offset);
bail:
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
return rc;
}
EXPORT_SYMBOL_GPL(aspeed_gpio_copro_grab_gpio);
@@ -1060,7 +1060,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
return -EINVAL;
bindex = offset >> 3;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

/* Sanity check, this shouldn't happen */
if (gpio->cf_copro_bankmap[bindex] == 0) {
@@ -1074,7 +1074,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
aspeed_gpio_change_cmd_source(gpio, bank, bindex,
GPIO_CMDSRC_ARM);
bail:
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
return rc;
}
EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
@@ -1148,7 +1148,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
if (IS_ERR(gpio->base))
return PTR_ERR(gpio->base);

- spin_lock_init(&gpio->lock);
+ raw_spin_lock_init(&gpio->lock);

gpio_id = of_match_node(aspeed_gpio_of_table, pdev->dev.of_node);
if (!gpio_id)
--
2.31.1



2021-12-04 17:11:12

by Winiarska, Iwona

[permalink] [raw]
Subject: [PATCH 2/2] gpio: aspeed-sgpio: Convert aspeed_sgpio.lock to raw_spinlock

The gpio-aspeed-sgpio driver implements an irq_chip which need to be
invoked from hardirq context. Since spin_lock() can sleep with
PREEMPT_RT, it is no longer legal to invoke it while interrupts are
disabled.
This also causes lockdep to complain about:
[ 25.919465] [ BUG: Invalid wait context ]
because aspeed_sgpio.lock (spin_lock_t) is taken under irq_desc.lock
(raw_spinlock_t).
Let's use of raw_spinlock_t instead of spinlock_t.

Signed-off-by: Iwona Winiarska <[email protected]>
---
drivers/gpio/gpio-aspeed-sgpio.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c
index 3d6ef37a7702..931d5c38d7de 100644
--- a/drivers/gpio/gpio-aspeed-sgpio.c
+++ b/drivers/gpio/gpio-aspeed-sgpio.c
@@ -31,7 +31,7 @@ struct aspeed_sgpio {
struct gpio_chip chip;
struct irq_chip intc;
struct clk *pclk;
- spinlock_t lock;
+ raw_spinlock_t lock;
void __iomem *base;
int irq;
};
@@ -173,12 +173,12 @@ static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
enum aspeed_sgpio_reg reg;
int rc = 0;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

reg = aspeed_sgpio_is_input(offset) ? reg_val : reg_rdata;
rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return rc;
}
@@ -215,11 +215,11 @@ static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)
struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
unsigned long flags;

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

sgpio_set_value(gc, offset, val);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
@@ -236,9 +236,9 @@ static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int v
/* No special action is required for setting the direction; we'll
* error-out in sgpio_set_value if this isn't an output GPIO */

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);
rc = sgpio_set_value(gc, offset, val);
- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return rc;
}
@@ -277,11 +277,11 @@ static void aspeed_sgpio_irq_ack(struct irq_data *d)

status_addr = bank_reg(gpio, bank, reg_irq_status);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

iowrite32(bit, status_addr);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set)
@@ -296,7 +296,7 @@ static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set)
irqd_to_aspeed_sgpio_data(d, &gpio, &bank, &bit, &offset);
addr = bank_reg(gpio, bank, reg_irq_enable);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

reg = ioread32(addr);
if (set)
@@ -306,7 +306,7 @@ static void aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set)

iowrite32(reg, addr);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);
}

static void aspeed_sgpio_irq_mask(struct irq_data *d)
@@ -355,7 +355,7 @@ static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int type)
return -EINVAL;
}

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

addr = bank_reg(gpio, bank, reg_irq_type0);
reg = ioread32(addr);
@@ -372,7 +372,7 @@ static int aspeed_sgpio_set_type(struct irq_data *d, unsigned int type)
reg = (reg & ~bit) | type2;
iowrite32(reg, addr);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

irq_set_handler_locked(d, handler);

@@ -467,7 +467,7 @@ static int aspeed_sgpio_reset_tolerance(struct gpio_chip *chip,

reg = bank_reg(gpio, to_bank(offset), reg_tolerance);

- spin_lock_irqsave(&gpio->lock, flags);
+ raw_spin_lock_irqsave(&gpio->lock, flags);

val = readl(reg);

@@ -478,7 +478,7 @@ static int aspeed_sgpio_reset_tolerance(struct gpio_chip *chip,

writel(val, reg);

- spin_unlock_irqrestore(&gpio->lock, flags);
+ raw_spin_unlock_irqrestore(&gpio->lock, flags);

return 0;
}
@@ -575,7 +575,7 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
iowrite32(FIELD_PREP(ASPEED_SGPIO_CLK_DIV_MASK, sgpio_clk_div) | gpio_cnt_regval |
ASPEED_SGPIO_ENABLE, gpio->base + ASPEED_SGPIO_CTRL);

- spin_lock_init(&gpio->lock);
+ raw_spin_lock_init(&gpio->lock);

gpio->chip.parent = &pdev->dev;
gpio->chip.ngpio = nr_gpios * 2;
--
2.31.1


2021-12-10 15:15:55

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: aspeed: Convert aspeed_gpio.lock to raw_spinlock

On Sat, Dec 4, 2021 at 6:11 PM Iwona Winiarska
<[email protected]> wrote:
>
> The gpio-aspeed driver implements an irq_chip which need to be invoked
> from hardirq context. Since spin_lock() can sleep with PREEMPT_RT, it is
> no longer legal to invoke it while interrupts are disabled.
> This also causes lockdep to complain about:
> [ 0.649797] [ BUG: Invalid wait context ]
> because aspeed_gpio.lock (spin_lock_t) is taken under irq_desc.lock
> (raw_spinlock_t).
> Let's use of raw_spinlock_t instead of spinlock_t.
>
> Signed-off-by: Iwona Winiarska <[email protected]>
> ---

Applied, thanks!

Bart

2021-12-10 15:16:11

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 2/2] gpio: aspeed-sgpio: Convert aspeed_sgpio.lock to raw_spinlock

On Sat, Dec 4, 2021 at 6:11 PM Iwona Winiarska
<[email protected]> wrote:
>
> The gpio-aspeed-sgpio driver implements an irq_chip which need to be
> invoked from hardirq context. Since spin_lock() can sleep with
> PREEMPT_RT, it is no longer legal to invoke it while interrupts are
> disabled.
> This also causes lockdep to complain about:
> [ 25.919465] [ BUG: Invalid wait context ]
> because aspeed_sgpio.lock (spin_lock_t) is taken under irq_desc.lock
> (raw_spinlock_t).
> Let's use of raw_spinlock_t instead of spinlock_t.
>
> Signed-off-by: Iwona Winiarska <[email protected]>
> ---

Applied, thanks!

Bart