| Message ID | 20140130134819.GA31909@dhcp-26-207.brq.redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Alexander Gordeev <agordeev@redhat.com> writes: > Signed-off-by: Alexander Gordeev <agordeev@redhat.com> > --- > drivers/net/wireless/ath/ath10k/pci.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c > index 29fd197..6525e1f 100644 > --- a/drivers/net/wireless/ath/ath10k/pci.c > +++ b/drivers/net/wireless/ath/ath10k/pci.c > @@ -2414,8 +2414,6 @@ static int ath10k_pci_init_irq(struct ath10k *ar) > ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs); > if (ret == 0) > return 0; > - if (ret > 0) > - pci_disable_msi(ar_pci->pdev); I don't understand how this is superfluous. When I read the documentation for pci_enable_msi_block() it states that if it can't allocate all requests, it will return the number requests it could allocate. And in that case we want to fall back other modes. Am I missing something?
On Tue, Feb 04, 2014 at 08:32:12PM +0200, Kalle Valo wrote: > Alexander Gordeev <agordeev@redhat.com> writes: > > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com> > > --- > > drivers/net/wireless/ath/ath10k/pci.c | 2 -- > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c > > index 29fd197..6525e1f 100644 > > --- a/drivers/net/wireless/ath/ath10k/pci.c > > +++ b/drivers/net/wireless/ath/ath10k/pci.c > > @@ -2414,8 +2414,6 @@ static int ath10k_pci_init_irq(struct ath10k *ar) > > ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs); > > if (ret == 0) > > return 0; > > - if (ret > 0) > > - pci_disable_msi(ar_pci->pdev); > > I don't understand how this is superfluous. When I read the > documentation for pci_enable_msi_block() it states that if it can't > allocate all requests, it will return the number requests it could > allocate. And in that case we want to fall back other modes. > > Am I missing something? Yep. The documentation states 'could have been allocated', not 'could allocate'. IOW, MSIs are *not* enabled if a positive value returned. The code I changed tries to disable MSIs in such case, although it is not necessary, nor required. Just superfluous. HTH.
Alexander Gordeev <agordeev@redhat.com> writes: > On Tue, Feb 04, 2014 at 08:32:12PM +0200, Kalle Valo wrote: >> Alexander Gordeev <agordeev@redhat.com> writes: >> >> I don't understand how this is superfluous. When I read the >> documentation for pci_enable_msi_block() it states that if it can't >> allocate all requests, it will return the number requests it could >> allocate. And in that case we want to fall back other modes. >> >> Am I missing something? > > Yep. The documentation states 'could have been allocated', not 'could > allocate'. IOW, MSIs are *not* enabled if a positive value returned. > The code I changed tries to disable MSIs in such case, although it is > not necessary, nor required. Just superfluous. Ah, thanks for explaining that. I added this to the commit log (I hate empty commit logs anyway): ath10k: Get rid of superfluous call to pci_disable_msi() The documentation states that pci_enable_msi_block() returns the number of requests 'could have been allocated', not 'could allocate'. IOW, MSIs are *not* enabled if a positive value returned. kvalo: add commit log based on Alexander's email Signed-off-by: Alexander Gordeev <agordeev@redhat.com> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> Is it ok for me to take these patches to my ath.git tree or would you prefer to route them some other way?
On Wed, Feb 05, 2014 at 10:21:28AM +0200, Kalle Valo wrote: > Is it ok for me to take these patches to my ath.git tree or would you > prefer to route them some other way? Yeah, Bjorn has indicated he would pull it to his tree. I get it you are fine with 2/3 and 3/3?
Alexander Gordeev <agordeev@redhat.com> writes: > On Wed, Feb 05, 2014 at 10:21:28AM +0200, Kalle Valo wrote: >> Is it ok for me to take these patches to my ath.git tree or would you >> prefer to route them some other way? > > Yeah, Bjorn has indicated he would pull it to his tree. Ok, I'll drop these from my pending branch then. I just think it would have been easier if I would take these, smaller chance of conflicts that way. > I get it you are fine with 2/3 and 3/3? Yes, with the addition of the commit log to 1/3 I'll give: Acked-by: Kalle Valo <kvalo@qca.qualcomm.com>
On Wed, Feb 05, 2014 at 10:54:37AM +0200, Kalle Valo wrote: > Alexander Gordeev <agordeev@redhat.com> writes: > > > On Wed, Feb 05, 2014 at 10:21:28AM +0200, Kalle Valo wrote: > >> Is it ok for me to take these patches to my ath.git tree or would you > >> prefer to route them some other way? > > > > Yeah, Bjorn has indicated he would pull it to his tree. > > Ok, I'll drop these from my pending branch then. I just think it would > have been easier if I would take these, smaller chance of conflicts that > way. > > > I get it you are fine with 2/3 and 3/3? > > Yes, with the addition of the commit log to 1/3 I'll give: > > Acked-by: Kalle Valo <kvalo@qca.qualcomm.com> I haven't put these in my branch, so you can take them. Alexander has a whole batch of network driver updates that I think David Miller is going to apply; would it make sense to include these in that batch? There's also the wil6210 patch for drivers/net/wireless/ath/wil6210/pcie_bus.c, which seems like it maybe could be in that batch, too. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 11, 2014 at 05:31:43PM -0700, Bjorn Helgaas wrote: > I haven't put these in my branch, so you can take them. > > Alexander has a whole batch of network driver updates that I think David > Miller is going to apply; would it make sense to include these in that > batch? > > There's also the wil6210 patch for > drivers/net/wireless/ath/wil6210/pcie_bus.c, which seems like it maybe > could be in that batch, too. Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though. > Bjorn
On Wed, Feb 12, 2014 at 6:38 AM, Alexander Gordeev <agordeev@redhat.com> wrote: > On Tue, Feb 11, 2014 at 05:31:43PM -0700, Bjorn Helgaas wrote: >> I haven't put these in my branch, so you can take them. >> >> Alexander has a whole batch of network driver updates that I think David >> Miller is going to apply; would it make sense to include these in that >> batch? >> >> There's also the wil6210 patch for >> drivers/net/wireless/ath/wil6210/pcie_bus.c, which seems like it maybe >> could be in that batch, too. > > Well, as this series is small I thought it could quickly go thru your > tree. But since ipr had conflicts, there is no point routing all patches > altogether, so up to you guys. The wil6210 patch is already in your pci/msi > branch though. It's in pci/msi, but that's not in my -next branch yet, so I can easily drop it. Do drivers/net/wireless patches normally follow a different path than the other drivers/net patches? The wil6210 and ath10k patches look just like the others in the 34-patch series (bnx2, bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense to include them there. I think I need to put the ahci regression fix in v3.14, so I'll move that to my for-linus branch and just keep the other odds and ends (ahci and vfio) for v3.15. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas <bhelgaas@google.com> writes: >> Well, as this series is small I thought it could quickly go thru your >> tree. But since ipr had conflicts, there is no point routing all patches >> altogether, so up to you guys. The wil6210 patch is already in your pci/msi >> branch though. > > It's in pci/msi, but that's not in my -next branch yet, so I can > easily drop it. Do drivers/net/wireless patches normally follow a > different path than the other drivers/net patches? The wil6210 and > ath10k patches look just like the others in the 34-patch series (bnx2, > bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense > to include them there. ath10k patches normally go through my ath.git tree to Linville and then to David Miller. To avoid conflicts I would prefer to take ath10k patches to my tree whenever possible.
On Wed, Feb 12, 2014 at 2:30 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Bjorn Helgaas <bhelgaas@google.com> writes: > >>> Well, as this series is small I thought it could quickly go thru your >>> tree. But since ipr had conflicts, there is no point routing all patches >>> altogether, so up to you guys. The wil6210 patch is already in your pci/msi >>> branch though. >> >> It's in pci/msi, but that's not in my -next branch yet, so I can >> easily drop it. Do drivers/net/wireless patches normally follow a >> different path than the other drivers/net patches? The wil6210 and >> ath10k patches look just like the others in the 34-patch series (bnx2, >> bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense >> to include them there. > > ath10k patches normally go through my ath.git tree to Linville and then > to David Miller. To avoid conflicts I would prefer to take ath10k > patches to my tree whenever possible. OK, I won't do anything with ath10k (I haven't applied it anywhere). And if Alexander re-posts the networking series (I think he might, to add a pci_enable_msix_exact() interface), maybe he can include wil6210 with that series. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas <bhelgaas@google.com> writes: > On Wed, Feb 12, 2014 at 2:30 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Bjorn Helgaas <bhelgaas@google.com> writes: >> >>>> Well, as this series is small I thought it could quickly go thru your >>>> tree. But since ipr had conflicts, there is no point routing all patches >>>> altogether, so up to you guys. The wil6210 patch is already in your pci/msi >>>> branch though. >>> >>> It's in pci/msi, but that's not in my -next branch yet, so I can >>> easily drop it. Do drivers/net/wireless patches normally follow a >>> different path than the other drivers/net patches? The wil6210 and >>> ath10k patches look just like the others in the 34-patch series (bnx2, >>> bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense >>> to include them there. >> >> ath10k patches normally go through my ath.git tree to Linville and then >> to David Miller. To avoid conflicts I would prefer to take ath10k >> patches to my tree whenever possible. > > OK, I won't do anything with ath10k (I haven't applied it anywhere). I have now taken the ath10k patches to my pending branch, will apply them soon.
On Wed, Feb 12, 2014 at 11:30:44PM +0200, Kalle Valo wrote: > Bjorn Helgaas <bhelgaas@google.com> writes: > > >> Well, as this series is small I thought it could quickly go thru your > >> tree. But since ipr had conflicts, there is no point routing all patches > >> altogether, so up to you guys. The wil6210 patch is already in your pci/msi > >> branch though. > > > > It's in pci/msi, but that's not in my -next branch yet, so I can > > easily drop it. Do drivers/net/wireless patches normally follow a > > different path than the other drivers/net patches? The wil6210 and > > ath10k patches look just like the others in the 34-patch series (bnx2, > > bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense > > to include them there. > > ath10k patches normally go through my ath.git tree to Linville and then > to David Miller. To avoid conflicts I would prefer to take ath10k > patches to my tree whenever possible. CC'ing Vladimir, in case he decides to do the same with wil6210. > -- > Kalle Valo
Alexander Gordeev <agordeev@redhat.com> writes:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Thanks, all three patches applied to my ath.git tree.
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c index 29fd197..6525e1f 100644 --- a/drivers/net/wireless/ath/ath10k/pci.c +++ b/drivers/net/wireless/ath/ath10k/pci.c @@ -2414,8 +2414,6 @@ static int ath10k_pci_init_irq(struct ath10k *ar) ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs); if (ret == 0) return 0; - if (ret > 0) - pci_disable_msi(ar_pci->pdev); /* fall-through */ }
Signed-off-by: Alexander Gordeev <agordeev@redhat.com> --- drivers/net/wireless/ath/ath10k/pci.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)