mbox series

[v4,0/7] Provide and use generic_handle_irq_safe() where appropriate.

Message ID 20220211181500.1856198-1-bigeasy@linutronix.de
Headers show
Series Provide and use generic_handle_irq_safe() where appropriate. | expand

Message

Sebastian Andrzej Siewior Feb. 11, 2022, 6:14 p.m. UTC
handler/ interrupt controller entry). It is low level code and the
function expects that interrupts are disabled at entry point.

This isn't the case for invocations from tasklets, workqueues or the
primary interrupt handler on PREEMPT_RT. Once this gets noticed a
"local_irq_disable|safe()" is added. To avoid further confusion this
series adds generic_handle_irq_safe() which can be used from any context
and adds a few user.

v2…v4:
  - Correct kernel doc for generic_handle_irq_safe() as per Wolfram Sang.
  - Use "misc" instead of "mfd" for the hi6421-spmi-pmic driver.

v2…v1:
 https://lore.kernel.org/all/20220131123404.175438-1-bigeasy@linutronix.de/
 - Redo kernel-doc for generic_handle_irq_safe() in #1.
 - Use generic_handle_irq_safe() instead of generic_handle_irq() in the
   patch description where I accidently used the wrong one.
v1:
 https://lore.kernel.org/all/20220127113303.3012207-1-bigeasy@linutronix.de/

Comments

Lee Jones Feb. 15, 2022, 2:36 p.m. UTC | #1
On Fri, 11 Feb 2022, Sebastian Andrzej Siewior wrote:

> handler/ interrupt controller entry). It is low level code and the
> function expects that interrupts are disabled at entry point.
> 
> This isn't the case for invocations from tasklets, workqueues or the
> primary interrupt handler on PREEMPT_RT. Once this gets noticed a
> "local_irq_disable|safe()" is added. To avoid further confusion this
> series adds generic_handle_irq_safe() which can be used from any context
> and adds a few user.
> 
> v2…v4:
>   - Correct kernel doc for generic_handle_irq_safe() as per Wolfram Sang.
>   - Use "misc" instead of "mfd" for the hi6421-spmi-pmic driver.
> 
> v2…v1:
>  https://lore.kernel.org/all/20220131123404.175438-1-bigeasy@linutronix.de/
>  - Redo kernel-doc for generic_handle_irq_safe() in #1.
>  - Use generic_handle_irq_safe() instead of generic_handle_irq() in the
>    patch description where I accidently used the wrong one.
> v1:
>  https://lore.kernel.org/all/20220127113303.3012207-1-bigeasy@linutronix.de/

Please use the official cover-letter format (--cover-letter).

It would have been nice to at least find a diff stat here.

...

Do we really need to coordinate this series cross-subsystem?

Can we first apply the API, then have each of the subsystems adapted
separately?  Does the change-over all need to happen concurrently?

If the latter is the case, is this set bisectable?
Sebastian Andrzej Siewior Feb. 15, 2022, 2:50 p.m. UTC | #2
On 2022-02-15 14:36:01 [+0000], Lee Jones wrote:
> Do we really need to coordinate this series cross-subsystem?

I would suggest to merge it via irq subsystem but I leave the logistics
to tglx.

Sebastian
Lee Jones Feb. 15, 2022, 3:16 p.m. UTC | #3
On Tue, 15 Feb 2022, Sebastian Andrzej Siewior wrote:

> On 2022-02-15 14:36:01 [+0000], Lee Jones wrote:
> > Do we really need to coordinate this series cross-subsystem?
> 
> I would suggest to merge it via irq subsystem but I leave the logistics
> to tglx.

Could you answer by other questions too please?
Sebastian Andrzej Siewior Feb. 15, 2022, 3:33 p.m. UTC | #4
On 2022-02-15 15:16:36 [+0000], Lee Jones wrote:
> On Tue, 15 Feb 2022, Sebastian Andrzej Siewior wrote:
> 
> > On 2022-02-15 14:36:01 [+0000], Lee Jones wrote:
> > > Do we really need to coordinate this series cross-subsystem?
> > 
> > I would suggest to merge it via irq subsystem but I leave the logistics
> > to tglx.
> 
> Could you answer by other questions too please?

I don't think that I can answer them. I said I leave the logistics to
tglx.

This can go via one merge via irq. This can also go differently i.e.
feature branch on top of 5.17-rc1 (with 1/7) which is merge into each
subsystem and then the "feature" on top.

Either way it remains bisect-able since each driver is changed
individually. There is no need to merge them in one go but since it is
that small it probably makes sense. But I don't do the logistics here.

Sebastian
Lee Jones Feb. 15, 2022, 3:42 p.m. UTC | #5
On Tue, 15 Feb 2022, Sebastian Andrzej Siewior wrote:

