From: Marek Szyprowski <m.szyprowski@samsung•com>
To: Thomas Gleixner <tglx@kernel•org>,
linux-arm-kernel@lists•infradead.org,
linux-samsung-soc@vger•kernel.org,
linux-rt-devel@lists•linux.dev
Cc: Krzysztof Kozlowski <krzk@kernel•org>,
Alim Akhtar <alim.akhtar@samsung•com>,
Sebastian Andrzej Siewior <bigeasy@linutronix•de>,
Clark Williams <clrkwllms@kernel•org>,
Steven Rostedt <rostedt@goodmis•org>
Subject: Re: [PATCH] irqchip/exynos-combiner: switch to raw_spinlock
Date: Thu, 21 May 2026 13:26:01 +0200 [thread overview]
Message-ID: <97f33a9d-5fdd-4766-aaff-d10d5f5fdf28@samsung.com> (raw)
In-Reply-To: <87ecj5w2qf.ffs@tglx>
On 21.05.2026 11:06, Thomas Gleixner wrote:
> On Thu, May 21 2026 at 00:04, Marek Szyprowski wrote:
>> The exynos-combiner driver uses a regular spinlock to protect access to
>> the combiner interrupt status register in combiner_handle_cascade_irq(),
>> which is invoked in hard IRQ context as a chained interrupt handler.
>>
>> When PREEMPT_RT is enabled on ARM, regular spinlock is converted to a
>> sleeping lock (mutex-based), which must not be used in atomic context
>> such as hard interrupt handlers. Switch the irq_controller_lock to
>> raw_spinlock, which remains a true non-sleeping spinlock even under
>> PREEMPT_RT.
> Mechanically this makes sense, but out of curiosity I have to ask:
>
>> -static DEFINE_SPINLOCK(irq_controller_lock);
>> +static DEFINE_RAW_SPINLOCK(irq_controller_lock);
>>
>> struct combiner_chip_data {
>> unsigned int hwirq_offset;
>> @@ -72,9 +72,9 @@ static void combiner_handle_cascade_irq(struct irq_desc *desc)
>>
>> chained_irq_enter(chip, desc);
>>
>> - spin_lock(&irq_controller_lock);
>> + raw_spin_lock(&irq_controller_lock);
>> status = readl_relaxed(chip_data->base + COMBINER_INT_STATUS);
>> - spin_unlock(&irq_controller_lock);
>> + raw_spin_unlock(&irq_controller_lock);
> What is this lock actually protecting?
>
> Each combiner has it's own @base address, so there is no concurrency
> problem between two cascade interrupts being handled at the same time.
>
> That means the only possible problem would be that the same cascade
> interrupt is handled on two CPUs concurrently. Is that even possible?
Frankly speaking I did this conversion mechanically, late in the evening to fix
the bug warning I've spotted. Indeed this spinlock looks like a copy&paste or
development leftover. The only side-effect of it I see is a memory barrier,
which might affect how the register access happens, but this should not affect
cascade operation imho. Do You want me to send a patch removing it completely?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
next prev parent reply other threads:[~2026-05-21 11:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20260520220432eucas1p10502ca0f9368bd6de5ce027ad8170109@eucas1p1.samsung.com>
2026-05-20 22:04 ` [PATCH] irqchip/exynos-combiner: switch to raw_spinlock Marek Szyprowski
2026-05-21 9:04 ` Sebastian Andrzej Siewior
2026-05-21 9:06 ` Thomas Gleixner
2026-05-21 11:26 ` Marek Szyprowski [this message]
2026-05-21 12:31 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=97f33a9d-5fdd-4766-aaff-d10d5f5fdf28@samsung.com \
--to=m.szyprowski@samsung$(echo .)com \
--cc=alim.akhtar@samsung$(echo .)com \
--cc=bigeasy@linutronix$(echo .)de \
--cc=clrkwllms@kernel$(echo .)org \
--cc=krzk@kernel$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-rt-devel@lists$(echo .)linux.dev \
--cc=linux-samsung-soc@vger$(echo .)kernel.org \
--cc=rostedt@goodmis$(echo .)org \
--cc=tglx@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox