Patchwork powerpc: disable MSI using new interface if possible

login
register
mail settings
Submitter Nishanth Aravamudan
Date March 4, 2011, 1:41 a.m.
Message ID <1299202862-10682-1-git-send-email-nacc@us.ibm.com>
Download mbox | patch
Permalink /patch/85360/
State Accepted, archived
Headers show

Comments

Nishanth Aravamudan - March 4, 2011, 1:41 a.m.
On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > On upcoming hardware, we have a PCI adapter with two functions, one of
> > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > vendor to ensure that the hardware is not at fault, but if we use the
> > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > automatically disable MSI-X and the adapter does not appear to signal
> > any stray MSI-X interrupt.
> 
> It seems this could also be a firmware bug, have we heard anything from
> them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).

We're tracking that down too. I think the fact that the interrupt is
coming in is a hardware bug in this particular adapter.

I'm looking at PAPR again and I see what might be a contradiction:

7.3.10.5.1: "To removing all MSIs, set the Requested Number of
Interrupts to zero."

Table 71: "Function ... 1: Request to set to a new number of MSI or
MSI-X (platform choice) interrupts (including set to 0)"

It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
change only MSI or MSI-X and still be not a bug?

It does seem like it should disable both, but from what we've seen with
this adapter, it doesn't appear to.

> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > Cc: Milton Miller <miltonm@bga.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> 
> Cc: Me  :)

Sorry! I was in a hurry to get this out the door, my fault. Note, you
don't show up per scripts/get_maintainer.pl :)

> > diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> > index 1164c34..9434576 100644
> > --- a/arch/powerpc/platforms/pseries/msi.c
> > +++ b/arch/powerpc/platforms/pseries/msi.c
> > @@ -93,8 +93,18 @@ static void rtas_disable_msi(struct pci_dev *pdev)
> >     if (!pdn)
> >             return;
> >  
> > -   if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
> > -           pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
> > +   /*
> > +    * disabling MSI with the explicit interface also disables MSI-X
> > +    */
> > +   if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> > +           /* 
> > +            * may have failed due to lacking
> > +            * "ibm,change-msix-capable" property
> > +            */
> > +           if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0) {
> > +                   pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
> > +           }
> > +   }
> >  }
> 
> This is probably a pretty safe change anyway, ie. use the newer API if
> it is present. The comment is backward though, the call fails because
> the new interface is not implemented, and that fact is signalled by the
> absence of the property.

Updated the comment, thanks:

On upcoming hardware, we have a PCI adapter with two functions, one of
which uses MSI and the other uses MSI-X. This adapter, when MSI is
disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
signals an MSI-X interrupt and triggers an EEH. We are working with the
vendor to ensure that the hardware is not at fault, but if we use the
"new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
automatically disable MSI-X and the adapter does not appear to signal
any stray MSI-X interrupt.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---

 arch/powerpc/platforms/pseries/msi.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)
Michael Ellerman - March 4, 2011, 3:01 a.m.
On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > vendor to ensure that the hardware is not at fault, but if we use the
> > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > automatically disable MSI-X and the adapter does not appear to signal
> > > any stray MSI-X interrupt.
> > 
> > It seems this could also be a firmware bug, have we heard anything from
> > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> 
> We're tracking that down too. I think the fact that the interrupt is
> coming in is a hardware bug in this particular adapter.
> 
> I'm looking at PAPR again and I see what might be a contradiction:
> 
> 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> Interrupts to zero."
> 
> Table 71: "Function ... 1: Request to set to a new number of MSI or
> MSI-X (platform choice) interrupts (including set to 0)"
> 
> It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> change only MSI or MSI-X and still be not a bug?

Yeah I guess you could read it that way, though I think that would be a
bug.

The idea is that it chooses for you whether it uses MSI or MSI-X. So the
only sane semantic is that when deconfiguring it deconfigures either,
ie. both, kinds.

Looking closer at your patch, now I don't understand :)

+       /*
+        * disabling MSI with the explicit interface also disables MSI-X
+        */
+       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {


So we first disable using function 3, which should:

        3: Request to set to a new number of MSI interrupts (including set to 0)

Which does not mention MSI-X at all, implying it has no effect on them.
Which contradicts what you see, and the comment in the code?

So I think I'm not sure what's going on here :)

cheers
Michael Ellerman - March 4, 2011, 3:06 a.m.
On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > On upcoming hardware, we have a PCI adapter with two functions, one of
..
> > > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > > Cc: Milton Miller <miltonm@bga.com>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > 
> > Cc: Me  :)
> 
> Sorry! I was in a hurry to get this out the door, my fault. Note, you
> don't show up per scripts/get_maintainer.pl :)

No worries, though I will remember never to use get_maintainer.pl, it is
obviously utterly broken:

$ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
Andre Detsch adetsch@br.ibm.com
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au

And the patch from Andre was also signed off by me.

cheers
Joe Perches - March 4, 2011, 3:29 a.m.
On Fri, 2011-03-04 at 14:06 +1100, Michael Ellerman wrote:
> On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > Cc: Me  :)
> > Sorry! I was in a hurry to get this out the door, my fault. Note, you
> > don't show up per scripts/get_maintainer.pl :)
> No worries, though I will remember never to use get_maintainer.pl, it is
> obviously utterly broken:

Blah, blah, stupid tool doesn't work exactly as I want,
and it doesn't credit me for my over 2 year old patches,
therefore it's not only stupid, it's broken...

$ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
linux-kernel@vger.kernel.org (open list)
devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)

If you don't like how it currently works, suggest
improvements.

> $ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
> Andre Detsch adetsch@br.ibm.com
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> 
> And the patch from Andre was also signed off by me.

How nice for you.  Last patch from you was
2 years ago.

$ git log --format="%an %ae %ad" arch/powerpc/platforms/pseries/msi.c
Andre Detsch adetsch@br.ibm.com Wed Nov 4 13:03:19 2009 -0200
Michael Ellerman michael@ellerman.id.au Thu Mar 5 14:44:26 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Feb 17 00:21:56 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Feb 17 00:18:49 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:33 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:32 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:31 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:31 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Oct 23 14:23:44 2007 +1000
Michael Ellerman michael@ellerman.id.au Thu Sep 20 16:36:50 2007 +1000
Michael Ellerman michael@ellerman.id.au Thu Sep 20 16:36:48 2007 +1000
Michael Ellerman michael@ellerman.id.au Tue May 8 12:58:35 2007 +1000

CC'ing inactive non named maintainers via git
history also draws complaints btw.
Nishanth Aravamudan - March 4, 2011, 7:24 a.m.
On 04.03.2011 [14:01:24 +1100], Michael Ellerman wrote:
> On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > > vendor to ensure that the hardware is not at fault, but if we use the
> > > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > > automatically disable MSI-X and the adapter does not appear to signal
> > > > any stray MSI-X interrupt.
> > > 
> > > It seems this could also be a firmware bug, have we heard anything from
> > > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> > 
> > We're tracking that down too. I think the fact that the interrupt is
> > coming in is a hardware bug in this particular adapter.
> > 
> > I'm looking at PAPR again and I see what might be a contradiction:
> > 
> > 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> > Interrupts to zero."
> > 
> > Table 71: "Function ... 1: Request to set to a new number of MSI or
> > MSI-X (platform choice) interrupts (including set to 0)"
> > 
> > It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> > change only MSI or MSI-X and still be not a bug?
> 
> Yeah I guess you could read it that way, though I think that would be a
> bug.
> 
> The idea is that it chooses for you whether it uses MSI or MSI-X. So the
> only sane semantic is that when deconfiguring it deconfigures either,
> ie. both, kinds.

I agree with you that is how it should be :) I'm asking the firmware
folks to make sure I'm not misunderstanding the underlying issue.

> Looking closer at your patch, now I don't understand :)
> 
> +       /*
> +        * disabling MSI with the explicit interface also disables MSI-X
> +        */
> +       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> 
> 
> So we first disable using function 3, which should:
> 
>         3: Request to set to a new number of MSI interrupts (including set to 0)
> 
> Which does not mention MSI-X at all, implying it has no effect on them.
> Which contradicts what you see, and the comment in the code?

Thanks for the thorough review!

Per PAPR 2.4 from Power.org, look at the page before that table, page
169:

"Specifying Function 3 (MSI) also disables MSI-X for the specified IOA
function, and likewise specifying Function 4 (MSI-X) disables MSI for
the IOA function....Specifying the Requested Number of Interrupts to
zero for either Function 3 or 4 removes all MSI & MSI-X interrupts from
the IOA function."

So I'm relying on this aspect of PAPR being enforced by the firmware,
which I think it is in my testing.

Thanks,
Nish
florian@mickler.org - March 4, 2011, 11:13 a.m.
On Thu, 03 Mar 2011 19:29:19 -0800
Joe Perches <joe@perches.com> wrote:

> > $ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
> > Andre Detsch adetsch@br.ibm.com
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > 
> > And the patch from Andre was also signed off by me.
> 
> How nice for you.  Last patch from you was
> 2 years ago.
> 

