Re: [PATCH v5 1/2] PCI: designware: Fix crash in dw_msi_teardown_irq

[Marek Vasut <marex@denx.de>]

Dear Bjørn Erik Nilsen,
[...]
> > Won't simple
> > 
> > for (i = 0; i < nvec; i++) {
> > 
> > do here?
> 
> Yes.
> 
> The very same syntax ('while i < nvec') is used in assign_irq. That is
> why I wanted to keep it like that to avoid adding too much confusion, or
> at least make it easy to recognize the same pattern.
> 
> You still want me to change it?

Your loop does exactly what a for() loop would do, there's no need to emulate it 
with a while() loop. If you can fix the other one, fix the other one as well 
please.

> As for the other nitpick, I don't agree with you.
> 
> In fact, dw_msi_teardown_irq has no return value itself. Moreover, if
> setting the msi desc to NULL fails, then it simply means there is no irq
> desc and there is nothing to unwind. Also, skipping the other unwind
> operations just because that single operation failed would leave the
> driver in a much worse state. At least in my opinion.
> 
> What's your opinion on that?

I see this:

$ git grep irq_set_msi_desc_off include/
include/linux/irq.h:extern int irq_set_msi_desc_off(unsigned int irq_base, 
unsigned int irq_offset,

So it has a return value which needs to be handled. Sorry if I wasn't clear on 
the first try.

> Now, if you want me to make another patch do you prefer a standalone
> patch on top of the other patches, or a completely new patchset?

Let's keep iterating :) But please wait for more feedback first.

Best regards,
Marek Vasut