2017-03-21 23:57:17

by Julia Cartwright

Subject: [PATCH v2 0/9] fixup usage of non-raw spinlocks in irqchips

Changes since v1[1]:
- Dropped already applied patches in pinctrl, gpio, and mux trees.
- Gained three new patches destined for gpio tree, due to loosening constraints in
the semantic patch.

The following patchset introduces a new coccinelle patch,
irq_chip_raw_spinlock.cocci, which is used to identify irq_chip implementors
which acquire/release non-raw spinlocks, and in addition, a set of generated
patches for most cases identified.

On mainline builds, there exists no functional difference between
raw_spinlock_t and spinlock_t. However, w/ PREEMPT_RT, the spinlock_t will
cause the calling thread to sleep when the lock is contended. Because sleeping
is illegal in hardirq context, and because the irqchip callbacks are invoked in
hardirq context, irqchip implementations must not use spin_lock_t for
synchronization.

Patches build tested only. All patches are independent of one another and can
be applied to the relevant trees.

Some notes:

- In order to ensure that latency problems are not introduced for realtime
kernels, the generated patches need to be hand audited to ensure that
raw-spinlock protected regions are bounded and minimal. I've done a quick
audit for each of the modified drivers in this series, but please check my
work.

- There are a couple of matches which will require further intervention to
fully fix. Namely the Intel LPE Audio driver:

drivers/gpu/drm/i915/intel_lpe_audio.c:169:30-38: Use of non-raw spinlock is illegal in this context (struct drm_i915_private::irq_lock)

and the adi2 pinctrl driver:

drivers/pinctrl/pinctrl-adi2.c:308:26-30: Use of non-raw spinlock is illegal in this context (struct gpio_port::lock)

- This semantic patch is not comprehensive, there are other equally broken
usecases which are not properly identified yet, such as the use of a single
global spinlock_t declared via DEFINE_SPINLOCK() (found in arch/alpha, and
perhaps elsewhere).

An additional currently unhandled case is the irq_chip_generic callbacks.

Julia

[1]: http://lkml.kernel.org/r/[email protected]

Cc: Thomas Gleixner <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Julia Lawall <[email protected]>
Cc: Gilles Muller <[email protected]>
Cc: Nicolas Palix <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: [email protected]
--------------
Julia Cartwright (9):
Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in
irqchip implementations
alpha: marvel: make use of raw_spinlock variants
powerpc: mpc52xx_gpt: make use of raw_spinlock variants
mfd: asic3: make use of raw_spinlock variants
mfd: t7l66xb: make use of raw_spinlock variants
mfd: tc6393xb: make use of raw_spinlock variants
gpio: 104-idi-48: make use of raw_spinlock variants
gpio: 104-idio-16: make use of raw_spinlock variants
gpio: pci-idio-16: make use of raw_spinlock variants

arch/alpha/include/asm/core_marvel.h | 2 +-
arch/alpha/kernel/core_marvel.c | 2 +-
arch/alpha/kernel/sys_marvel.c | 12 +--
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 52 ++++++------
drivers/gpio/gpio-104-idi-48.c | 18 ++--
drivers/gpio/gpio-104-idio-16.c | 24 +++---
drivers/gpio/gpio-pci-idio-16.c | 28 +++----
drivers/mfd/asic3.c | 56 ++++++-------
drivers/mfd/t7l66xb.c | 20 ++---
drivers/mfd/tc6393xb.c | 52 ++++++------
.../coccinelle/locks/irq_chip_raw_spinlock.cocci | 96 ++++++++++++++++++++++
11 files changed, 230 insertions(+), 132 deletions(-)
create mode 100644 scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci

--
2.12.0

2017-03-21 22:48:07

by Julia Cartwright

Subject: [PATCH v2 1/9] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations

On PREEMPT_RT, the spinlock_t type becomes an object which sleeps under
contention. The codepaths used to support scheduling (irq dispatching, arch
code, the scheduler, timers) therefore must make use of the
raw_spin_lock{,_irq,_irqsave}() variations which preserve the non-sleeping
spinlock behavior.

Because the irq_chip callbacks are invoked in the process of interrupt
dispatch, they cannot therefore make use of spin_lock_t type. Instead, the
usage of raw_spinlock_t is appropriate.

Provide a spatch to identify (and attempt to patch) such problematic irqchip
implementations.

Note to those generating patches using this spatch; in order to maintain
correct semantics w/ PREEMPT_RT, it is necessary to audit the
raw_spinlock_t-protected codepaths to ensure their execution is bounded and
minimal. This is a manual audit process.

See commit 47b03ca903fb0 ("pinctrl: qcom: Use raw spinlock variants") as an
example of _one_ such instance, which fixed a real bug seen in the field.

Cc: Thomas Gleixner <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Linus Walleij <[email protected]>
Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- Make use of 'exists' on match to find any codepath which acquires a lock. (via Julia Lawall)
- Use 'when any' on '...' matches to loosen constraint (via Julia Lawall).

.../coccinelle/locks/irq_chip_raw_spinlock.cocci | 96 ++++++++++++++++++++++
1 file changed, 96 insertions(+)
create mode 100644 scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci

diff --git a/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
new file mode 100644
index 000000000000..0294831297d3
--- /dev/null
+++ b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
@@ -0,0 +1,96 @@
+// Copyright: (C) 2017 National Instruments Corp. GPLv2.
+// Author: Julia Cartwright <[email protected]>
+//
+// Identify callers of non-raw spinlock_t functions in hardirq irq_chip
+// callbacks.
+//
+// spin_lock{_irq,_irqsave}(), w/ PREEMPT_RT are "sleeping" spinlocks, and so
+// therefore "sleep" under contention; identify (and potentially patch) callers
+// to use raw_spinlock_t instead.
+//
+// Confidence: Moderate
+
+virtual report
+virtual patch
+
+@match@
+identifier __irqchip;
+identifier __irq_mask;
+@@
+ static struct irq_chip __irqchip = {
+ .irq_mask = __irq_mask,
+ };
+
+@match2 depends on match exists@
+identifier match.__irq_mask;
+identifier data;
+identifier x;
+identifier l;
+type T;
+position j0;
+expression flags;
+@@
+ static void __irq_mask(struct irq_data *data)
+ {
+ ... when any
+ T *x;
+ ... when any
+(
+ spin_lock_irqsave(&x->l@j0, flags);
+|
+ spin_lock_irq(&x->l@j0);
+|
+ spin_lock(&x->l@j0);
+)
+ ... when any
+ }
+
+@match3 depends on match2 && patch@
+type match2.T;
+identifier match2.l;
+@@
+ T {
+ ...
+- spinlock_t l;
++ raw_spinlock_t l;
+ ...
+ };
+
+@match4 depends on match2 && patch@
+type match2.T;
+identifier match2.l;
+expression flags;
+T *x;
+@@
+
+(
+-spin_lock(&x->l)
++raw_spin_lock(&x->l)
+|
+-spin_lock_irqsave(&x->l, flags)
++raw_spin_lock_irqsave(&x->l, flags)
+|
+-spin_lock_irq(&x->l)
++raw_spin_lock_irq(&x->l)
+|
+-spin_unlock(&x->l)
++raw_spin_unlock(&x->l)
+|
+-spin_unlock_irq(&x->l)
++raw_spin_unlock_irq(&x->l)
+|
+-spin_unlock_irqrestore(&x->l, flags)
++raw_spin_unlock_irqrestore(&x->l, flags)
+|
+-spin_lock_init(&x->l)
++raw_spin_lock_init(&x->l)
+)
+
+@script:python wat depends on match2 && report@
+j0 << match2.j0;
+t << match2.T;
+l << match2.l;
+@@
+
+msg = "Use of non-raw spinlock is illegal in this context (%s::%s)" % (t, l)
+coccilib.report.print_report(j0[0], msg)
--
2.12.0

2017-03-21 22:49:48

by Julia Cartwright

Subject: [PATCH v2 4/9] mfd: asic3: make use of raw_spinlock variants

The asic3 mfd 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.

Acked-for-MFD-by: Lee Jones <[email protected]>
Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- No functional change. Added Lee's ack.

drivers/mfd/asic3.c | 56 ++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
index 0413c8159551..cf2e25ab2940 100644
--- a/drivers/mfd/asic3.c
+++ b/drivers/mfd/asic3.c
@@ -78,7 +78,7 @@ struct asic3 {
unsigned int bus_shift;
unsigned int irq_nr;
unsigned int irq_base;
- spinlock_t lock;
+ raw_spinlock_t lock;
u16 irq_bothedge[4];
struct gpio_chip gpio;
struct device *dev;
@@ -108,14 +108,14 @@ static void asic3_set_register(struct asic3 *asic, u32 reg, u32 bits, bool set)
unsigned long flags;
u32 val;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
val = asic3_read_register(asic, reg);
if (set)
val |= bits;
else
val &= ~bits;
asic3_write_register(asic, reg, val);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

/* IRQs */
@@ -129,13 +129,13 @@ static void asic3_irq_flip_edge(struct asic3 *asic,
u16 edge;
unsigned long flags;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
edge = asic3_read_register(asic,
base + ASIC3_GPIO_EDGE_TRIGGER);
edge ^= bit;
asic3_write_register(asic,
base + ASIC3_GPIO_EDGE_TRIGGER, edge);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static void asic3_irq_demux(struct irq_desc *desc)
@@ -151,10 +151,10 @@ static void asic3_irq_demux(struct irq_desc *desc)
u32 status;
int bank;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
status = asic3_read_register(asic,
ASIC3_OFFSET(INTR, P_INT_STAT));
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);

/* Check all ten register bits */
if ((status & 0x3ff) == 0)
@@ -167,7 +167,7 @@ static void asic3_irq_demux(struct irq_desc *desc)

base = ASIC3_GPIO_A_BASE
+ bank * ASIC3_GPIO_BASE_INCR;
- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
istat = asic3_read_register(asic,
base +
ASIC3_GPIO_INT_STATUS);
@@ -175,7 +175,7 @@ static void asic3_irq_demux(struct irq_desc *desc)
asic3_write_register(asic,
base +
ASIC3_GPIO_INT_STATUS, 0);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);

