Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-28 Thread Rafael J. Wysocki
On Friday, September 27, 2013 04:44:20 PM Yinghai Lu wrote: > [+ Rafael] > > On Fri, Sep 27, 2013 at 4:19 PM, Benjamin Herrenschmidt > wrote: > > On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: > > > >> ok, please if you are ok attached one instead. It will print some warning > >> about >

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-28 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 8:05 PM, Benjamin Herrenschmidt wrote: > On Fri, 2013-09-27 at 16:44 -0700, Yinghai Lu wrote: > > In the meantime, can you properly submit the other one with the warning > to Linus ? It will make things more robust overall... https://patchwork.kernel.org/patch/2959121/ --

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-28 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 8:05 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Fri, 2013-09-27 at 16:44 -0700, Yinghai Lu wrote: In the meantime, can you properly submit the other one with the warning to Linus ? It will make things more robust overall...

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-28 Thread Rafael J. Wysocki
On Friday, September 27, 2013 04:44:20 PM Yinghai Lu wrote: [+ Rafael] On Fri, Sep 27, 2013 at 4:19 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: ok, please if you are ok attached one instead. It will print some warning

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 16:44 -0700, Yinghai Lu wrote: > > Thus the port driver bails out before calling pci_set_master(). The fix > > is to call pci_set_master() unconditionally. However that lead me to > > find to a few interesting oddities in that port driver code: > > can we revert that

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
[+ Rafael] On Fri, Sep 27, 2013 at 4:19 PM, Benjamin Herrenschmidt wrote: > On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: > >> ok, please if you are ok attached one instead. It will print some warning >> about >> driver skipping pci_set_master, so we can catch more problem with drivers.

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: > ok, please if you are ok attached one instead. It will print some warning > about > driver skipping pci_set_master, so we can catch more problem with drivers. Except that the message is pretty cryptic :-) Especially since the driver causing

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 3:38 PM, Benjamin Herrenschmidt wrote: > On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: >> On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt >> wrote: >> >> > Wouldn't it be better to simply have pci_enable_device() always set bus >> > master on a bridge? I

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: > On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt > wrote: > > > Wouldn't it be better to simply have pci_enable_device() always set bus > > master on a bridge? I don't see any case where it makes sense to have > > an enabled bridge

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 10:44 -0700, Yinghai Lu wrote: > |/* Get and check PCI Express port services */ > |capabilities = get_port_device_capability(dev); > |if (!capabilities) > |return 0; > | > |pci_set_master(dev); > > so how come that

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: > On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt > wrote: > > > Wouldn't it be better to simply have pci_enable_device() always set bus > > master on a bridge? I don't see any case where it makes sense to have > > an enabled bridge

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt wrote: > Wouldn't it be better to simply have pci_enable_device() always set bus > master on a bridge? I don't see any case where it makes sense to have > an enabled bridge without the master bit set on it... Do you mean attached?

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 10:10 -0700, Linus Torvalds wrote: > > So i would like to use the first way that you suggest : call pci_set_master > > PCIe port driver. > > So I have to say, that if we can fix this with just adding a single > new pci_set_master() call, we should do that before we decide to

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 9:01 AM, Yinghai Lu wrote: > On Fri, Sep 27, 2013 at 1:28 AM, Benjamin Herrenschmidt > wrote: >> Hi Linus, Yinghai ! >> >> Please consider reverting: >> >> 928bea964827d7824b548c1f8e06eccbbc4d0d7d >> PCI: Delay enabling bridges until they're needed >> >> (I'd suggest to

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Linus Torvalds
On Fri, Sep 27, 2013 at 9:01 AM, Yinghai Lu wrote: > > So i would like to use the first way that you suggest : call pci_set_master > PCIe port driver. So I have to say, that if we can fix this with just adding a single new pci_set_master() call, we should do that before we decide to revert. If

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 1:28 AM, Benjamin Herrenschmidt wrote: > Hi Linus, Yinghai ! > > Please consider reverting: > > 928bea964827d7824b548c1f8e06eccbbc4d0d7d > PCI: Delay enabling bridges until they're needed > > (I'd suggest to revert now and maybe merge a better patch later) > > This breaks

Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
Hi Linus, Yinghai ! Please consider reverting: 928bea964827d7824b548c1f8e06eccbbc4d0d7d PCI: Delay enabling bridges until they're needed (I'd suggest to revert now and maybe merge a better patch later) This breaks PCI on the PowerPC "powernv" platform (which is booted via kexec) and probably

Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
Hi Linus, Yinghai ! Please consider reverting: 928bea964827d7824b548c1f8e06eccbbc4d0d7d PCI: Delay enabling bridges until they're needed (I'd suggest to revert now and maybe merge a better patch later) This breaks PCI on the PowerPC powernv platform (which is booted via kexec) and probably x86

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 1:28 AM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Hi Linus, Yinghai ! Please consider reverting: 928bea964827d7824b548c1f8e06eccbbc4d0d7d PCI: Delay enabling bridges until they're needed (I'd suggest to revert now and maybe merge a better patch later)

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Linus Torvalds
On Fri, Sep 27, 2013 at 9:01 AM, Yinghai Lu ying...@kernel.org wrote: So i would like to use the first way that you suggest : call pci_set_master PCIe port driver. So I have to say, that if we can fix this with just adding a single new pci_set_master() call, we should do that before we decide

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 9:01 AM, Yinghai Lu ying...@kernel.org wrote: On Fri, Sep 27, 2013 at 1:28 AM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Hi Linus, Yinghai ! Please consider reverting: 928bea964827d7824b548c1f8e06eccbbc4d0d7d PCI: Delay enabling bridges until they're

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 10:10 -0700, Linus Torvalds wrote: So i would like to use the first way that you suggest : call pci_set_master PCIe port driver. So I have to say, that if we can fix this with just adding a single new pci_set_master() call, we should do that before we decide to

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Wouldn't it be better to simply have pci_enable_device() always set bus master on a bridge? I don't see any case where it makes sense to have an enabled bridge without the master bit set on it... Do you

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Wouldn't it be better to simply have pci_enable_device() always set bus master on a bridge? I don't see any case where it makes sense to have an

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 10:44 -0700, Yinghai Lu wrote: |/* Get and check PCI Express port services */ |capabilities = get_port_device_capability(dev); |if (!capabilities) |return 0; | |pci_set_master(dev); so how come that pci_set_master is

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Wouldn't it be better to simply have pci_enable_device() always set bus master on a bridge? I don't see any case where it makes sense to have an

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
On Fri, Sep 27, 2013 at 3:38 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Fri, 2013-09-27 at 14:54 -0700, Yinghai Lu wrote: On Fri, Sep 27, 2013 at 2:46 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Wouldn't it be better to simply have pci_enable_device() always

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: ok, please if you are ok attached one instead. It will print some warning about driver skipping pci_set_master, so we can catch more problem with drivers. Except that the message is pretty cryptic :-) Especially since the driver causing

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Yinghai Lu
[+ Rafael] On Fri, Sep 27, 2013 at 4:19 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote: ok, please if you are ok attached one instead. It will print some warning about driver skipping pci_set_master, so we can catch more

Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

2013-09-27 Thread Benjamin Herrenschmidt
On Fri, 2013-09-27 at 16:44 -0700, Yinghai Lu wrote: Thus the port driver bails out before calling pci_set_master(). The fix is to call pci_set_master() unconditionally. However that lead me to find to a few interesting oddities in that port driver code: can we revert that partially