* [PATCH] PCI/portdrv: Set master earlier
@ 2013-10-09 14:33 Jean Delvare
2013-12-07 22:23 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2013-10-09 14:33 UTC (permalink / raw)
To: linux-pci; +Cc: Bjorn Helgaas, Takashi Iwai
Since kernel 3.12-rc3, I get the following warning messages at boot:
pcieport 0000:00:07.0: driver skip pci_set_master, fix it!
pcieport 0000:00:01.0: driver skip pci_set_master, fix it!
These are:
00:01.0 PCI bridge: Intel Corporation 5520/5500/X58 I/O Hub PCI Express Root Port 1 (rev 13)
00:07.0 PCI bridge: Intel Corporation 5520/5500/X58 I/O Hub PCI Express Root Port 7 (rev 13)
Calling pci_set_master() immediately after pci_enable_device() makes
the warning messages go away.
Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
Please note that I am far from certain that this is the correct fix.
Another approach would be to disable the device if
get_port_device_capability() returns no capabilities. I suppose the
device would then be re-enabled later if really needed. What's the best
approach?
drivers/pci/pcie/portdrv_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-3.12-rc4.orig/drivers/pci/pcie/portdrv_core.c 2013-09-24 00:41:09.000000000 +0200
+++ linux-3.12-rc4/drivers/pci/pcie/portdrv_core.c 2013-10-09 15:43:54.205943783 +0200
@@ -366,13 +366,13 @@ int pcie_port_device_register(struct pci
status = pci_enable_device(dev);
if (status)
return status;
+ pci_set_master(dev);
/* Get and check PCI Express port services */
capabilities = get_port_device_capability(dev);
if (!capabilities)
return 0;
- pci_set_master(dev);
/*
* Initialize service irqs. Don't use service devices that
* require interrupts if there is no way to generate them.
--
Jean Delvare
Suse L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/portdrv: Set master earlier
2013-10-09 14:33 [PATCH] PCI/portdrv: Set master earlier Jean Delvare
@ 2013-12-07 22:23 ` Bjorn Helgaas
2013-12-08 3:26 ` Yinghai Lu
2013-12-08 13:09 ` Jean Delvare
0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2013-12-07 22:23 UTC (permalink / raw)
To: Jean Delvare; +Cc: linux-pci@vger.kernel.org, Takashi Iwai, Yinghai Lu
[+cc Yinghai]
On Wed, Oct 9, 2013 at 8:33 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Since kernel 3.12-rc3, I get the following warning messages at boot:
> pcieport 0000:00:07.0: driver skip pci_set_master, fix it!
> pcieport 0000:00:01.0: driver skip pci_set_master, fix it!
cf3e1feba7f9 ("PCI: Workaround missing pci_set_master in pci drivers")
added this warning and, at the same time, turned on bus mastering.
There didn't seem to be any reason for the warning and no reason why
bus mastering should be enabled in the driver rather than the core, so
fbeeb822f6f4 ("PCI: Drop warning about drivers that don't use
pci_set_master()") just dropped the warning.
This should resolve the problem, so I don't think there's a need for
this patch. Let me know if otherwise.
Bjorn
> These are:
> 00:01.0 PCI bridge: Intel Corporation 5520/5500/X58 I/O Hub PCI Express Root Port 1 (rev 13)
> 00:07.0 PCI bridge: Intel Corporation 5520/5500/X58 I/O Hub PCI Express Root Port 7 (rev 13)
>
> Calling pci_set_master() immediately after pci_enable_device() makes
> the warning messages go away.
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> Please note that I am far from certain that this is the correct fix.
> Another approach would be to disable the device if
> get_port_device_capability() returns no capabilities. I suppose the
> device would then be re-enabled later if really needed. What's the best
> approach?
>
> drivers/pci/pcie/portdrv_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-3.12-rc4.orig/drivers/pci/pcie/portdrv_core.c 2013-09-24 00:41:09.000000000 +0200
> +++ linux-3.12-rc4/drivers/pci/pcie/portdrv_core.c 2013-10-09 15:43:54.205943783 +0200
> @@ -366,13 +366,13 @@ int pcie_port_device_register(struct pci
> status = pci_enable_device(dev);
> if (status)
> return status;
> + pci_set_master(dev);
>
> /* Get and check PCI Express port services */
> capabilities = get_port_device_capability(dev);
> if (!capabilities)
> return 0;
>
> - pci_set_master(dev);
> /*
> * Initialize service irqs. Don't use service devices that
> * require interrupts if there is no way to generate them.
>
>
> --
> Jean Delvare
> Suse L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PCI/portdrv: Set master earlier
2013-12-07 22:23 ` Bjorn Helgaas
@ 2013-12-08 3:26 ` Yinghai Lu
2013-12-09 3:07 ` Benjamin Herrenschmidt
2013-12-08 13:09 ` Jean Delvare
1 sibling, 1 reply; 6+ messages in thread
From: Yinghai Lu @ 2013-12-08 3:26 UTC (permalink / raw)
To: Bjorn Helgaas, Benjamin Herrenschmidt
Cc: Jean Delvare, linux-pci@vger.kernel.org, Takashi Iwai
[+to BenH]
On Sat, Dec 7, 2013 at 2:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Yinghai]
>
> On Wed, Oct 9, 2013 at 8:33 AM, Jean Delvare <jdelvare@suse.de> wrote:
>> Since kernel 3.12-rc3, I get the following warning messages at boot:
>> pcieport 0000:00:07.0: driver skip pci_set_master, fix it!
>> pcieport 0000:00:01.0: driver skip pci_set_master, fix it!
>
> cf3e1feba7f9 ("PCI: Workaround missing pci_set_master in pci drivers")
> added this warning and, at the same time, turned on bus mastering.
>
> There didn't seem to be any reason for the warning and no reason why
> bus mastering should be enabled in the driver rather than the core, so
> fbeeb822f6f4 ("PCI: Drop warning about drivers that don't use
> pci_set_master()") just dropped the warning.
>
> This should resolve the problem, so I don't think there's a need for
> this patch. Let me know if otherwise.
BenH mentioned that get_port_device_capability have some problem on powerpc
and will return null.
>>
>> drivers/pci/pcie/portdrv_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux-3.12-rc4.orig/drivers/pci/pcie/portdrv_core.c 2013-09-24 00:41:09.000000000 +0200
>> +++ linux-3.12-rc4/drivers/pci/pcie/portdrv_core.c 2013-10-09 15:43:54.205943783 +0200
>> @@ -366,13 +366,13 @@ int pcie_port_device_register(struct pci
>> status = pci_enable_device(dev);
>> if (status)
>> return status;
>> + pci_set_master(dev);
>>
>> /* Get and check PCI Express port services */
>> capabilities = get_port_device_capability(dev);
>> if (!capabilities)
>> return 0;
>>
>> - pci_set_master(dev);
>> /*
>> * Initialize service irqs. Don't use service devices that
>> * require interrupts if there is no way to generate them.
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PCI/portdrv: Set master earlier
2013-12-08 3:26 ` Yinghai Lu
@ 2013-12-09 3:07 ` Benjamin Herrenschmidt
2013-12-17 0:53 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2013-12-09 3:07 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Jean Delvare, linux-pci@vger.kernel.org,
Takashi Iwai
On Sat, 2013-12-07 at 19:26 -0800, Yinghai Lu wrote:
> BenH mentioned that get_port_device_capability have some problem on
> powerpc and will return null.
Well, it has problems on anything that doesn't have ACPI :-)
I don't care what the "right" fix is but we need to makes sure nobody
does a pci_enable_device() on a bridge and doesn't also enable bus
master, otherwise the bridge bus master will never be set. This is what
was happening an breaking us.
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/portdrv: Set master earlier
2013-12-09 3:07 ` Benjamin Herrenschmidt
@ 2013-12-17 0:53 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2013-12-17 0:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Yinghai Lu, Jean Delvare, linux-pci@vger.kernel.org, Takashi Iwai
On Sun, Dec 8, 2013 at 8:07 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sat, 2013-12-07 at 19:26 -0800, Yinghai Lu wrote:
>> BenH mentioned that get_port_device_capability have some problem on
>> powerpc and will return null.
>
> Well, it has problems on anything that doesn't have ACPI :-)
>
> I don't care what the "right" fix is but we need to makes sure nobody
> does a pci_enable_device() on a bridge and doesn't also enable bus
> master, otherwise the bridge bus master will never be set. This is what
> was happening an breaking us.
I assume there's nothing more I need to do here. If there is, please
let me know.
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/portdrv: Set master earlier
2013-12-07 22:23 ` Bjorn Helgaas
2013-12-08 3:26 ` Yinghai Lu
@ 2013-12-08 13:09 ` Jean Delvare
1 sibling, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2013-12-08 13:09 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, Takashi Iwai, Yinghai Lu
Hi Bjorn,
On Sat, 7 Dec 2013 15:23:25 -0700, Bjorn Helgaas wrote:
> [+cc Yinghai]
>
> On Wed, Oct 9, 2013 at 8:33 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > Since kernel 3.12-rc3, I get the following warning messages at boot:
> > pcieport 0000:00:07.0: driver skip pci_set_master, fix it!
> > pcieport 0000:00:01.0: driver skip pci_set_master, fix it!
>
> cf3e1feba7f9 ("PCI: Workaround missing pci_set_master in pci drivers")
> added this warning and, at the same time, turned on bus mastering.
>
> There didn't seem to be any reason for the warning and no reason why
> bus mastering should be enabled in the driver rather than the core, so
> fbeeb822f6f4 ("PCI: Drop warning about drivers that don't use
> pci_set_master()") just dropped the warning.
>
> This should resolve the problem, so I don't think there's a need for
> this patch. Let me know if otherwise.
This is fine with me, all I really wanted was to get rid of the
warning. I am not aware of any problem either (but I am no PCI expert
by any means.)
Thanks,
--
Jean Delvare
Suse L3 Support
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-17 0:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-09 14:33 [PATCH] PCI/portdrv: Set master earlier Jean Delvare
2013-12-07 22:23 ` Bjorn Helgaas
2013-12-08 3:26 ` Yinghai Lu
2013-12-09 3:07 ` Benjamin Herrenschmidt
2013-12-17 0:53 ` Bjorn Helgaas
2013-12-08 13:09 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).