for (i = 0; i < ASIC3_GPIOS_PER_BANK; i++) {
int bit = (1 << i);
@@ -230,11 +230,11 @@ static void asic3_mask_gpio_irq(struct irq_data *data)
bank = asic3_irq_to_bank(asic, data->irq);
index = asic3_irq_to_index(asic, data->irq);

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
val = asic3_read_register(asic, bank + ASIC3_GPIO_MASK);
val |= 1 << index;
asic3_write_register(asic, bank + ASIC3_GPIO_MASK, val);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static void asic3_mask_irq(struct irq_data *data)
@@ -243,7 +243,7 @@ static void asic3_mask_irq(struct irq_data *data)
int regval;
unsigned long flags;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
regval = asic3_read_register(asic,
ASIC3_INTR_BASE +
ASIC3_INTR_INT_MASK);
@@ -255,7 +255,7 @@ static void asic3_mask_irq(struct irq_data *data)
ASIC3_INTR_BASE +
ASIC3_INTR_INT_MASK,
regval);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static void asic3_unmask_gpio_irq(struct irq_data *data)
@@ -267,11 +267,11 @@ static void asic3_unmask_gpio_irq(struct irq_data *data)
bank = asic3_irq_to_bank(asic, data->irq);
index = asic3_irq_to_index(asic, data->irq);

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
val = asic3_read_register(asic, bank + ASIC3_GPIO_MASK);
val &= ~(1 << index);
asic3_write_register(asic, bank + ASIC3_GPIO_MASK, val);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static void asic3_unmask_irq(struct irq_data *data)
@@ -280,7 +280,7 @@ static void asic3_unmask_irq(struct irq_data *data)
int regval;
unsigned long flags;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
regval = asic3_read_register(asic,
ASIC3_INTR_BASE +
ASIC3_INTR_INT_MASK);
@@ -292,7 +292,7 @@ static void asic3_unmask_irq(struct irq_data *data)
ASIC3_INTR_BASE +
ASIC3_INTR_INT_MASK,
regval);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
@@ -306,7 +306,7 @@ static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
index = asic3_irq_to_index(asic, data->irq);
bit = 1<<index;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
level = asic3_read_register(asic,
bank + ASIC3_GPIO_LEVEL_TRIGGER);
edge = asic3_read_register(asic,
@@ -348,7 +348,7 @@ static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
edge);
asic3_write_register(asic, bank + ASIC3_GPIO_TRIGGER_TYPE,
trigger);
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
return 0;
}

@@ -455,7 +455,7 @@ static int asic3_gpio_direction(struct gpio_chip *chip,
return -EINVAL;
}

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

out_reg = asic3_read_register(asic, gpio_base + ASIC3_GPIO_DIRECTION);

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

asic3_write_register(asic, gpio_base + ASIC3_GPIO_DIRECTION, out_reg);

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

return 0;

@@ -524,7 +524,7 @@ static void asic3_gpio_set(struct gpio_chip *chip,

mask = ASIC3_GPIO_TO_MASK(offset);

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

out_reg = asic3_read_register(asic, gpio_base + ASIC3_GPIO_OUT);

@@ -535,7 +535,7 @@ static void asic3_gpio_set(struct gpio_chip *chip,

asic3_write_register(asic, gpio_base + ASIC3_GPIO_OUT, out_reg);

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

static int asic3_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
@@ -611,13 +611,13 @@ static void asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
unsigned long flags;
u32 cdex;

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
if (clk->enabled++ == 0) {
cdex = asic3_read_register(asic, ASIC3_OFFSET(CLOCK, CDEX));
cdex |= clk->cdex;
asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
}
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
@@ -627,13 +627,13 @@ static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)

WARN_ON(clk->enabled == 0);

- spin_lock_irqsave(&asic->lock, flags);
+ raw_spin_lock_irqsave(&asic->lock, flags);
if (--clk->enabled == 0) {
cdex = asic3_read_register(asic, ASIC3_OFFSET(CLOCK, CDEX));
cdex &= ~clk->cdex;
asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
}
- spin_unlock_irqrestore(&asic->lock, flags);
+ raw_spin_unlock_irqrestore(&asic->lock, flags);
}

/* MFD cells (SPI, PWM, LED, DS1WM, MMC) */
@@ -963,7 +963,7 @@ static int __init asic3_probe(struct platform_device *pdev)
if (!asic)
return -ENOMEM;

- spin_lock_init(&asic->lock);
+ raw_spin_lock_init(&asic->lock);
platform_set_drvdata(pdev, asic);
asic->dev = &pdev->dev;

--
2.12.0

2017-03-21 22:51:06

by Julia Cartwright

Subject: [PATCH v2 9/9] gpio: pci-idio-16: make use of raw_spinlock variants

The pci-idio-16 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.

Signed-off-by: Julia Cartwright <[email protected]>
---
New patch as of v2 of series.

drivers/gpio/gpio-pci-idio-16.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-pci-idio-16.c b/drivers/gpio/gpio-pci-idio-16.c
index 269ab628634b..7de4f6a2cb49 100644
--- a/drivers/gpio/gpio-pci-idio-16.c
+++ b/drivers/gpio/gpio-pci-idio-16.c
@@ -59,7 +59,7 @@ struct idio_16_gpio_reg {
*/
struct idio_16_gpio {
struct gpio_chip chip;
- spinlock_t lock;
+ raw_spinlock_t lock;
struct idio_16_gpio_reg __iomem *reg;
unsigned long irq_mask;
};
@@ -121,7 +121,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,
} else
base = &idio16gpio->reg->out0_7;

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

if (value)
out_state = ioread8(base) | mask;
@@ -130,7 +130,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,

iowrite8(out_state, base);

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

static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
@@ -140,7 +140,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
unsigned long flags;
unsigned int out_state;

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

/* process output lines 0-7 */
if (*mask & 0xFF) {
@@ -160,7 +160,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
iowrite8(out_state, &idio16gpio->reg->out8_15);
}

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

static void idio_16_irq_ack(struct irq_data *data)
@@ -177,11 +177,11 @@ static void idio_16_irq_mask(struct irq_data *data)
idio16gpio->irq_mask &= ~mask;

if (!idio16gpio->irq_mask) {
- spin_lock_irqsave(&idio16gpio->lock, flags);
+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);

iowrite8(0, &idio16gpio->reg->irq_ctl);

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

@@ -196,11 +196,11 @@ static void idio_16_irq_unmask(struct irq_data *data)
idio16gpio->irq_mask |= mask;

if (!prev_irq_mask) {
- spin_lock_irqsave(&idio16gpio->lock, flags);
+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);

ioread8(&idio16gpio->reg->irq_ctl);

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

@@ -229,11 +229,11 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
struct gpio_chip *const chip = &idio16gpio->chip;
int gpio;

- spin_lock(&idio16gpio->lock);
+ raw_spin_lock(&idio16gpio->lock);

irq_status = ioread8(&idio16gpio->reg->irq_status);

- spin_unlock(&idio16gpio->lock);
+ raw_spin_unlock(&idio16gpio->lock);

/* Make sure our device generated IRQ */
if (!(irq_status & 0x3) || !(irq_status & 0x4))
@@ -242,12 +242,12 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
generic_handle_irq(irq_find_mapping(chip->irqdomain, gpio));

- spin_lock(&idio16gpio->lock);
+ raw_spin_lock(&idio16gpio->lock);

/* Clear interrupt */
iowrite8(0, &idio16gpio->reg->in0_7);

- spin_unlock(&idio16gpio->lock);
+ raw_spin_unlock(&idio16gpio->lock);

return IRQ_HANDLED;
}
@@ -302,7 +302,7 @@ static int idio_16_probe(struct pci_dev *pdev, const struct pci_device_id *id)
idio16gpio->chip.set = idio_16_gpio_set;
idio16gpio->chip.set_multiple = idio_16_gpio_set_multiple;

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

err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
if (err) {
--
2.12.0

2017-03-21 22:54:46

by Julia Cartwright

Subject: [PATCH v2 6/9] mfd: tc6393xb: make use of raw_spinlock variants

The tc6393xb mfd 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.

Acked-for-MFD-by: Lee Jones <[email protected]>
Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- No functional change. Added Lee's ack.

drivers/mfd/tc6393xb.c | 52 +++++++++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
index d42d322ac7ca..d16e71bd9482 100644
--- a/drivers/mfd/tc6393xb.c
+++ b/drivers/mfd/tc6393xb.c
@@ -95,7 +95,7 @@ struct tc6393xb {

struct clk *clk; /* 3,6 Mhz */

- spinlock_t lock; /* protects RMW cycles */
+ raw_spinlock_t lock; /* protects RMW cycles */

struct {
u8 fer;
@@ -126,13 +126,13 @@ static int tc6393xb_nand_enable(struct platform_device *nand)
struct tc6393xb *tc6393xb = platform_get_drvdata(dev);
unsigned long flags;

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

/* SMD buffer on */
dev_dbg(&dev->dev, "SMD buffer on\n");
tmio_iowrite8(0xff, tc6393xb->scr + SCR_GPI_BCR(1));

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

return 0;
}
@@ -226,7 +226,7 @@ static int tc6393xb_ohci_enable(struct platform_device *dev)
u16 ccr;
u8 fer;

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

ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
ccr |= SCR_CCR_USBCK;
@@ -236,7 +236,7 @@ static int tc6393xb_ohci_enable(struct platform_device *dev)
fer |= SCR_FER_USBEN;
tmio_iowrite8(fer, tc6393xb->scr + SCR_FER);

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

return 0;
}
@@ -248,7 +248,7 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
u16 ccr;
u8 fer;

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

fer = tmio_ioread8(tc6393xb->scr + SCR_FER);
fer &= ~SCR_FER_USBEN;
@@ -258,7 +258,7 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
ccr &= ~SCR_CCR_USBCK;
tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);

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

return 0;
}
@@ -280,14 +280,14 @@ static int tc6393xb_fb_enable(struct platform_device *dev)
unsigned long flags;
u16 ccr;

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

ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
ccr &= ~SCR_CCR_MCLK_MASK;
ccr |= SCR_CCR_MCLK_48;
tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);

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

return 0;
}
@@ -298,14 +298,14 @@ static int tc6393xb_fb_disable(struct platform_device *dev)
unsigned long flags;
u16 ccr;

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

ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
ccr &= ~SCR_CCR_MCLK_MASK;
ccr |= SCR_CCR_MCLK_OFF;
tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);

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

return 0;
}
@@ -317,7 +317,7 @@ int tc6393xb_lcd_set_power(struct platform_device *fb, bool on)
u8 fer;
unsigned long flags;

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

fer = ioread8(tc6393xb->scr + SCR_FER);
if (on)
@@ -326,7 +326,7 @@ int tc6393xb_lcd_set_power(struct platform_device *fb, bool on)
fer &= ~SCR_FER_SLCDEN;
iowrite8(fer, tc6393xb->scr + SCR_FER);

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