The problem is, that it get's pretty slow if you don't do a time
cut-off... i don't know the complexity but i guess it's linear in
(number of files) * (number of commits) * (avg size of diffs) ... 
(if you compute the diffstat) 

What I'll probably try as soon as I have another few hours to spare
for get_maintainer.pl, is to implement a fall back to older history if
the data basis is not stable enough. 


> 
> CC'ing inactive non named maintainers via git
> history also draws complaints btw.
> 
Yes.  I guess, my current version could use an staggered approach where
it does more digging if the results are too volatile... but first
try to be fast ...

Regards,
Flo
Nishanth Aravamudan - March 8, 2011, 5:34 a.m.
On 03.03.2011 [23:24:44 -0800], Nishanth Aravamudan wrote:
> On 04.03.2011 [14:01:24 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > > > vendor to ensure that the hardware is not at fault, but if we use the
> > > > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > > > automatically disable MSI-X and the adapter does not appear to signal
> > > > > any stray MSI-X interrupt.
> > > > 
> > > > It seems this could also be a firmware bug, have we heard anything from
> > > > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > > > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> > > 
> > > We're tracking that down too. I think the fact that the interrupt is
> > > coming in is a hardware bug in this particular adapter.
> > > 
> > > I'm looking at PAPR again and I see what might be a contradiction:
> > > 
> > > 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> > > Interrupts to zero."
> > > 
> > > Table 71: "Function ... 1: Request to set to a new number of MSI or
> > > MSI-X (platform choice) interrupts (including set to 0)"
> > > 
> > > It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> > > change only MSI or MSI-X and still be not a bug?
> > 
> > Yeah I guess you could read it that way, though I think that would be a
> > bug.
> > 
> > The idea is that it chooses for you whether it uses MSI or MSI-X. So the
> > only sane semantic is that when deconfiguring it deconfigures either,
> > ie. both, kinds.
> 
> I agree with you that is how it should be :) I'm asking the firmware
> folks to make sure I'm not misunderstanding the underlying issue.

It would appear that if a device does support both MSI and MSI-X and the
old (non-explicit) interface is used, only one of MSI or MSI-X is
guaranteed to be disabled, with preference given to MSI. Now, it turns
out that there is also a firmware dragon here (because this particular
device is misbehaving), but we can also fix it by using the new
interface, which shouldn't cause any harm, given the fallback.

> > Looking closer at your patch, now I don't understand :)
> > 
> > +       /*
> > +        * disabling MSI with the explicit interface also disables MSI-X
> > +        */
> > +       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> > 
> > 
> > So we first disable using function 3, which should:
> > 
> >         3: Request to set to a new number of MSI interrupts (including set to 0)
> > 
> > Which does not mention MSI-X at all, implying it has no effect on them.
> > Which contradicts what you see, and the comment in the code?
> 
> Thanks for the thorough review!
> 
> Per PAPR 2.4 from Power.org, look at the page before that table, page
> 169:
> 
> "Specifying Function 3 (MSI) also disables MSI-X for the specified IOA
> function, and likewise specifying Function 4 (MSI-X) disables MSI for
> the IOA function....Specifying the Requested Number of Interrupts to
> zero for either Function 3 or 4 removes all MSI & MSI-X interrupts from
> the IOA function."
> 
> So I'm relying on this aspect of PAPR being enforced by the firmware,
> which I think it is in my testing.

Given all that, do I have your Ack? :)

Thanks,
Nish
Michael Ellerman - March 9, 2011, 10:54 p.m.
On Mon, 2011-03-07 at 21:34 -0800, Nishanth Aravamudan wrote:
> On 03.03.2011 [23:24:44 -0800], Nishanth Aravamudan wrote:
> > On 04.03.2011 [14:01:24 +1100], Michael Ellerman wrote:
> > > Looking closer at your patch, now I don't understand :)
> > > 
> > > +       /*
> > > +        * disabling MSI with the explicit interface also disables MSI-X
> > > +        */
> > > +       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> > > 
> > > 
> > > So we first disable using function 3, which should:
> > > 
> > >         3: Request to set to a new number of MSI interrupts (including set to 0)
> > > 
> > > Which does not mention MSI-X at all, implying it has no effect on them.
> > > Which contradicts what you see, and the comment in the code?
> > 
> > Thanks for the thorough review!
> > 
> > Per PAPR 2.4 from Power.org, look at the page before that table, page
> > 169:
> > 
> > "Specifying Function 3 (MSI) also disables MSI-X for the specified IOA
> > function, and likewise specifying Function 4 (MSI-X) disables MSI for
> > the IOA function....Specifying the Requested Number of Interrupts to
> > zero for either Function 3 or 4 removes all MSI & MSI-X interrupts from
> > the IOA function."
> > 
> > So I'm relying on this aspect of PAPR being enforced by the firmware,
> > which I think it is in my testing.
> 
> Given all that, do I have your Ack? :)

