Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-11-05 Thread Bjorn Helgaas
On Mon, Nov 04, 2013 at 01:44:08PM +0100, Paul Bolle wrote:
> On Fri, 2013-10-04 at 09:55 -0600, Bjorn Helgaas wrote:
> > On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu  wrote:
> > > On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas  wrote:
> > >> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> > >>> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
> > >>>
> > >>>   pci_enable_bridge(dev->bus->self);
> > >>>
> > >>> - if (pci_is_enabled(dev))
> > >>> + if (pci_is_enabled(dev)) {
> > >>> + if (!dev->is_busmaster) {
> > >>> + dev_warn(>dev, "driver skip pci_set_master, 
> > >>> fix it!\n");
> > >>
> > >> I know this is already in Linus' tree, but if we're going to enable
> > >> bus mastering here, what's the point of the warning?  If somebody
> > >> fixes the driver by adding a pci_set_master() call there, does that
> > >> improve something?
> > >
> > > Help us to catch other offender and fix them.
> > 
> > What is improved by doing it in the driver instead of here?
> 
> After booting v3.12 for the first time on a laptop I noticed two new
> warnings: 
> <4>[4.427959] pcieport :00:1c.4: driver skip pci_set_master, fix 
> it!
> <4>[4.448630] pcieport :00:1c.1: driver skip pci_set_master, fix 
> it!
> 
> These warnings aren't entirely clear, but luckily they are easily
> greppable. It turns out they can be traced back to this patch.
> 
> So some further grepping, looking at the code, etc. suggests these
> warnings could be silenced by calling pci_set_master() before calling
> pci_enable_device(). Ie, reverse the current order of those calls in
> drivers/pci/pcie/portdrv_core.c:pcie_port_device_register(). Is that
> correct? But what should then be done if pci_enable_device() fails?
> 
> And Bjorn's question - what's the point of this warning if
> pci_set_master() will be called anyway - also came up when I looked at
> that code segment for the first time. But I'm not familiar with the PCI
> code.

Unless somebody can point out a problem with doing the pci_set_master()
inside pci_enable_bridges(), I plan to merge the following patch to
remove the warning.


PCI: Drop warning about drivers that don't use pci_set_master()

From: Bjorn Helgaas 

f41f064cf4 ("PCI: Workaround missing pci_set_master in pci drivers") made
pci_enable_bridge() turn on bus mastering if the driver hadn't done so
already.  It also added a warning in this case.  But there's no reason to
warn about it unless it's actually a problem to enable bus mastering here.

This patch drops the warning because I'm not aware of any such problem.

Signed-off-by: Bjorn Helgaas 
CC: Paul Bolle 
---
 drivers/pci/pci.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7a92d81..ac40f90 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1156,10 +1156,8 @@ static void pci_enable_bridge(struct pci_dev *dev)
pci_enable_bridge(dev->bus->self);
 
if (pci_is_enabled(dev)) {
-   if (!dev->is_busmaster) {
-   dev_warn(>dev, "driver skip pci_set_master, fix 
it!\n");
+   if (!dev->is_busmaster)
pci_set_master(dev);
-   }
return;
}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-11-05 Thread Bjorn Helgaas
On Mon, Nov 04, 2013 at 01:44:08PM +0100, Paul Bolle wrote:
 On Fri, 2013-10-04 at 09:55 -0600, Bjorn Helgaas wrote:
  On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu ying...@kernel.org wrote:
   On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas bhelg...@google.com wrote:
   On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
   @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
  
 pci_enable_bridge(dev-bus-self);
  
   - if (pci_is_enabled(dev))
   + if (pci_is_enabled(dev)) {
   + if (!dev-is_busmaster) {
   + dev_warn(dev-dev, driver skip pci_set_master, 
   fix it!\n);
  
   I know this is already in Linus' tree, but if we're going to enable
   bus mastering here, what's the point of the warning?  If somebody
   fixes the driver by adding a pci_set_master() call there, does that
   improve something?
  
   Help us to catch other offender and fix them.
  
  What is improved by doing it in the driver instead of here?
 
 After booting v3.12 for the first time on a laptop I noticed two new
 warnings: 
 4[4.427959] pcieport :00:1c.4: driver skip pci_set_master, fix 
 it!
 4[4.448630] pcieport :00:1c.1: driver skip pci_set_master, fix 
 it!
 
 These warnings aren't entirely clear, but luckily they are easily
 greppable. It turns out they can be traced back to this patch.
 
 So some further grepping, looking at the code, etc. suggests these
 warnings could be silenced by calling pci_set_master() before calling
 pci_enable_device(). Ie, reverse the current order of those calls in
 drivers/pci/pcie/portdrv_core.c:pcie_port_device_register(). Is that
 correct? But what should then be done if pci_enable_device() fails?
 
 And Bjorn's question - what's the point of this warning if
 pci_set_master() will be called anyway - also came up when I looked at
 that code segment for the first time. But I'm not familiar with the PCI
 code.

Unless somebody can point out a problem with doing the pci_set_master()
inside pci_enable_bridges(), I plan to merge the following patch to
remove the warning.


PCI: Drop warning about drivers that don't use pci_set_master()

From: Bjorn Helgaas bhelg...@google.com

f41f064cf4 (PCI: Workaround missing pci_set_master in pci drivers) made
pci_enable_bridge() turn on bus mastering if the driver hadn't done so
already.  It also added a warning in this case.  But there's no reason to
warn about it unless it's actually a problem to enable bus mastering here.

This patch drops the warning because I'm not aware of any such problem.

Signed-off-by: Bjorn Helgaas bhelg...@google.com
CC: Paul Bolle pebo...@tiscali.nl
---
 drivers/pci/pci.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7a92d81..ac40f90 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1156,10 +1156,8 @@ static void pci_enable_bridge(struct pci_dev *dev)
pci_enable_bridge(dev-bus-self);
 
if (pci_is_enabled(dev)) {
-   if (!dev-is_busmaster) {
-   dev_warn(dev-dev, driver skip pci_set_master, fix 
it!\n);
+   if (!dev-is_busmaster)
pci_set_master(dev);
-   }
return;
}
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-11-04 Thread Paul Bolle
On Fri, 2013-10-04 at 09:55 -0600, Bjorn Helgaas wrote:
> On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu  wrote:
> > On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas  wrote:
> >> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> >>> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
> >>>
> >>>   pci_enable_bridge(dev->bus->self);
> >>>
> >>> - if (pci_is_enabled(dev))
> >>> + if (pci_is_enabled(dev)) {
> >>> + if (!dev->is_busmaster) {
> >>> + dev_warn(>dev, "driver skip pci_set_master, 
> >>> fix it!\n");
> >>
> >> I know this is already in Linus' tree, but if we're going to enable
> >> bus mastering here, what's the point of the warning?  If somebody
> >> fixes the driver by adding a pci_set_master() call there, does that
> >> improve something?
> >
> > Help us to catch other offender and fix them.
> 
> What is improved by doing it in the driver instead of here?

After booting v3.12 for the first time on a laptop I noticed two new
warnings: 
<4>[4.427959] pcieport :00:1c.4: driver skip pci_set_master, fix it!
<4>[4.448630] pcieport :00:1c.1: driver skip pci_set_master, fix it!

These warnings aren't entirely clear, but luckily they are easily
greppable. It turns out they can be traced back to this patch.

So some further grepping, looking at the code, etc. suggests these
warnings could be silenced by calling pci_set_master() before calling
pci_enable_device(). Ie, reverse the current order of those calls in
drivers/pci/pcie/portdrv_core.c:pcie_port_device_register(). Is that
correct? But what should then be done if pci_enable_device() fails?

And Bjorn's question - what's the point of this warning if
pci_set_master() will be called anyway - also came up when I looked at
that code segment for the first time. But I'm not familiar with the PCI
code.


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-11-04 Thread Paul Bolle
On Fri, 2013-10-04 at 09:55 -0600, Bjorn Helgaas wrote:
 On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu ying...@kernel.org wrote:
  On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas bhelg...@google.com wrote:
  On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
  @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
 
pci_enable_bridge(dev-bus-self);
 
  - if (pci_is_enabled(dev))
  + if (pci_is_enabled(dev)) {
  + if (!dev-is_busmaster) {
  + dev_warn(dev-dev, driver skip pci_set_master, 
  fix it!\n);
 
  I know this is already in Linus' tree, but if we're going to enable
  bus mastering here, what's the point of the warning?  If somebody
  fixes the driver by adding a pci_set_master() call there, does that
  improve something?
 
  Help us to catch other offender and fix them.
 
 What is improved by doing it in the driver instead of here?

After booting v3.12 for the first time on a laptop I noticed two new
warnings: 
4[4.427959] pcieport :00:1c.4: driver skip pci_set_master, fix it!
4[4.448630] pcieport :00:1c.1: driver skip pci_set_master, fix it!

These warnings aren't entirely clear, but luckily they are easily
greppable. It turns out they can be traced back to this patch.

So some further grepping, looking at the code, etc. suggests these
warnings could be silenced by calling pci_set_master() before calling
pci_enable_device(). Ie, reverse the current order of those calls in
drivers/pci/pcie/portdrv_core.c:pcie_port_device_register(). Is that
correct? But what should then be done if pci_enable_device() fails?

And Bjorn's question - what's the point of this warning if
pci_set_master() will be called anyway - also came up when I looked at
that code segment for the first time. But I'm not familiar with the PCI
code.


Paul Bolle

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-04 Thread Bjorn Helgaas
On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu  wrote:
> On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas  wrote:
>> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
>>> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
>>>
>>>   pci_enable_bridge(dev->bus->self);
>>>
>>> - if (pci_is_enabled(dev))
>>> + if (pci_is_enabled(dev)) {
>>> + if (!dev->is_busmaster) {
>>> + dev_warn(>dev, "driver skip pci_set_master, fix 
>>> it!\n");
>>
>> I know this is already in Linus' tree, but if we're going to enable
>> bus mastering here, what's the point of the warning?  If somebody
>> fixes the driver by adding a pci_set_master() call there, does that
>> improve something?
>
> Help us to catch other offender and fix them.

What is improved by doing it in the driver instead of here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-04 Thread Bjorn Helgaas
On Thu, Oct 3, 2013 at 5:35 PM, Yinghai Lu ying...@kernel.org wrote:
 On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas bhelg...@google.com wrote:
 On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
 @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci

   pci_enable_bridge(dev-bus-self);

 - if (pci_is_enabled(dev))
 + if (pci_is_enabled(dev)) {
 + if (!dev-is_busmaster) {
 + dev_warn(dev-dev, driver skip pci_set_master, fix 
 it!\n);

 I know this is already in Linus' tree, but if we're going to enable
 bus mastering here, what's the point of the warning?  If somebody
 fixes the driver by adding a pci_set_master() call there, does that
 improve something?

 Help us to catch other offender and fix them.

What is improved by doing it in the driver instead of here?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-03 Thread Yinghai Lu
On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas  wrote:
> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
>> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
>>
>>   pci_enable_bridge(dev->bus->self);
>>
>> - if (pci_is_enabled(dev))
>> + if (pci_is_enabled(dev)) {
>> + if (!dev->is_busmaster) {
>> + dev_warn(>dev, "driver skip pci_set_master, fix 
>> it!\n");
>
> I know this is already in Linus' tree, but if we're going to enable
> bus mastering here, what's the point of the warning?  If somebody
> fixes the driver by adding a pci_set_master() call there, does that
> improve something?

Help us to catch other offender and fix them.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-03 Thread Bjorn Helgaas
On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> BenH found:
> | 928bea964827d7824b548c1f8e06eccbbc4d0d7d
> | PCI: Delay enabling bridges until they're needed
> 
> break PCI on powerpc.  The reason is that the PCIe port driver will
> call pci_enable_device() on the bridge, so device enabled (but skip
> pci_set_master because pcie_port_auto and no acpi on powerpc ).
> 
> Because of that, pci_enable_bridge() later on (called as a result of the
> child device driver doing pci_enable_device) will see the bridge as
> already enabled and will not call pci_set_master() on it.
> 
> Fixed by add checking in pci_enable_bridge, and call pci_set_master
> if driver skip that.
> That will make the code more robot and wade off problem for missing
> pci_set_master in drivers.
> 
> Reported-by: Benjamin Herrenschmidt 
> Signed-off-by: Yinghai Lu 
> 
> ---
>  drivers/pci/pci.c |8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/pci/pci.c
> ===
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
>  
>   pci_enable_bridge(dev->bus->self);
>  
> - if (pci_is_enabled(dev))
> + if (pci_is_enabled(dev)) {
> + if (!dev->is_busmaster) {
> + dev_warn(>dev, "driver skip pci_set_master, fix 
> it!\n");

I know this is already in Linus' tree, but if we're going to enable
bus mastering here, what's the point of the warning?  If somebody
fixes the driver by adding a pci_set_master() call there, does that
improve something?

Bjorn

> + pci_set_master(dev);
> + }
>   return;
> + }
> +
>   retval = pci_enable_device(dev);
>   if (retval)
>   dev_err(>dev, "Error enabling bridge (%d), continuing\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-03 Thread Bjorn Helgaas
On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
 BenH found:
 | 928bea964827d7824b548c1f8e06eccbbc4d0d7d
 | PCI: Delay enabling bridges until they're needed
 
 break PCI on powerpc.  The reason is that the PCIe port driver will
 call pci_enable_device() on the bridge, so device enabled (but skip
 pci_set_master because pcie_port_auto and no acpi on powerpc ).
 
 Because of that, pci_enable_bridge() later on (called as a result of the
 child device driver doing pci_enable_device) will see the bridge as
 already enabled and will not call pci_set_master() on it.
 
 Fixed by add checking in pci_enable_bridge, and call pci_set_master
 if driver skip that.
 That will make the code more robot and wade off problem for missing
 pci_set_master in drivers.
 
 Reported-by: Benjamin Herrenschmidt b...@kernel.crashing.org
 Signed-off-by: Yinghai Lu ying...@kernel.org
 
 ---
  drivers/pci/pci.c |8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)
 
 Index: linux-2.6/drivers/pci/pci.c
 ===
 --- linux-2.6.orig/drivers/pci/pci.c
 +++ linux-2.6/drivers/pci/pci.c
 @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
  
   pci_enable_bridge(dev-bus-self);
  
 - if (pci_is_enabled(dev))
 + if (pci_is_enabled(dev)) {
 + if (!dev-is_busmaster) {
 + dev_warn(dev-dev, driver skip pci_set_master, fix 
 it!\n);

I know this is already in Linus' tree, but if we're going to enable
bus mastering here, what's the point of the warning?  If somebody
fixes the driver by adding a pci_set_master() call there, does that
improve something?

Bjorn

 + pci_set_master(dev);
 + }
   return;
 + }
 +
   retval = pci_enable_device(dev);
   if (retval)
   dev_err(dev-dev, Error enabling bridge (%d), continuing\n,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-10-03 Thread Yinghai Lu
On Thu, Oct 3, 2013 at 3:06 PM, Bjorn Helgaas bhelg...@google.com wrote:
 On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
 @@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci

   pci_enable_bridge(dev-bus-self);

 - if (pci_is_enabled(dev))
 + if (pci_is_enabled(dev)) {
 + if (!dev-is_busmaster) {
 + dev_warn(dev-dev, driver skip pci_set_master, fix 
 it!\n);

 I know this is already in Linus' tree, but if we're going to enable
 bus mastering here, what's the point of the warning?  If somebody
 fixes the driver by adding a pci_set_master() call there, does that
 improve something?

Help us to catch other offender and fix them.

Yinghai
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Theodore Ts'o
On Sun, Sep 29, 2013 at 03:46:33PM -0700, Linus Torvalds wrote:
> On Sun, Sep 29, 2013 at 3:41 PM, Theodore Ts'o  wrote:
> > On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> >> Fixed by add checking in pci_enable_bridge, and call pci_set_master
> >> if driver skip that.
> >> That will make the code more robot and wade off problem for missing
> >> pci_set_master in drivers.
> >
> > Petty spelling nit; feel free to ignore unless you need to respin the
> > patch anyway
> >
> > s/robot/robost/
> 
> That's an oddly spelled nitpick "correction".

Sorry, typed too quickly.  :-(

- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Linus Torvalds
On Sun, Sep 29, 2013 at 3:41 PM, Theodore Ts'o  wrote:
> On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
>> Fixed by add checking in pci_enable_bridge, and call pci_set_master
>> if driver skip that.
>> That will make the code more robot and wade off problem for missing
>> pci_set_master in drivers.
>
> Petty spelling nit; feel free to ignore unless you need to respin the
> patch anyway
>
> s/robot/robost/

That's an oddly spelled nitpick "correction".

If you really want to fix it, it's "robust" and "ward off problems".
But it's too late now, and the wrong spelling and word choice is in
the git tree and released in the -rc3 I just pushed out.

   Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Theodore Ts'o
On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
> Fixed by add checking in pci_enable_bridge, and call pci_set_master
> if driver skip that.
> That will make the code more robot and wade off problem for missing
> pci_set_master in drivers.

Petty spelling nit; feel free to ignore unless you need to respin the
patch anyway

s/robot/robost/

Cheers,

- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Theodore Ts'o
On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
 Fixed by add checking in pci_enable_bridge, and call pci_set_master
 if driver skip that.
 That will make the code more robot and wade off problem for missing
 pci_set_master in drivers.

Petty spelling nit; feel free to ignore unless you need to respin the
patch anyway

s/robot/robost/

Cheers,

- Ted
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Linus Torvalds
On Sun, Sep 29, 2013 at 3:41 PM, Theodore Ts'o ty...@mit.edu wrote:
 On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
 Fixed by add checking in pci_enable_bridge, and call pci_set_master
 if driver skip that.
 That will make the code more robot and wade off problem for missing
 pci_set_master in drivers.

 Petty spelling nit; feel free to ignore unless you need to respin the
 patch anyway

 s/robot/robost/

That's an oddly spelled nitpick correction.

If you really want to fix it, it's robust and ward off problems.
But it's too late now, and the wrong spelling and word choice is in
the git tree and released in the -rc3 I just pushed out.

   Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-29 Thread Theodore Ts'o
On Sun, Sep 29, 2013 at 03:46:33PM -0700, Linus Torvalds wrote:
 On Sun, Sep 29, 2013 at 3:41 PM, Theodore Ts'o ty...@mit.edu wrote:
  On Sat, Sep 28, 2013 at 01:13:07PM -0700, Yinghai Lu wrote:
  Fixed by add checking in pci_enable_bridge, and call pci_set_master
  if driver skip that.
  That will make the code more robot and wade off problem for missing
  pci_set_master in drivers.
 
  Petty spelling nit; feel free to ignore unless you need to respin the
  patch anyway
 
  s/robot/robost/
 
 That's an oddly spelled nitpick correction.

Sorry, typed too quickly.  :-(

- Ted
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-28 Thread Yinghai Lu
BenH found:
| 928bea964827d7824b548c1f8e06eccbbc4d0d7d
| PCI: Delay enabling bridges until they're needed

break PCI on powerpc.  The reason is that the PCIe port driver will
call pci_enable_device() on the bridge, so device enabled (but skip
pci_set_master because pcie_port_auto and no acpi on powerpc ).

Because of that, pci_enable_bridge() later on (called as a result of the
child device driver doing pci_enable_device) will see the bridge as
already enabled and will not call pci_set_master() on it.

Fixed by add checking in pci_enable_bridge, and call pci_set_master
if driver skip that.
That will make the code more robot and wade off problem for missing
pci_set_master in drivers.

Reported-by: Benjamin Herrenschmidt 
Signed-off-by: Yinghai Lu 

---
 drivers/pci/pci.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/pci.c
===
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
 
pci_enable_bridge(dev->bus->self);
 
-   if (pci_is_enabled(dev))
+   if (pci_is_enabled(dev)) {
+   if (!dev->is_busmaster) {
+   dev_warn(>dev, "driver skip pci_set_master, fix 
it!\n");
+   pci_set_master(dev);
+   }
return;
+   }
+
retval = pci_enable_device(dev);
if (retval)
dev_err(>dev, "Error enabling bridge (%d), continuing\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] PCI: Workaround missing pci_set_master in pci drivers

2013-09-28 Thread Yinghai Lu
BenH found:
| 928bea964827d7824b548c1f8e06eccbbc4d0d7d
| PCI: Delay enabling bridges until they're needed

break PCI on powerpc.  The reason is that the PCIe port driver will
call pci_enable_device() on the bridge, so device enabled (but skip
pci_set_master because pcie_port_auto and no acpi on powerpc ).

Because of that, pci_enable_bridge() later on (called as a result of the
child device driver doing pci_enable_device) will see the bridge as
already enabled and will not call pci_set_master() on it.

Fixed by add checking in pci_enable_bridge, and call pci_set_master
if driver skip that.
That will make the code more robot and wade off problem for missing
pci_set_master in drivers.

Reported-by: Benjamin Herrenschmidt b...@kernel.crashing.org
Signed-off-by: Yinghai Lu ying...@kernel.org

---
 drivers/pci/pci.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/pci.c
===
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1156,8 +1156,14 @@ static void pci_enable_bridge(struct pci
 
pci_enable_bridge(dev-bus-self);
 
-   if (pci_is_enabled(dev))
+   if (pci_is_enabled(dev)) {
+   if (!dev-is_busmaster) {
+   dev_warn(dev-dev, driver skip pci_set_master, fix 
it!\n);
+   pci_set_master(dev);
+   }
return;
+   }
+
retval = pci_enable_device(dev);
if (retval)
dev_err(dev-dev, Error enabling bridge (%d), continuing\n,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/