return 0;
}
@@ -338,12 +338,12 @@ int tc6393xb_lcd_mode(struct platform_device *fb,
struct tc6393xb *tc6393xb = platform_get_drvdata(dev);
unsigned long flags;

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

iowrite16(mode->pixclock, tc6393xb->scr + SCR_PLL1CR + 0);
iowrite16(mode->pixclock >> 16, tc6393xb->scr + SCR_PLL1CR + 2);

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

return 0;
}
@@ -462,11 +462,11 @@ static void tc6393xb_gpio_set(struct gpio_chip *chip,
struct tc6393xb *tc6393xb = gpiochip_get_data(chip);
unsigned long flags;

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

__tc6393xb_gpio_set(chip, offset, value);

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

static int tc6393xb_gpio_direction_input(struct gpio_chip *chip,
@@ -476,13 +476,13 @@ static int tc6393xb_gpio_direction_input(struct gpio_chip *chip,
unsigned long flags;
u8 doecr;

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

doecr = tmio_ioread8(tc6393xb->scr + SCR_GPO_DOECR(offset / 8));
doecr &= ~TC_GPIO_BIT(offset);
tmio_iowrite8(doecr, tc6393xb->scr + SCR_GPO_DOECR(offset / 8));

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

return 0;
}
@@ -494,7 +494,7 @@ static int tc6393xb_gpio_direction_output(struct gpio_chip *chip,
unsigned long flags;
u8 doecr;

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

__tc6393xb_gpio_set(chip, offset, value);

@@ -502,7 +502,7 @@ static int tc6393xb_gpio_direction_output(struct gpio_chip *chip,
doecr |= TC_GPIO_BIT(offset);
tmio_iowrite8(doecr, tc6393xb->scr + SCR_GPO_DOECR(offset / 8));

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

return 0;
}
@@ -548,11 +548,11 @@ static void tc6393xb_irq_mask(struct irq_data *data)
unsigned long flags;
u8 imr;

- spin_lock_irqsave(&tc6393xb->lock, flags);
+ raw_spin_lock_irqsave(&tc6393xb->lock, flags);
imr = tmio_ioread8(tc6393xb->scr + SCR_IMR);
imr |= 1 << (data->irq - tc6393xb->irq_base);
tmio_iowrite8(imr, tc6393xb->scr + SCR_IMR);
- spin_unlock_irqrestore(&tc6393xb->lock, flags);
+ raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
}

static void tc6393xb_irq_unmask(struct irq_data *data)
@@ -561,11 +561,11 @@ static void tc6393xb_irq_unmask(struct irq_data *data)
unsigned long flags;
u8 imr;

- spin_lock_irqsave(&tc6393xb->lock, flags);
+ raw_spin_lock_irqsave(&tc6393xb->lock, flags);
imr = tmio_ioread8(tc6393xb->scr + SCR_IMR);
imr &= ~(1 << (data->irq - tc6393xb->irq_base));
tmio_iowrite8(imr, tc6393xb->scr + SCR_IMR);
- spin_unlock_irqrestore(&tc6393xb->lock, flags);
+ raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
}

static struct irq_chip tc6393xb_chip = {
@@ -628,7 +628,7 @@ static int tc6393xb_probe(struct platform_device *dev)
goto err_kzalloc;
}

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

platform_set_drvdata(dev, tc6393xb);

--
2.12.0

2017-03-21 23:28:43

by Julia Cartwright

Subject: [PATCH v2 3/9] powerpc: mpc52xx_gpt: make use of raw_spinlock variants

The mpc52xx_gpt code 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.

Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- No change.

arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 52 +++++++++++++++----------------
1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 22645a7c6b8a..18c1383717f2 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -90,7 +90,7 @@ struct mpc52xx_gpt_priv {
struct list_head list; /* List of all GPT devices */
struct device *dev;
struct mpc52xx_gpt __iomem *regs;
- spinlock_t lock;
+ raw_spinlock_t lock;
struct irq_domain *irqhost;
u32 ipb_freq;
u8 wdt_mode;
@@ -141,9 +141,9 @@ static void mpc52xx_gpt_irq_unmask(struct irq_data *d)
struct mpc52xx_gpt_priv *gpt = irq_data_get_irq_chip_data(d);
unsigned long flags;

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
}

static void mpc52xx_gpt_irq_mask(struct irq_data *d)
@@ -151,9 +151,9 @@ static void mpc52xx_gpt_irq_mask(struct irq_data *d)
struct mpc52xx_gpt_priv *gpt = irq_data_get_irq_chip_data(d);
unsigned long flags;

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
}

static void mpc52xx_gpt_irq_ack(struct irq_data *d)
@@ -171,14 +171,14 @@ static int mpc52xx_gpt_irq_set_type(struct irq_data *d, unsigned int flow_type)

dev_dbg(gpt->dev, "%s: virq=%i type=%x\n", __func__, d->irq, flow_type);

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
reg = in_be32(&gpt->regs->mode) & ~MPC52xx_GPT_MODE_ICT_MASK;
if (flow_type & IRQF_TRIGGER_RISING)
reg |= MPC52xx_GPT_MODE_ICT_RISING;
if (flow_type & IRQF_TRIGGER_FALLING)
reg |= MPC52xx_GPT_MODE_ICT_FALLING;
out_be32(&gpt->regs->mode, reg);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);

return 0;
}
@@ -264,11 +264,11 @@ mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt, struct device_node *node)
/* If the GPT is currently disabled, then change it to be in Input
* Capture mode. If the mode is non-zero, then the pin could be
* already in use for something. */
- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
mode = in_be32(&gpt->regs->mode);
if ((mode & MPC52xx_GPT_MODE_MS_MASK) == 0)
out_be32(&gpt->regs->mode, mode | MPC52xx_GPT_MODE_MS_IC);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);

dev_dbg(gpt->dev, "%s() complete. virq=%i\n", __func__, cascade_virq);
}
@@ -295,9 +295,9 @@ mpc52xx_gpt_gpio_set(struct gpio_chip *gc, unsigned int gpio, int v)
dev_dbg(gpt->dev, "%s: gpio:%d v:%d\n", __func__, gpio, v);
r = v ? MPC52xx_GPT_MODE_GPIO_OUT_HIGH : MPC52xx_GPT_MODE_GPIO_OUT_LOW;

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
clrsetbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK, r);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
}

static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
@@ -307,9 +307,9 @@ static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)

dev_dbg(gpt->dev, "%s: gpio:%d\n", __func__, gpio);

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);

return 0;
}
@@ -436,16 +436,16 @@ static int mpc52xx_gpt_do_start(struct mpc52xx_gpt_priv *gpt, u64 period,
}

/* Set and enable the timer, reject an attempt to use a wdt as gpt */
- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
if (as_wdt)
gpt->wdt_mode |= MPC52xx_GPT_IS_WDT;
else if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT) != 0) {
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
return -EBUSY;
}
out_be32(&gpt->regs->count, prescale << 16 | clocks);
clrsetbits_be32(&gpt->regs->mode, clear, set);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);

return 0;
}
@@ -476,14 +476,14 @@ int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
unsigned long flags;

/* reject the operation if the timer is used as watchdog (gpt 0 only) */
- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
if ((gpt->wdt_mode & MPC52xx_GPT_IS_WDT) != 0) {
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
return -EBUSY;
}

clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);
return 0;
}
EXPORT_SYMBOL(mpc52xx_gpt_stop_timer);
@@ -500,9 +500,9 @@ u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt)
u64 prescale;
unsigned long flags;

- spin_lock_irqsave(&gpt->lock, flags);
+ raw_spin_lock_irqsave(&gpt->lock, flags);
period = in_be32(&gpt->regs->count);
- spin_unlock_irqrestore(&gpt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt->lock, flags);

prescale = period >> 16;
period &= 0xffff;
@@ -532,9 +532,9 @@ static inline void mpc52xx_gpt_wdt_ping(struct mpc52xx_gpt_priv *gpt_wdt)
{
unsigned long flags;

- spin_lock_irqsave(&gpt_wdt->lock, flags);
+ raw_spin_lock_irqsave(&gpt_wdt->lock, flags);
out_8((u8 *) &gpt_wdt->regs->mode, MPC52xx_GPT_MODE_WDT_PING);
- spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt_wdt->lock, flags);
}

/* wdt misc device api */
@@ -638,11 +638,11 @@ static int mpc52xx_wdt_release(struct inode *inode, struct file *file)
struct mpc52xx_gpt_priv *gpt_wdt = file->private_data;
unsigned long flags;

- spin_lock_irqsave(&gpt_wdt->lock, flags);
+ raw_spin_lock_irqsave(&gpt_wdt->lock, flags);
clrbits32(&gpt_wdt->regs->mode,
MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52xx_GPT_MODE_WDT_EN);
gpt_wdt->wdt_mode &= ~MPC52xx_GPT_IS_WDT;
- spin_unlock_irqrestore(&gpt_wdt->lock, flags);
+ raw_spin_unlock_irqrestore(&gpt_wdt->lock, flags);
#endif
clear_bit(0, &wdt_is_active);
return 0;
@@ -723,7 +723,7 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev)
if (!gpt)
return -ENOMEM;

- spin_lock_init(&gpt->lock);
+ raw_spin_lock_init(&gpt->lock);
gpt->dev = &ofdev->dev;
gpt->ipb_freq = mpc5xxx_get_bus_frequency(ofdev->dev.of_node);
gpt->regs = of_iomap(ofdev->dev.of_node, 0);
--
2.12.0

2017-03-21 23:32:03

by Julia Cartwright

Subject: [PATCH v2 5/9] mfd: t7l66xb: make use of raw_spinlock variants

The t7l66xb mfd 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.

Acked-for-MFD-by: Lee Jones <[email protected]>
Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- No functional change. Added Lee's ack.