Indeed. Thanks for clarifying it.

Acked-by: Michael Ellerman <michael@ellerman.id.au>

cheers
Michael Ellerman - March 9, 2011, 11:12 p.m.
On Thu, 2011-03-03 at 19:29 -0800, Joe Perches wrote:
> On Fri, 2011-03-04 at 14:06 +1100, Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > > Cc: Me  :)
> > > Sorry! I was in a hurry to get this out the door, my fault. Note, you
> > > don't show up per scripts/get_maintainer.pl :)
> > No worries, though I will remember never to use get_maintainer.pl, it is
> > obviously utterly broken:
> 
> Blah, blah, stupid tool doesn't work exactly as I want,
> and it doesn't credit me for my over 2 year old patches,
> therefore it's not only stupid, it's broken...

I do not give two shits about being given "credit" for this horrible
code, but if someone changes it I may have input.

True my last patch may have been two years ago, but I _wrote the entire
file_, and essentially no one else has ever touched it.

> $ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
> Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> linux-kernel@vger.kernel.org (open list)
> devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> 
> If you don't like how it currently works, suggest
> improvements.

I think I was suggesting that I should be in that list :)

In the life of this file there have been 553 lines changed (added or
deleted), of which I have written 551, and I signed off the other 2.

So I guess I'm suggesting that when someone has written a large number
of the changes to a file they should be CC'ed. I don't know what the cut
off for "large number" should be, but anything > 50% would seem
reasonable (in terms of lines, not commits).

> How nice for you.  Last patch from you was 2 years ago.

Last patch from _anyone_ was 2 years ago. As much as I may have tried to
banish that code from my mind I can still remember some of the details,
even after 2 years :)

> CC'ing inactive non named maintainers via git
> history also draws complaints btw.

Yeah I can imagine. But that is a case of someone who is no longer
interested getting one extra email, ie. a false positive, whereas in
this case it's someone who does care _not_ seeing the patch, ie. a false
negative.

cheers
Joe Perches - March 10, 2011, 12:28 a.m.
On Thu, 2011-03-10 at 10:12 +1100, Michael Ellerman wrote:
> True my last patch may have been two years ago, but I _wrote the entire
> file_, and essentially no one else has ever touched it.
> 
> > $ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
> > Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
> > Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> > linux-kernel@vger.kernel.org (open list)
> > devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> > 
> > If you don't like how it currently works, suggest
> > improvements.
> 
> I think I was suggesting that I should be in that list :)

$ ./scripts/get_maintainer.pl -f --git-blame arch/powerpc/platforms/pseries/msi.c
Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...,commits:8/11=73%)
Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...,commits:4/11=36%)
Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
Michael Ellerman <michael@ellerman.id.au> (authored lines:481/481=100%,commits:11/11=100%)
Linas Vepstas <linas@austin.ibm.com> (commits:1/11=9%)
linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
linux-kernel@vger.kernel.org (open list)
devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)

cheers, Joe
Nishanth Aravamudan - March 10, 2011, 12:46 a.m.
On 09.03.2011 [16:28:23 -0800], Joe Perches wrote:
> On Thu, 2011-03-10 at 10:12 +1100, Michael Ellerman wrote:
> > True my last patch may have been two years ago, but I _wrote the entire
> > file_, and essentially no one else has ever touched it.
> > 
> > > $ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
> > > Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
> > > Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> > > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> > > linux-kernel@vger.kernel.org (open list)
> > > devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> > > 
> > > If you don't like how it currently works, suggest
> > > improvements.
> > 
> > I think I was suggesting that I should be in that list :)
> 
> $ ./scripts/get_maintainer.pl -f --git-blame arch/powerpc/platforms/pseries/msi.c

Ah thanks, I didn't realize that flag existed, as I'd not needed to use
it before.

