Messages in this thread | | | Subject | Re: [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support | From | Oleksij Rempel <> | Date | Sun, 20 Sep 2015 15:47:59 +0200 |
| |
Am 20.09.2015 um 13:28 schrieb Marc Zyngier: > On Sat, 19 Sep 2015 07:53:34 +0200 > Oleksij Rempel <linux@rempel-privat.de> wrote: > >> Am 18.09.2015 um 12:42 schrieb Marc Zyngier: >>> On Fri, 18 Sep 2015 11:18:42 +0200 >>> Oleksij Rempel <linux@rempel-privat.de> wrote: >> > > [...] > >>>> +static void asm9260_mask_irq(struct irq_data *d) >>>> +{ >>>> + raw_spin_lock(&icoll_lock); >>>> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE), >>>> + icoll_intr_reg(d) + CLR_REG); >>>> + raw_spin_unlock(&icoll_lock); >>>> +} >>>> + >>>> +static void asm9260_unmask_irq(struct irq_data *d) >>>> +{ >>>> + raw_spin_lock(&icoll_lock); >>>> + __raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq), >>>> + icoll_priv.clear + >>>> + ASM9260_HW_ICOLL_CLEARn(d->hwirq)); >>>> + >>>> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE), >>>> + icoll_intr_reg(d) + SET_REG); >>>> + raw_spin_unlock(&icoll_lock); >>>> +} >>> >>> Can you please explain the rational for this lock? mask/unmask use >>> different registers, and it is not obvious to me what race you are >>> trying to avoid here. >> >> Uff... in one of earliest reviews i was asked to add lock.. >> I also was asked to add asm9260 to some existing driver. Not sure if it >> is still making sense. > > Adding or removing a lock is not about what people ask you to do. It is > about requirements dictated by either the HW (you need to perform > a given sequence atomically with respect to another code sequence), the > kernel, or both. So I'd like to understand what is the underlying > reason for this lock. It is not disabling interrupts, so it could end > up being called in an interrupt context -> deadlock.
You right.
> So either the HW requires it and you have the wrong accessors, or it > doesn't, and you can remove it. Either way, we need to know. > > M.
I don't see any need to lock it.
-- Regards, Oleksij
[unhandled content-type:application/pgp-signature] | |