> On 2022-02-15 15:16:36 [+0000], Lee Jones wrote:
> > On Tue, 15 Feb 2022, Sebastian Andrzej Siewior wrote:
> > 
> > > On 2022-02-15 14:36:01 [+0000], Lee Jones wrote:
> > > > Do we really need to coordinate this series cross-subsystem?
> > > 
> > > I would suggest to merge it via irq subsystem but I leave the logistics
> > > to tglx.
> > 
> > Could you answer by other questions too please?
> 
> I don't think that I can answer them. I said I leave the logistics to
> tglx.
> 
> This can go via one merge via irq. This can also go differently i.e.
> feature branch on top of 5.17-rc1 (with 1/7) which is merge into each
> subsystem and then the "feature" on top.

Apologies for the confusion.

I'm not asking you about merge strategies.

We can handle that without issue.

> Either way it remains bisect-able since each driver is changed
> individually. There is no need to merge them in one go but since it is
> that small it probably makes sense. But I don't do the logistics here.

Okay, this is what I was asking.

So there aren't any hard dependencies between the driver changes?

Only the drivers are dependent on the API.

So, if we choose to do so, we can merge the API and then subsequently
add the users one by one into their respective subsystem, in any
order.  This would save on creating an immutable topic branch which we
all pull from.

What is your preference Thomas?
Sebastian Andrzej Siewior Feb. 15, 2022, 3:46 p.m. UTC | #6
On 2022-02-15 15:42:13 [+0000], Lee Jones wrote:
> So there aren't any hard dependencies between the driver changes?

Correct. #2 - #7 depend on #1. The order of #2+ is not relevant.

Sebastian
Thomas Gleixner Feb. 21, 2022, 9:57 a.m. UTC | #7
Lee,

On Tue, Feb 15 2022 at 15:42, Lee Jones wrote:
> On Tue, 15 Feb 2022, Sebastian Andrzej Siewior wrote:
>> Either way it remains bisect-able since each driver is changed
>> individually. There is no need to merge them in one go but since it is
>> that small it probably makes sense. But I don't do the logistics here.
>
> Okay, this is what I was asking.
>
> So there aren't any hard dependencies between the driver changes?
>
> Only the drivers are dependent on the API.

Correct.

> So, if we choose to do so, we can merge the API and then subsequently
> add the users one by one into their respective subsystem, in any
> order.  This would save on creating an immutable topic branch which we
> all pull from.
>
> What is your preference Thomas?

I suggest doing it the following way:

 1) I apply 1/7 on top of -rc5 and tag it

 2) Driver maintainers who want to merge via their trees pull that tag
    apply the relevant driver changes

 3) I collect the leftovers and merge them via irq/core

Does that make sense?

Thanks,

        tglx
Thomas Gleixner Feb. 21, 2022, 11:33 a.m. UTC | #8
Lee & al!

On Mon, Feb 21 2022 at 10:57, Thomas Gleixner wrote:
> On Tue, Feb 15 2022 at 15:42, Lee Jones wrote:
>> What is your preference Thomas?
>
> I suggest doing it the following way:
>
>  1) I apply 1/7 on top of -rc5 and tag it

That's what I did now. The tag to pull from is:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-api-2022-02-21

>  2) Driver maintainers who want to merge via their trees pull that tag
>     apply the relevant driver changes
>
>  3) I collect the leftovers and merge them via irq/core

So everyone who wants to merge the relevant driver changes, please pull
and let me know which driver patch(es) you merged. I'll pick up the
leftovers after -rc6.

Thanks,

        tglx
Lee Jones Feb. 21, 2022, 12:38 p.m. UTC | #9
On Mon, 21 Feb 2022, Thomas Gleixner wrote:

> Lee & al!
> 
> On Mon, Feb 21 2022 at 10:57, Thomas Gleixner wrote:
> > On Tue, Feb 15 2022 at 15:42, Lee Jones wrote:
> >> What is your preference Thomas?
> >
> > I suggest doing it the following way:
> >
> >  1) I apply 1/7 on top of -rc5 and tag it
> 
> That's what I did now. The tag to pull from is:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-api-2022-02-21
> 
> >  2) Driver maintainers who want to merge via their trees pull that tag
> >     apply the relevant driver changes
> >
> >  3) I collect the leftovers and merge them via irq/core
> 
> So everyone who wants to merge the relevant driver changes, please pull
> and let me know which driver patch(es) you merged. I'll pick up the
> leftovers after -rc6.

Ideal.  Thanks Thomas.
Wolfram Sang Feb. 23, 2022, 1:19 p.m. UTC | #10
> That's what I did now. The tag to pull from is:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-api-2022-02-21

Thanks! Pulled into i2c/for-mergewindow.
Wolfram Sang Feb. 23, 2022, 1:21 p.m. UTC | #11
On Fri, Feb 11, 2022 at 07:14:56PM +0100, Sebastian Andrzej Siewior wrote:
> Instead of manually disabling interrupts before invoking use
> generic_handle_irq_safe() which can be invoked with enabled and disabled
> interrupts.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Wolfram Sang <wsa@kernel.org>

Applied to for-next, thanks!