-Nish
Michael Ellerman - March 10, 2011, 3:35 a.m.
On Wed, 2011-03-09 at 16:28 -0800, Joe Perches wrote:
> On Thu, 2011-03-10 at 10:12 +1100, Michael Ellerman wrote:
> > True my last patch may have been two years ago, but I _wrote the entire
> > file_, and essentially no one else has ever touched it.
> > 
> > > $ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
> > > Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
> > > Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> > > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> > > linux-kernel@vger.kernel.org (open list)
> > > devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> > > 
> > > If you don't like how it currently works, suggest
> > > improvements.
> > 
> > I think I was suggesting that I should be in that list :)
> 
> $ ./scripts/get_maintainer.pl -f --git-blame arch/powerpc/platforms/pseries/msi.c
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...,commits:8/11=73%)
> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...,commits:4/11=36%)
> Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> Michael Ellerman <michael@ellerman.id.au> (authored lines:481/481=100%,commits:11/11=100%)
> Linas Vepstas <linas@austin.ibm.com> (commits:1/11=9%)
> linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> linux-kernel@vger.kernel.org (open list)
> devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)

That's awesome. But it's not the default, so hardly anyone will ever use
it :)

cheers
Joe Perches - March 10, 2011, 3:45 a.m.
On Thu, 2011-03-10 at 14:35 +1100, Michael Ellerman wrote:
> On Wed, 2011-03-09 at 16:28 -0800, Joe Perches wrote:
> > On Thu, 2011-03-10 at 10:12 +1100, Michael Ellerman wrote:
> > > True my last patch may have been two years ago, but I _wrote the entire
> > > file_, and essentially no one else has ever touched it.
> > > > If you don't like how it currently works, suggest
> > > > improvements.
> > > I think I was suggesting that I should be in that list :)
> > $ ./scripts/get_maintainer.pl -f --git-blame arch/powerpc/platforms/pseries/msi.c
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...,commits:8/11=73%)
> > Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...,commits:4/11=36%)
> > Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
> > Michael Ellerman <michael@ellerman.id.au> (authored lines:481/481=100%,commits:11/11=100%)
> > Linas Vepstas <linas@austin.ibm.com> (commits:1/11=9%)
> > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> > linux-kernel@vger.kernel.org (open list)
> > devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> That's awesome. But it's not the default, so hardly anyone will ever use
> it :)

It's not the default for a few reasons:

o It can take a _long_ time to run
o Linus Torvalds is still the "author of record"
  for all the files in the initial commit
o For files with activity, it doesn't add very
  much useful as old commit authors do also tend
  to show up in the new commits.
o It can show maintainers that are no longer
  active.
florian@mickler.org - March 10, 2011, 7:04 a.m.
Am 10.03.2011 04:50 schrieb "Joe Perches" <joe@perches.com>:
>
> On Thu, 2011-03-10 at 14:35 +1100, Michael Ellerman wrote:
> > On Wed, 2011-03-09 at 16:28 -0800, Joe Perches wrote:
> > > On Thu, 2011-03-10 at 10:12 +1100, Michael Ellerman wrote:
> > > > True my last patch may have been two years ago, but I _wrote the
entire
> > > > file_, and essentially no one else has ever touched it.
> > > > > If you don't like how it currently works, suggest
> > > > > improvements.
> > > > I think I was suggesting that I should be in that list :)
> > > $ ./scripts/get_maintainer.pl -f --git-blame
arch/powerpc/platforms/pseries/msi.c
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR
POWERPC...,commits:8/11=73%)
> > > Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR
POWERPC...,commits:4/11=36%)
> > > Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE
AND...)
> > > Michael Ellerman <michael@ellerman.id.au> (authored
lines:481/481=100%,commits:11/11=100%)
> > > Linas Vepstas <linas@austin.ibm.com> (commits:1/11=9%)
> > > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
> > > linux-kernel@vger.kernel.org (open list)
> > > devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)
> > That's awesome. But it's not the default, so hardly anyone will ever use
> > it :)
>
> It's not the default for a few reasons:
>
> o It can take a _long_ time to run
> o Linus Torvalds is still the "author of record"
>  for all the files in the initial commit
> o For files with activity, it doesn't add very
>  much useful as old commit authors do also tend
>  to show up in the new commits.
> o It can show maintainers that are no longer
>  active.
>
Michael, maybe I'm missing something,  but why don't you have a MAINTAINERS
entry for that file? That would fix it for you.

Regards, Flo

Patch

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 1164c34..18ac801 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -93,8 +93,18 @@  static void rtas_disable_msi(struct pci_dev *pdev)
 	if (!pdn)
 		return;
 
-	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
-		pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
+	/*
+	 * disabling MSI with the explicit interface also disables MSI-X
+	 */
+	if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
+		/* 
+		 * may have failed because explicit interface is not
+		 * present
+		 */
+		if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0) {
+			pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
+		}
+	}
 }
 
 static int rtas_query_irq_number(struct pci_dn *pdn, int offset)