drivers/mfd/t7l66xb.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/t7l66xb.c b/drivers/mfd/t7l66xb.c
index 94bd89cb1f06..22c811396edc 100644
--- a/drivers/mfd/t7l66xb.c
+++ b/drivers/mfd/t7l66xb.c
@@ -69,7 +69,7 @@ static const struct resource t7l66xb_mmc_resources[] = {
struct t7l66xb {
void __iomem *scr;
/* Lock to protect registers requiring read/modify/write ops. */
- spinlock_t lock;
+ raw_spinlock_t lock;

struct resource rscr;
struct clk *clk48m;
@@ -89,13 +89,13 @@ static int t7l66xb_mmc_enable(struct platform_device *mmc)

clk_prepare_enable(t7l66xb->clk32k);

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

dev_ctl = tmio_ioread8(t7l66xb->scr + SCR_DEV_CTL);
dev_ctl |= SCR_DEV_CTL_MMC;
tmio_iowrite8(dev_ctl, t7l66xb->scr + SCR_DEV_CTL);

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

tmio_core_mmc_enable(t7l66xb->scr + 0x200, 0,
t7l66xb_mmc_resources[0].start & 0xfffe);
@@ -110,13 +110,13 @@ static int t7l66xb_mmc_disable(struct platform_device *mmc)
unsigned long flags;
u8 dev_ctl;

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

dev_ctl = tmio_ioread8(t7l66xb->scr + SCR_DEV_CTL);
dev_ctl &= ~SCR_DEV_CTL_MMC;
tmio_iowrite8(dev_ctl, t7l66xb->scr + SCR_DEV_CTL);

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

clk_disable_unprepare(t7l66xb->clk32k);

@@ -206,11 +206,11 @@ static void t7l66xb_irq_mask(struct irq_data *data)
unsigned long flags;
u8 imr;

- spin_lock_irqsave(&t7l66xb->lock, flags);
+ raw_spin_lock_irqsave(&t7l66xb->lock, flags);
imr = tmio_ioread8(t7l66xb->scr + SCR_IMR);
imr |= 1 << (data->irq - t7l66xb->irq_base);
tmio_iowrite8(imr, t7l66xb->scr + SCR_IMR);
- spin_unlock_irqrestore(&t7l66xb->lock, flags);
+ raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
}

static void t7l66xb_irq_unmask(struct irq_data *data)
@@ -219,11 +219,11 @@ static void t7l66xb_irq_unmask(struct irq_data *data)
unsigned long flags;
u8 imr;

- spin_lock_irqsave(&t7l66xb->lock, flags);
+ raw_spin_lock_irqsave(&t7l66xb->lock, flags);
imr = tmio_ioread8(t7l66xb->scr + SCR_IMR);
imr &= ~(1 << (data->irq - t7l66xb->irq_base));
tmio_iowrite8(imr, t7l66xb->scr + SCR_IMR);
- spin_unlock_irqrestore(&t7l66xb->lock, flags);
+ raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
}

static struct irq_chip t7l66xb_chip = {
@@ -321,7 +321,7 @@ static int t7l66xb_probe(struct platform_device *dev)
if (!t7l66xb)
return -ENOMEM;

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

platform_set_drvdata(dev, t7l66xb);

--
2.12.0

2017-03-21 23:37:25

by Julia Cartwright

Subject: [PATCH v2 2/9] alpha: marvel: make use of raw_spinlock variants

The alpha/marvel code 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.

Signed-off-by: Julia Cartwright <[email protected]>
---
v1 -> v2:
- Add hunk which properly initializes the spin-lock (via kbuild bot).

arch/alpha/include/asm/core_marvel.h | 2 +-
arch/alpha/kernel/core_marvel.c | 2 +-
arch/alpha/kernel/sys_marvel.c | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/alpha/include/asm/core_marvel.h b/arch/alpha/include/asm/core_marvel.h
index dad300fa14ce..8dcf9dbda618 100644
--- a/arch/alpha/include/asm/core_marvel.h
+++ b/arch/alpha/include/asm/core_marvel.h
@@ -312,7 +312,7 @@ struct io7 {
io7_port7_csrs *csrs;
struct io7_port ports[IO7_NUM_PORTS];

- spinlock_t irq_lock;
+ raw_spinlock_t irq_lock;
};

#ifndef __EXTERN_INLINE
diff --git a/arch/alpha/kernel/core_marvel.c b/arch/alpha/kernel/core_marvel.c
index d5f0580746a5..4300b1d1cf2c 100644
--- a/arch/alpha/kernel/core_marvel.c
+++ b/arch/alpha/kernel/core_marvel.c
@@ -118,7 +118,7 @@ alloc_io7(unsigned int pe)

io7 = alloc_bootmem(sizeof(*io7));
io7->pe = pe;
- spin_lock_init(&io7->irq_lock);
+ raw_spin_lock_init(&io7->irq_lock);

for (h = 0; h < 4; h++) {
io7->ports[h].io7 = io7;
diff --git a/arch/alpha/kernel/sys_marvel.c b/arch/alpha/kernel/sys_marvel.c
index 24e41bd7d3c9..3e533920371f 100644
--- a/arch/alpha/kernel/sys_marvel.c
+++ b/arch/alpha/kernel/sys_marvel.c
@@ -115,11 +115,11 @@ io7_enable_irq(struct irq_data *d)
return;
}

- spin_lock(&io7->irq_lock);
+ raw_spin_lock(&io7->irq_lock);
*ctl |= 1UL << 24;
mb();
*ctl;
- spin_unlock(&io7->irq_lock);
+ raw_spin_unlock(&io7->irq_lock);
}

static void
@@ -136,11 +136,11 @@ io7_disable_irq(struct irq_data *d)
return;
}

- spin_lock(&io7->irq_lock);
+ raw_spin_lock(&io7->irq_lock);
*ctl &= ~(1UL << 24);
mb();
*ctl;
- spin_unlock(&io7->irq_lock);
+ raw_spin_unlock(&io7->irq_lock);
}

static void
@@ -263,7 +263,7 @@ init_io7_irqs(struct io7 *io7,
*/
printk(" Interrupts reported to CPU at PE %u\n", boot_cpuid);

- spin_lock(&io7->irq_lock);
+ raw_spin_lock(&io7->irq_lock);

/* set up the error irqs */
io7_redirect_irq(io7, &io7->csrs->HLT_CTL.csr, boot_cpuid);
@@ -295,7 +295,7 @@ init_io7_irqs(struct io7 *io7,
for (i = 0; i < 16; ++i)
init_one_io7_msi(io7, i, boot_cpuid);

- spin_unlock(&io7->irq_lock);
+ raw_spin_unlock(&io7->irq_lock);
}

static void __init
--
2.12.0

2017-03-21 23:48:57

by Julia Cartwright

Subject: [PATCH v2 8/9] gpio: 104-idio-16: make use of raw_spinlock variants

The 104-idio-16 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.

Signed-off-by: Julia Cartwright <[email protected]>
---
New patch as of v2 of series.

drivers/gpio/gpio-104-idio-16.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c
index 7053cf736648..5281e1cedb01 100644
--- a/drivers/gpio/gpio-104-idio-16.c
+++ b/drivers/gpio/gpio-104-idio-16.c
@@ -50,7 +50,7 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDIO-16 interrupt line numbers");
*/
struct idio_16_gpio {
struct gpio_chip chip;
- spinlock_t lock;
+ raw_spinlock_t lock;
unsigned long irq_mask;
unsigned base;
unsigned out_state;
@@ -99,7 +99,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
if (offset > 15)
return;

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

if (value)
idio16gpio->out_state |= mask;
@@ -111,7 +111,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
else
outb(idio16gpio->out_state, idio16gpio->base);

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

static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
@@ -120,7 +120,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
unsigned long flags;

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

idio16gpio->out_state &= ~*mask;
idio16gpio->out_state |= *mask & *bits;
@@ -130,7 +130,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
if ((*mask >> 8) & 0xFF)
outb(idio16gpio->out_state >> 8, idio16gpio->base + 4);

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

static void idio_16_irq_ack(struct irq_data *data)
@@ -147,11 +147,11 @@ static void idio_16_irq_mask(struct irq_data *data)
idio16gpio->irq_mask &= ~mask;

if (!idio16gpio->irq_mask) {
- spin_lock_irqsave(&idio16gpio->lock, flags);
+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);

outb(0, idio16gpio->base + 2);

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

@@ -166,11 +166,11 @@ static void idio_16_irq_unmask(struct irq_data *data)
idio16gpio->irq_mask |= mask;

if (!prev_irq_mask) {
- spin_lock_irqsave(&idio16gpio->lock, flags);
+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);

inb(idio16gpio->base + 2);

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

@@ -201,11 +201,11 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
generic_handle_irq(irq_find_mapping(chip->irqdomain, gpio));

- spin_lock(&idio16gpio->lock);
+ raw_spin_lock(&idio16gpio->lock);

outb(0, idio16gpio->base + 1);

- spin_unlock(&idio16gpio->lock);
+ raw_spin_unlock(&idio16gpio->lock);

return IRQ_HANDLED;
}
@@ -249,7 +249,7 @@ static int idio_16_probe(struct device *dev, unsigned int id)
idio16gpio->base = base[id];
idio16gpio->out_state = 0xFFFF;

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

err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
if (err) {
--
2.12.0

2017-03-21 23:57:47

by Julia Cartwright

Subject: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

The 104-idi-48 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.

Signed-off-by: Julia Cartwright <[email protected]>
---
New patch as of v2 of series.

drivers/gpio/gpio-104-idi-48.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
index 568375a7ebc2..337c048168d8 100644
--- a/drivers/gpio/gpio-104-idi-48.c
+++ b/drivers/gpio/gpio-104-idi-48.c
@@ -51,7 +51,7 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");
*/
struct idi_48_gpio {
struct gpio_chip chip;
- spinlock_t lock;
+ raw_spinlock_t lock;
spinlock_t ack_lock;
unsigned char irq_mask[6];
unsigned base;
@@ -112,11 +112,12 @@ static void idi_48_irq_mask(struct irq_data *data)
if (!idi48gpio->irq_mask[boundary]) {
idi48gpio->cos_enb &= ~BIT(boundary);

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

outb(idi48gpio->cos_enb, idi48gpio->base + 7);

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

return;
@@ -145,11 +146,12 @@ static void idi_48_irq_unmask(struct irq_data *data)
if (!prev_irq_mask) {
idi48gpio->cos_enb |= BIT(boundary);

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

outb(idi48gpio->cos_enb, idi48gpio->base + 7);

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

return;
@@ -186,11 +188,11 @@ static irqreturn_t idi_48_irq_handler(int irq, void *dev_id)

spin_lock(&idi48gpio->ack_lock);

- spin_lock(&idi48gpio->lock);
+ raw_spin_lock(&idi48gpio->lock);

cos_status = inb(idi48gpio->base + 7);

- spin_unlock(&idi48gpio->lock);
+ raw_spin_unlock(&idi48gpio->lock);

/* IRQ Status (bit 6) is active low (0 = IRQ generated by device) */
if (cos_status & BIT(6)) {
@@ -256,7 +258,7 @@ static int idi_48_probe(struct device *dev, unsigned int id)
idi48gpio->chip.get = idi_48_gpio_get;
idi48gpio->base = base[id];

- spin_lock_init(&idi48gpio->lock);
+ raw_spin_lock_init(&idi48gpio->lock);
spin_lock_init(&idi48gpio->ack_lock);

err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio);
--
2.12.0

2017-03-22 09:54:25

by Julia Lawall

Subject: Re: [PATCH v2 1/9] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations

On Tue, 21 Mar 2017, Julia Cartwright wrote:

> On PREEMPT_RT, the spinlock_t type becomes an object which sleeps under
> contention. The codepaths used to support scheduling (irq dispatching, arch
> code, the scheduler, timers) therefore must make use of the
> raw_spin_lock{,_irq,_irqsave}() variations which preserve the non-sleeping
> spinlock behavior.
>
> Because the irq_chip callbacks are invoked in the process of interrupt
> dispatch, they cannot therefore make use of spin_lock_t type. Instead, the
> usage of raw_spinlock_t is appropriate.
>
> Provide a spatch to identify (and attempt to patch) such problematic irqchip
> implementations.
>
> Note to those generating patches using this spatch; in order to maintain
> correct semantics w/ PREEMPT_RT, it is necessary to audit the
> raw_spinlock_t-protected codepaths to ensure their execution is bounded and
> minimal. This is a manual audit process.
>
> See commit 47b03ca903fb0 ("pinctrl: qcom: Use raw spinlock variants") as an
> example of _one_ such instance, which fixed a real bug seen in the field.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Sebastian Andrzej Siewior <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Signed-off-by: Julia Cartwright <[email protected]>

Acked-by: Julia Lawall <[email protected]>

> ---
> v1 -> v2:
> - Make use of 'exists' on match to find any codepath which acquires a lock. (via Julia Lawall)
> - Use 'when any' on '...' matches to loosen constraint (via Julia Lawall).
>
> .../coccinelle/locks/irq_chip_raw_spinlock.cocci | 96 ++++++++++++++++++++++
> 1 file changed, 96 insertions(+)
> create mode 100644 scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
>
> diff --git a/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
> new file mode 100644
> index 000000000000..0294831297d3
> --- /dev/null
> +++ b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
> @@ -0,0 +1,96 @@
> +// Copyright: (C) 2017 National Instruments Corp. GPLv2.
> +// Author: Julia Cartwright <[email protected]>
> +//
> +// Identify callers of non-raw spinlock_t functions in hardirq irq_chip
> +// callbacks.
> +//
> +// spin_lock{_irq,_irqsave}(), w/ PREEMPT_RT are "sleeping" spinlocks, and so
> +// therefore "sleep" under contention; identify (and potentially patch) callers
> +// to use raw_spinlock_t instead.
> +//
> +// Confidence: Moderate
> +
> +virtual report
> +virtual patch
> +
> +@match@
> +identifier __irqchip;
> +identifier __irq_mask;
> +@@
> + static struct irq_chip __irqchip = {
> + .irq_mask = __irq_mask,
> + };
> +
> +@match2 depends on match exists@
> +identifier match.__irq_mask;
> +identifier data;
> +identifier x;
> +identifier l;
> +type T;
> +position j0;
> +expression flags;
> +@@
> + static void __irq_mask(struct irq_data *data)
> + {
> + ... when any
> + T *x;
> + ... when any
> +(
> + spin_lock_irqsave(&x->l@j0, flags);
> +|
> + spin_lock_irq(&x->l@j0);
> +|
> + spin_lock(&x->l@j0);
> +)
> + ... when any
> + }
> +
> +@match3 depends on match2 && patch@
> +type match2.T;
> +identifier match2.l;
> +@@
> + T {
> + ...
> +- spinlock_t l;
> ++ raw_spinlock_t l;
> + ...
> + };
> +
> +@match4 depends on match2 && patch@
> +type match2.T;
> +identifier match2.l;
> +expression flags;
> +T *x;
> +@@
> +
> +(
> +-spin_lock(&x->l)
> ++raw_spin_lock(&x->l)
> +|
> +-spin_lock_irqsave(&x->l, flags)
> ++raw_spin_lock_irqsave(&x->l, flags)
> +|
> +-spin_lock_irq(&x->l)
> ++raw_spin_lock_irq(&x->l)
> +|
> +-spin_unlock(&x->l)
> ++raw_spin_unlock(&x->l)
> +|
> +-spin_unlock_irq(&x->l)
> ++raw_spin_unlock_irq(&x->l)
> +|
> +-spin_unlock_irqrestore(&x->l, flags)
> ++raw_spin_unlock_irqrestore(&x->l, flags)
> +|
> +-spin_lock_init(&x->l)
> ++raw_spin_lock_init(&x->l)
> +)
> +
> +@script:python wat depends on match2 && report@
> +j0 << match2.j0;
> +t << match2.T;
> +l << match2.l;
> +@@
> +
> +msg = "Use of non-raw spinlock is illegal in this context (%s::%s)" % (t, l)
> +coccilib.report.print_report(j0[0], msg)
> --
> 2.12.0
>
>

2017-03-22 12:48:28

by William Breathitt Gray

Subject: Re: [PATCH v2 8/9] gpio: 104-idio-16: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 05:43:08PM -0500, Julia Cartwright wrote:
>The 104-idio-16 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.
>
>Signed-off-by: Julia Cartwright <[email protected]>

Acked-by: William Breathitt Gray <[email protected]>

>---
>New patch as of v2 of series.
>
> drivers/gpio/gpio-104-idio-16.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c
>index 7053cf736648..5281e1cedb01 100644
>--- a/drivers/gpio/gpio-104-idio-16.c
>+++ b/drivers/gpio/gpio-104-idio-16.c
>@@ -50,7 +50,7 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDIO-16 interrupt line numbers");
> */
> struct idio_16_gpio {
> struct gpio_chip chip;
>- spinlock_t lock;
>+ raw_spinlock_t lock;
> unsigned long irq_mask;
> unsigned base;
> unsigned out_state;
>@@ -99,7 +99,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> if (offset > 15)
> return;
>
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> if (value)
> idio16gpio->out_state |= mask;
>@@ -111,7 +111,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> else
> outb(idio16gpio->out_state, idio16gpio->base);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
>
> static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
>@@ -120,7 +120,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
> struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
> unsigned long flags;
>
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> idio16gpio->out_state &= ~*mask;
> idio16gpio->out_state |= *mask & *bits;
>@@ -130,7 +130,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
> if ((*mask >> 8) & 0xFF)
> outb(idio16gpio->out_state >> 8, idio16gpio->base + 4);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
>
> static void idio_16_irq_ack(struct irq_data *data)
>@@ -147,11 +147,11 @@ static void idio_16_irq_mask(struct irq_data *data)
> idio16gpio->irq_mask &= ~mask;
>
> if (!idio16gpio->irq_mask) {
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> outb(0, idio16gpio->base + 2);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
> }
>
>@@ -166,11 +166,11 @@ static void idio_16_irq_unmask(struct irq_data *data)
> idio16gpio->irq_mask |= mask;
>
> if (!prev_irq_mask) {
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> inb(idio16gpio->base + 2);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
> }
>
>@@ -201,11 +201,11 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
> for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
> generic_handle_irq(irq_find_mapping(chip->irqdomain, gpio));
>
>- spin_lock(&idio16gpio->lock);
>+ raw_spin_lock(&idio16gpio->lock);
>
> outb(0, idio16gpio->base + 1);
>
>- spin_unlock(&idio16gpio->lock);
>+ raw_spin_unlock(&idio16gpio->lock);
>
> return IRQ_HANDLED;
> }
>@@ -249,7 +249,7 @@ static int idio_16_probe(struct device *dev, unsigned int id)
> idio16gpio->base = base[id];
> idio16gpio->out_state = 0xFFFF;
>
>- spin_lock_init(&idio16gpio->lock);
>+ raw_spin_lock_init(&idio16gpio->lock);
>
> err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
> if (err) {
>--
>2.12.0
>

2017-03-22 12:51:31

by William Breathitt Gray

Subject: Re: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 05:43:07PM -0500, Julia Cartwright wrote:
>The 104-idi-48 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.
>
>Signed-off-by: Julia Cartwright <[email protected]>

Hi Julia,

This driver also uses a second spinlock_t, called ack_lock, to prevent
reentrance into the idi_48_irq_handler function. Should ack_lock also be
implemented as a raw_spinlock_t?

Thanks,

William Breathitt Gray

>---
>New patch as of v2 of series.
>
> drivers/gpio/gpio-104-idi-48.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
>index 568375a7ebc2..337c048168d8 100644
>--- a/drivers/gpio/gpio-104-idi-48.c
>+++ b/drivers/gpio/gpio-104-idi-48.c
>@@ -51,7 +51,7 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");
> */
> struct idi_48_gpio {
> struct gpio_chip chip;
>- spinlock_t lock;
>+ raw_spinlock_t lock;
> spinlock_t ack_lock;
> unsigned char irq_mask[6];
> unsigned base;
>@@ -112,11 +112,12 @@ static void idi_48_irq_mask(struct irq_data *data)
> if (!idi48gpio->irq_mask[boundary]) {
> idi48gpio->cos_enb &= ~BIT(boundary);
>
>- spin_lock_irqsave(&idi48gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idi48gpio->lock, flags);
>
> outb(idi48gpio->cos_enb, idi48gpio->base + 7);
>
>- spin_unlock_irqrestore(&idi48gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idi48gpio->lock,
>+ flags);
> }
>
> return;
>@@ -145,11 +146,12 @@ static void idi_48_irq_unmask(struct irq_data *data)
> if (!prev_irq_mask) {
> idi48gpio->cos_enb |= BIT(boundary);
>
>- spin_lock_irqsave(&idi48gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idi48gpio->lock, flags);
>
> outb(idi48gpio->cos_enb, idi48gpio->base + 7);
>
>- spin_unlock_irqrestore(&idi48gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idi48gpio->lock,
>+ flags);
> }
>
> return;
>@@ -186,11 +188,11 @@ static irqreturn_t idi_48_irq_handler(int irq, void *dev_id)
>
> spin_lock(&idi48gpio->ack_lock);
>
>- spin_lock(&idi48gpio->lock);
>+ raw_spin_lock(&idi48gpio->lock);
>
> cos_status = inb(idi48gpio->base + 7);
>
>- spin_unlock(&idi48gpio->lock);
>+ raw_spin_unlock(&idi48gpio->lock);
>
> /* IRQ Status (bit 6) is active low (0 = IRQ generated by device) */
> if (cos_status & BIT(6)) {
>@@ -256,7 +258,7 @@ static int idi_48_probe(struct device *dev, unsigned int id)
> idi48gpio->chip.get = idi_48_gpio_get;
> idi48gpio->base = base[id];
>
>- spin_lock_init(&idi48gpio->lock);
>+ raw_spin_lock_init(&idi48gpio->lock);
> spin_lock_init(&idi48gpio->ack_lock);
>
> err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio);
>--
>2.12.0
>

2017-03-22 12:55:18

by William Breathitt Gray

Subject: Re: [PATCH v2 9/9] gpio: pci-idio-16: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 05:43:09PM -0500, Julia Cartwright wrote:
>The pci-idio-16 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.
>
>Signed-off-by: Julia Cartwright <[email protected]>

Acked-by: William Breathitt Gray <[email protected]>

>---
>New patch as of v2 of series.
>
> drivers/gpio/gpio-pci-idio-16.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/gpio/gpio-pci-idio-16.c b/drivers/gpio/gpio-pci-idio-16.c
>index 269ab628634b..7de4f6a2cb49 100644
>--- a/drivers/gpio/gpio-pci-idio-16.c
>+++ b/drivers/gpio/gpio-pci-idio-16.c
>@@ -59,7 +59,7 @@ struct idio_16_gpio_reg {
> */
> struct idio_16_gpio {
> struct gpio_chip chip;
>- spinlock_t lock;
>+ raw_spinlock_t lock;
> struct idio_16_gpio_reg __iomem *reg;
> unsigned long irq_mask;
> };
>@@ -121,7 +121,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,
> } else
> base = &idio16gpio->reg->out0_7;
>
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> if (value)
> out_state = ioread8(base) | mask;
>@@ -130,7 +130,7 @@ static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,
>
> iowrite8(out_state, base);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
>
> static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
>@@ -140,7 +140,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
> unsigned long flags;
> unsigned int out_state;
>
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> /* process output lines 0-7 */
> if (*mask & 0xFF) {
>@@ -160,7 +160,7 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
> iowrite8(out_state, &idio16gpio->reg->out8_15);
> }
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
>
> static void idio_16_irq_ack(struct irq_data *data)
>@@ -177,11 +177,11 @@ static void idio_16_irq_mask(struct irq_data *data)
> idio16gpio->irq_mask &= ~mask;
>
> if (!idio16gpio->irq_mask) {
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> iowrite8(0, &idio16gpio->reg->irq_ctl);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
> }
>
>@@ -196,11 +196,11 @@ static void idio_16_irq_unmask(struct irq_data *data)
> idio16gpio->irq_mask |= mask;
>
> if (!prev_irq_mask) {
>- spin_lock_irqsave(&idio16gpio->lock, flags);
>+ raw_spin_lock_irqsave(&idio16gpio->lock, flags);
>
> ioread8(&idio16gpio->reg->irq_ctl);
>
>- spin_unlock_irqrestore(&idio16gpio->lock, flags);
>+ raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
> }
> }
>
>@@ -229,11 +229,11 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
> struct gpio_chip *const chip = &idio16gpio->chip;
> int gpio;
>
>- spin_lock(&idio16gpio->lock);
>+ raw_spin_lock(&idio16gpio->lock);
>
> irq_status = ioread8(&idio16gpio->reg->irq_status);
>
>- spin_unlock(&idio16gpio->lock);
>+ raw_spin_unlock(&idio16gpio->lock);
>
> /* Make sure our device generated IRQ */
> if (!(irq_status & 0x3) || !(irq_status & 0x4))
>@@ -242,12 +242,12 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
> for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
> generic_handle_irq(irq_find_mapping(chip->irqdomain, gpio));
>
>- spin_lock(&idio16gpio->lock);
>+ raw_spin_lock(&idio16gpio->lock);
>
> /* Clear interrupt */
> iowrite8(0, &idio16gpio->reg->in0_7);
>
>- spin_unlock(&idio16gpio->lock);
>+ raw_spin_unlock(&idio16gpio->lock);
>
> return IRQ_HANDLED;
> }
>@@ -302,7 +302,7 @@ static int idio_16_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> idio16gpio->chip.set = idio_16_gpio_set;
> idio16gpio->chip.set_multiple = idio_16_gpio_set_multiple;
>
>- spin_lock_init(&idio16gpio->lock);
>+ raw_spin_lock_init(&idio16gpio->lock);
>
> err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
> if (err) {
>--
>2.12.0
>

2017-03-22 16:11:41

by Julia Cartwright

Subject: Re: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

On Wed, Mar 22, 2017 at 08:44:14AM -0400, William Breathitt Gray wrote:
> On Tue, Mar 21, 2017 at 05:43:07PM -0500, Julia Cartwright wrote:
> >The 104-idi-48 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.
> >
> >Signed-off-by: Julia Cartwright <[email protected]>
>
> Hi Julia,
>
> This driver also uses a second spinlock_t, called ack_lock, to prevent
> reentrance into the idi_48_irq_handler function. Should ack_lock also be
> implemented as a raw_spinlock_t?

I saw this lock, and I don't even understand it's purpose.

However, I think I convinced myself that it's harmless. Why? It's only
ever acquired in a handler registered with request_irq(), which, on RT,
is invoked in a context which can sleep.

Thanks for taking a closer look!

Julia

2017-03-22 17:33:30

by Julia Cartwright

Subject: Re: [PATCH v2 1/9] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations

On Wed, Mar 22, 2017 at 10:54:15AM +0100, Julia Lawall wrote:
> On Tue, 21 Mar 2017, Julia Cartwright wrote:
> > On PREEMPT_RT, the spinlock_t type becomes an object which sleeps under
> > contention. The codepaths used to support scheduling (irq dispatching, arch
> > code, the scheduler, timers) therefore must make use of the
> > raw_spin_lock{,_irq,_irqsave}() variations which preserve the non-sleeping
> > spinlock behavior.
> >
> > Because the irq_chip callbacks are invoked in the process of interrupt
> > dispatch, they cannot therefore make use of spin_lock_t type. Instead, the
> > usage of raw_spinlock_t is appropriate.
> >
> > Provide a spatch to identify (and attempt to patch) such problematic irqchip
> > implementations.
> >
> > Note to those generating patches using this spatch; in order to maintain
> > correct semantics w/ PREEMPT_RT, it is necessary to audit the
> > raw_spinlock_t-protected codepaths to ensure their execution is bounded and
> > minimal. This is a manual audit process.
> >
> > See commit 47b03ca903fb0 ("pinctrl: qcom: Use raw spinlock variants") as an
> > example of _one_ such instance, which fixed a real bug seen in the field.
> >
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Sebastian Andrzej Siewior <[email protected]>
> > Cc: Linus Walleij <[email protected]>
> > Signed-off-by: Julia Cartwright <[email protected]>
>
> Acked-by: Julia Lawall <[email protected]>

Thanks, Julia.

How do these semantic patches normally land? It looks like they quite a
few have gone through the kbuild tree, is this the norm?

Thanks,
Other Julia

2017-03-22 21:46:00

by Julia Lawall

Subject: Re: [PATCH v2 1/9] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations

On Wed, 22 Mar 2017, Julia Cartwright wrote:

> On Wed, Mar 22, 2017 at 10:54:15AM +0100, Julia Lawall wrote:
> > On Tue, 21 Mar 2017, Julia Cartwright wrote:
> > > On PREEMPT_RT, the spinlock_t type becomes an object which sleeps under
> > > contention. The codepaths used to support scheduling (irq dispatching, arch
> > > code, the scheduler, timers) therefore must make use of the
> > > raw_spin_lock{,_irq,_irqsave}() variations which preserve the non-sleeping
> > > spinlock behavior.
> > >
> > > Because the irq_chip callbacks are invoked in the process of interrupt
> > > dispatch, they cannot therefore make use of spin_lock_t type. Instead, the
> > > usage of raw_spinlock_t is appropriate.
> > >
> > > Provide a spatch to identify (and attempt to patch) such problematic irqchip
> > > implementations.
> > >
> > > Note to those generating patches using this spatch; in order to maintain
> > > correct semantics w/ PREEMPT_RT, it is necessary to audit the
> > > raw_spinlock_t-protected codepaths to ensure their execution is bounded and
> > > minimal. This is a manual audit process.
> > >
> > > See commit 47b03ca903fb0 ("pinctrl: qcom: Use raw spinlock variants") as an
> > > example of _one_ such instance, which fixed a real bug seen in the field.
> > >
> > > Cc: Thomas Gleixner <[email protected]>
> > > Cc: Sebastian Andrzej Siewior <[email protected]>
> > > Cc: Linus Walleij <[email protected]>
> > > Signed-off-by: Julia Cartwright <[email protected]>
> >
> > Acked-by: Julia Lawall <[email protected]>
>
> Thanks, Julia.
>
> How do these semantic patches normally land? It looks like they quite a
> few have gone through the kbuild tree, is this the norm?

Michal Marek takes care of them.

julia

>
> Thanks,
> Other Julia
>

2017-03-23 13:42:54

by Lee Jones

Subject: Re: [PATCH v2 4/9] mfd: asic3: make use of raw_spinlock variants

On Tue, 21 Mar 2017, Julia Cartwright wrote:

> The asic3 mfd 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.
>
> Acked-for-MFD-by: Lee Jones <[email protected]>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> v1 -> v2:
> - No functional change. Added Lee's ack.
>
> drivers/mfd/asic3.c | 56 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 28 insertions(+), 28 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
> index 0413c8159551..cf2e25ab2940 100644
> --- a/drivers/mfd/asic3.c
> +++ b/drivers/mfd/asic3.c
> @@ -78,7 +78,7 @@ struct asic3 {
> unsigned int bus_shift;
> unsigned int irq_nr;
> unsigned int irq_base;
> - spinlock_t lock;
> + raw_spinlock_t lock;
> u16 irq_bothedge[4];
> struct gpio_chip gpio;
> struct device *dev;
> @@ -108,14 +108,14 @@ static void asic3_set_register(struct asic3 *asic, u32 reg, u32 bits, bool set)
> unsigned long flags;
> u32 val;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> val = asic3_read_register(asic, reg);
> if (set)
> val |= bits;
> else
> val &= ~bits;
> asic3_write_register(asic, reg, val);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> /* IRQs */
> @@ -129,13 +129,13 @@ static void asic3_irq_flip_edge(struct asic3 *asic,
> u16 edge;
> unsigned long flags;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> edge = asic3_read_register(asic,
> base + ASIC3_GPIO_EDGE_TRIGGER);
> edge ^= bit;
> asic3_write_register(asic,
> base + ASIC3_GPIO_EDGE_TRIGGER, edge);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static void asic3_irq_demux(struct irq_desc *desc)
> @@ -151,10 +151,10 @@ static void asic3_irq_demux(struct irq_desc *desc)
> u32 status;
> int bank;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> status = asic3_read_register(asic,
> ASIC3_OFFSET(INTR, P_INT_STAT));
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
>
> /* Check all ten register bits */
> if ((status & 0x3ff) == 0)
> @@ -167,7 +167,7 @@ static void asic3_irq_demux(struct irq_desc *desc)
>
> base = ASIC3_GPIO_A_BASE
> + bank * ASIC3_GPIO_BASE_INCR;
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> istat = asic3_read_register(asic,
> base +
> ASIC3_GPIO_INT_STATUS);
> @@ -175,7 +175,7 @@ static void asic3_irq_demux(struct irq_desc *desc)
> asic3_write_register(asic,
> base +
> ASIC3_GPIO_INT_STATUS, 0);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
>
> for (i = 0; i < ASIC3_GPIOS_PER_BANK; i++) {
> int bit = (1 << i);
> @@ -230,11 +230,11 @@ static void asic3_mask_gpio_irq(struct irq_data *data)
> bank = asic3_irq_to_bank(asic, data->irq);
> index = asic3_irq_to_index(asic, data->irq);
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> val = asic3_read_register(asic, bank + ASIC3_GPIO_MASK);
> val |= 1 << index;
> asic3_write_register(asic, bank + ASIC3_GPIO_MASK, val);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static void asic3_mask_irq(struct irq_data *data)
> @@ -243,7 +243,7 @@ static void asic3_mask_irq(struct irq_data *data)
> int regval;
> unsigned long flags;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> regval = asic3_read_register(asic,
> ASIC3_INTR_BASE +
> ASIC3_INTR_INT_MASK);
> @@ -255,7 +255,7 @@ static void asic3_mask_irq(struct irq_data *data)
> ASIC3_INTR_BASE +
> ASIC3_INTR_INT_MASK,
> regval);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static void asic3_unmask_gpio_irq(struct irq_data *data)
> @@ -267,11 +267,11 @@ static void asic3_unmask_gpio_irq(struct irq_data *data)
> bank = asic3_irq_to_bank(asic, data->irq);
> index = asic3_irq_to_index(asic, data->irq);
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> val = asic3_read_register(asic, bank + ASIC3_GPIO_MASK);
> val &= ~(1 << index);
> asic3_write_register(asic, bank + ASIC3_GPIO_MASK, val);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static void asic3_unmask_irq(struct irq_data *data)
> @@ -280,7 +280,7 @@ static void asic3_unmask_irq(struct irq_data *data)
> int regval;
> unsigned long flags;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> regval = asic3_read_register(asic,
> ASIC3_INTR_BASE +
> ASIC3_INTR_INT_MASK);
> @@ -292,7 +292,7 @@ static void asic3_unmask_irq(struct irq_data *data)
> ASIC3_INTR_BASE +
> ASIC3_INTR_INT_MASK,
> regval);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
> @@ -306,7 +306,7 @@ static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
> index = asic3_irq_to_index(asic, data->irq);
> bit = 1<<index;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> level = asic3_read_register(asic,
> bank + ASIC3_GPIO_LEVEL_TRIGGER);
> edge = asic3_read_register(asic,
> @@ -348,7 +348,7 @@ static int asic3_gpio_irq_type(struct irq_data *data, unsigned int type)
> edge);
> asic3_write_register(asic, bank + ASIC3_GPIO_TRIGGER_TYPE,
> trigger);
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> return 0;
> }
>
> @@ -455,7 +455,7 @@ static int asic3_gpio_direction(struct gpio_chip *chip,
> return -EINVAL;
> }
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
>
> out_reg = asic3_read_register(asic, gpio_base + ASIC3_GPIO_DIRECTION);
>
> @@ -467,7 +467,7 @@ static int asic3_gpio_direction(struct gpio_chip *chip,
>
> asic3_write_register(asic, gpio_base + ASIC3_GPIO_DIRECTION, out_reg);
>
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
>
> return 0;
>
> @@ -524,7 +524,7 @@ static void asic3_gpio_set(struct gpio_chip *chip,
>
> mask = ASIC3_GPIO_TO_MASK(offset);
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
>
> out_reg = asic3_read_register(asic, gpio_base + ASIC3_GPIO_OUT);
>
> @@ -535,7 +535,7 @@ static void asic3_gpio_set(struct gpio_chip *chip,
>
> asic3_write_register(asic, gpio_base + ASIC3_GPIO_OUT, out_reg);
>
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static int asic3_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> @@ -611,13 +611,13 @@ static void asic3_clk_enable(struct asic3 *asic, struct asic3_clk *clk)
> unsigned long flags;
> u32 cdex;
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> if (clk->enabled++ == 0) {
> cdex = asic3_read_register(asic, ASIC3_OFFSET(CLOCK, CDEX));
> cdex |= clk->cdex;
> asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
> }
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
> @@ -627,13 +627,13 @@ static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
>
> WARN_ON(clk->enabled == 0);
>
> - spin_lock_irqsave(&asic->lock, flags);
> + raw_spin_lock_irqsave(&asic->lock, flags);
> if (--clk->enabled == 0) {
> cdex = asic3_read_register(asic, ASIC3_OFFSET(CLOCK, CDEX));
> cdex &= ~clk->cdex;
> asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
> }
> - spin_unlock_irqrestore(&asic->lock, flags);
> + raw_spin_unlock_irqrestore(&asic->lock, flags);
> }
>
> /* MFD cells (SPI, PWM, LED, DS1WM, MMC) */
> @@ -963,7 +963,7 @@ static int __init asic3_probe(struct platform_device *pdev)
> if (!asic)
> return -ENOMEM;
>
> - spin_lock_init(&asic->lock);
> + raw_spin_lock_init(&asic->lock);
> platform_set_drvdata(pdev, asic);
> asic->dev = &pdev->dev;
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2017-03-23 13:43:52

by Lee Jones

Subject: Re: [PATCH v2 6/9] mfd: tc6393xb: make use of raw_spinlock variants

On Tue, 21 Mar 2017, Julia Cartwright wrote:

> The tc6393xb mfd 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.
>
> Acked-for-MFD-by: Lee Jones <[email protected]>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> v1 -> v2:
> - No functional change. Added Lee's ack.
>
> drivers/mfd/tc6393xb.c | 52 +++++++++++++++++++++++++-------------------------
> 1 file changed, 26 insertions(+), 26 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/tc6393xb.c b/drivers/mfd/tc6393xb.c
> index d42d322ac7ca..d16e71bd9482 100644
> --- a/drivers/mfd/tc6393xb.c
> +++ b/drivers/mfd/tc6393xb.c
> @@ -95,7 +95,7 @@ struct tc6393xb {
>
> struct clk *clk; /* 3,6 Mhz */
>
> - spinlock_t lock; /* protects RMW cycles */
> + raw_spinlock_t lock; /* protects RMW cycles */
>
> struct {
> u8 fer;
> @@ -126,13 +126,13 @@ static int tc6393xb_nand_enable(struct platform_device *nand)
> struct tc6393xb *tc6393xb = platform_get_drvdata(dev);
> unsigned long flags;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> /* SMD buffer on */
> dev_dbg(&dev->dev, "SMD buffer on\n");
> tmio_iowrite8(0xff, tc6393xb->scr + SCR_GPI_BCR(1));
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -226,7 +226,7 @@ static int tc6393xb_ohci_enable(struct platform_device *dev)
> u16 ccr;
> u8 fer;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
> ccr |= SCR_CCR_USBCK;
> @@ -236,7 +236,7 @@ static int tc6393xb_ohci_enable(struct platform_device *dev)
> fer |= SCR_FER_USBEN;
> tmio_iowrite8(fer, tc6393xb->scr + SCR_FER);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -248,7 +248,7 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
> u16 ccr;
> u8 fer;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> fer = tmio_ioread8(tc6393xb->scr + SCR_FER);
> fer &= ~SCR_FER_USBEN;
> @@ -258,7 +258,7 @@ static int tc6393xb_ohci_disable(struct platform_device *dev)
> ccr &= ~SCR_CCR_USBCK;
> tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -280,14 +280,14 @@ static int tc6393xb_fb_enable(struct platform_device *dev)
> unsigned long flags;
> u16 ccr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
> ccr &= ~SCR_CCR_MCLK_MASK;
> ccr |= SCR_CCR_MCLK_48;
> tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -298,14 +298,14 @@ static int tc6393xb_fb_disable(struct platform_device *dev)
> unsigned long flags;
> u16 ccr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> ccr = tmio_ioread16(tc6393xb->scr + SCR_CCR);
> ccr &= ~SCR_CCR_MCLK_MASK;
> ccr |= SCR_CCR_MCLK_OFF;
> tmio_iowrite16(ccr, tc6393xb->scr + SCR_CCR);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -317,7 +317,7 @@ int tc6393xb_lcd_set_power(struct platform_device *fb, bool on)
> u8 fer;
> unsigned long flags;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> fer = ioread8(tc6393xb->scr + SCR_FER);
> if (on)
> @@ -326,7 +326,7 @@ int tc6393xb_lcd_set_power(struct platform_device *fb, bool on)
> fer &= ~SCR_FER_SLCDEN;
> iowrite8(fer, tc6393xb->scr + SCR_FER);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -338,12 +338,12 @@ int tc6393xb_lcd_mode(struct platform_device *fb,
> struct tc6393xb *tc6393xb = platform_get_drvdata(dev);
> unsigned long flags;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> iowrite16(mode->pixclock, tc6393xb->scr + SCR_PLL1CR + 0);
> iowrite16(mode->pixclock >> 16, tc6393xb->scr + SCR_PLL1CR + 2);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -462,11 +462,11 @@ static void tc6393xb_gpio_set(struct gpio_chip *chip,
> struct tc6393xb *tc6393xb = gpiochip_get_data(chip);
> unsigned long flags;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> __tc6393xb_gpio_set(chip, offset, value);
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
> }
>
> static int tc6393xb_gpio_direction_input(struct gpio_chip *chip,
> @@ -476,13 +476,13 @@ static int tc6393xb_gpio_direction_input(struct gpio_chip *chip,
> unsigned long flags;
> u8 doecr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> doecr = tmio_ioread8(tc6393xb->scr + SCR_GPO_DOECR(offset / 8));
> doecr &= ~TC_GPIO_BIT(offset);
> tmio_iowrite8(doecr, tc6393xb->scr + SCR_GPO_DOECR(offset / 8));
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -494,7 +494,7 @@ static int tc6393xb_gpio_direction_output(struct gpio_chip *chip,
> unsigned long flags;
> u8 doecr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
>
> __tc6393xb_gpio_set(chip, offset, value);
>
> @@ -502,7 +502,7 @@ static int tc6393xb_gpio_direction_output(struct gpio_chip *chip,
> doecr |= TC_GPIO_BIT(offset);
> tmio_iowrite8(doecr, tc6393xb->scr + SCR_GPO_DOECR(offset / 8));
>
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
>
> return 0;
> }
> @@ -548,11 +548,11 @@ static void tc6393xb_irq_mask(struct irq_data *data)
> unsigned long flags;
> u8 imr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
> imr = tmio_ioread8(tc6393xb->scr + SCR_IMR);
> imr |= 1 << (data->irq - tc6393xb->irq_base);
> tmio_iowrite8(imr, tc6393xb->scr + SCR_IMR);
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
> }
>
> static void tc6393xb_irq_unmask(struct irq_data *data)
> @@ -561,11 +561,11 @@ static void tc6393xb_irq_unmask(struct irq_data *data)
> unsigned long flags;
> u8 imr;
>
> - spin_lock_irqsave(&tc6393xb->lock, flags);
> + raw_spin_lock_irqsave(&tc6393xb->lock, flags);
> imr = tmio_ioread8(tc6393xb->scr + SCR_IMR);
> imr &= ~(1 << (data->irq - tc6393xb->irq_base));
> tmio_iowrite8(imr, tc6393xb->scr + SCR_IMR);
> - spin_unlock_irqrestore(&tc6393xb->lock, flags);
> + raw_spin_unlock_irqrestore(&tc6393xb->lock, flags);
> }
>
> static struct irq_chip tc6393xb_chip = {
> @@ -628,7 +628,7 @@ static int tc6393xb_probe(struct platform_device *dev)
> goto err_kzalloc;
> }
>
> - spin_lock_init(&tc6393xb->lock);
> + raw_spin_lock_init(&tc6393xb->lock);
>
> platform_set_drvdata(dev, tc6393xb);
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2017-03-23 13:43:57

by Lee Jones

Subject: Re: [PATCH v2 5/9] mfd: t7l66xb: make use of raw_spinlock variants

On Tue, 21 Mar 2017, Julia Cartwright wrote:

> The t7l66xb mfd 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.
>
> Acked-for-MFD-by: Lee Jones <[email protected]>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> v1 -> v2:
> - No functional change. Added Lee's ack.
>
> drivers/mfd/t7l66xb.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/t7l66xb.c b/drivers/mfd/t7l66xb.c
> index 94bd89cb1f06..22c811396edc 100644
> --- a/drivers/mfd/t7l66xb.c
> +++ b/drivers/mfd/t7l66xb.c
> @@ -69,7 +69,7 @@ static const struct resource t7l66xb_mmc_resources[] = {
> struct t7l66xb {
> void __iomem *scr;
> /* Lock to protect registers requiring read/modify/write ops. */
> - spinlock_t lock;
> + raw_spinlock_t lock;
>
> struct resource rscr;
> struct clk *clk48m;
> @@ -89,13 +89,13 @@ static int t7l66xb_mmc_enable(struct platform_device *mmc)
>
> clk_prepare_enable(t7l66xb->clk32k);
>
> - spin_lock_irqsave(&t7l66xb->lock, flags);
> + raw_spin_lock_irqsave(&t7l66xb->lock, flags);
>
> dev_ctl = tmio_ioread8(t7l66xb->scr + SCR_DEV_CTL);
> dev_ctl |= SCR_DEV_CTL_MMC;
> tmio_iowrite8(dev_ctl, t7l66xb->scr + SCR_DEV_CTL);
>
> - spin_unlock_irqrestore(&t7l66xb->lock, flags);
> + raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
>
> tmio_core_mmc_enable(t7l66xb->scr + 0x200, 0,
> t7l66xb_mmc_resources[0].start & 0xfffe);
> @@ -110,13 +110,13 @@ static int t7l66xb_mmc_disable(struct platform_device *mmc)
> unsigned long flags;
> u8 dev_ctl;
>
> - spin_lock_irqsave(&t7l66xb->lock, flags);
> + raw_spin_lock_irqsave(&t7l66xb->lock, flags);
>
> dev_ctl = tmio_ioread8(t7l66xb->scr + SCR_DEV_CTL);
> dev_ctl &= ~SCR_DEV_CTL_MMC;
> tmio_iowrite8(dev_ctl, t7l66xb->scr + SCR_DEV_CTL);
>
> - spin_unlock_irqrestore(&t7l66xb->lock, flags);
> + raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
>
> clk_disable_unprepare(t7l66xb->clk32k);
>
> @@ -206,11 +206,11 @@ static void t7l66xb_irq_mask(struct irq_data *data)
> unsigned long flags;
> u8 imr;
>
> - spin_lock_irqsave(&t7l66xb->lock, flags);
> + raw_spin_lock_irqsave(&t7l66xb->lock, flags);
> imr = tmio_ioread8(t7l66xb->scr + SCR_IMR);
> imr |= 1 << (data->irq - t7l66xb->irq_base);
> tmio_iowrite8(imr, t7l66xb->scr + SCR_IMR);
> - spin_unlock_irqrestore(&t7l66xb->lock, flags);
> + raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
> }
>
> static void t7l66xb_irq_unmask(struct irq_data *data)
> @@ -219,11 +219,11 @@ static void t7l66xb_irq_unmask(struct irq_data *data)
> unsigned long flags;
> u8 imr;
>
> - spin_lock_irqsave(&t7l66xb->lock, flags);
> + raw_spin_lock_irqsave(&t7l66xb->lock, flags);
> imr = tmio_ioread8(t7l66xb->scr + SCR_IMR);
> imr &= ~(1 << (data->irq - t7l66xb->irq_base));
> tmio_iowrite8(imr, t7l66xb->scr + SCR_IMR);
> - spin_unlock_irqrestore(&t7l66xb->lock, flags);
> + raw_spin_unlock_irqrestore(&t7l66xb->lock, flags);
> }
>
> static struct irq_chip t7l66xb_chip = {
> @@ -321,7 +321,7 @@ static int t7l66xb_probe(struct platform_device *dev)
> if (!t7l66xb)
> return -ENOMEM;
>
> - spin_lock_init(&t7l66xb->lock);
> + raw_spin_lock_init(&t7l66xb->lock);
>
> platform_set_drvdata(dev, t7l66xb);
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2017-03-28 09:12:03

by Linus Walleij

Subject: Re: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

On Wed, Mar 22, 2017 at 1:44 PM, William Breathitt Gray
<[email protected]> wrote:
> On Tue, Mar 21, 2017 at 05:43:07PM -0500, Julia Cartwright wrote:
>>The 104-idi-48 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.
>>
>>Signed-off-by: Julia Cartwright <[email protected]>
>
> Hi Julia,
>
> This driver also uses a second spinlock_t, called ack_lock, to prevent
> reentrance into the idi_48_irq_handler function. Should ack_lock also be
> implemented as a raw_spinlock_t?

Hm, can I apply this one patch or not?

Linus Walleij

2017-03-28 09:13:32

by Linus Walleij

Subject: Re: [PATCH v2 8/9] gpio: 104-idio-16: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 11:43 PM, Julia Cartwright <[email protected]> wrote:

> The 104-idio-16 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.
>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> New patch as of v2 of series.

Patch applied to the GPIO tree with William's ACK.

Yours,
Linus Walleij

2017-03-28 09:22:46

by Linus Walleij

Subject: Re: [PATCH v2 9/9] gpio: pci-idio-16: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 11:43 PM, Julia Cartwright <[email protected]> wrote:

> The pci-idio-16 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.
>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> New patch as of v2 of series.

Patch applied with William's ACK.

Yours,
Linus Walleij

2017-03-28 11:40:39

by William Breathitt Gray

Subject: Re: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

On Tue, Mar 28, 2017 at 11:11:59AM +0200, Linus Walleij wrote:
>On Wed, Mar 22, 2017 at 1:44 PM, William Breathitt Gray
><[email protected]> wrote:
>> On Tue, Mar 21, 2017 at 05:43:07PM -0500, Julia Cartwright wrote:
>>>The 104-idi-48 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.
>>>
>>>Signed-off-by: Julia Cartwright <[email protected]>
>>
>> Hi Julia,
>>
>> This driver also uses a second spinlock_t, called ack_lock, to prevent
>> reentrance into the idi_48_irq_handler function. Should ack_lock also be
>> implemented as a raw_spinlock_t?
>
>Hm, can I apply this one patch or not?
>
>Linus Walleij

Oops, sorry for missing this reply. Julia is correct that ack_lock does
not need to be implemented as raw_spinlock_t. For reference, ack_lock is
used to prevent a race condition on the device hardware itself related
to how the 104-IDI-48 acknowledges IRQ (check out the commit description
for it for a more in-depth explanation if you're curious).

Long story short: Julia's patch is prefectly acceptable as is.

Acked-by: William Breathitt Gray <[email protected]>

William Breathitt Gray

2017-03-28 13:02:51

by Linus Walleij

Subject: Re: [PATCH v2 7/9] gpio: 104-idi-48: make use of raw_spinlock variants

On Tue, Mar 21, 2017 at 11:43 PM, Julia Cartwright <[email protected]> wrote:

> The 104-idi-48 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.
>
> Signed-off-by: Julia Cartwright <[email protected]>
> ---
> New patch as of v2 of series.

Patch applied with William's ACK.

Yours,
Linus Walleij

2018-01-29 04:17:19

by Michael Ellerman

Subject: Re: [v2,3/9] powerpc: mpc52xx_gpt: make use of raw_spinlock variants

On Tue, 2017-03-21 at 22:43:03 UTC, Julia Cartwright wrote:
> The mpc52xx_gpt code 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.
>
> Signed-off-by: Julia Cartwright <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/77720c82915a8b7797e0041af95707

cheers