diff mbox

[v2,1/2] PCI: generic: remove dependency on hw_pci

Message ID 1430791333-26013-1-git-send-email-jchandra@broadcom.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jayachandran C. May 5, 2015, 2:02 a.m. UTC
The current code in pci-host-generic.c uses pci_common_init_dev()
from the arch/arm/ to do a part of the PCI initialization, and this
prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really
needed by pci-host-generic.c can be done in the same file without
using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
this is be handled by setting up 'struct gen_pci' to embed a
pci_sys_data variable as the first element on the ARM platform.

Signed-off-by: Jayachandran C <jchandra@broadcom.com>
---
Here's v2 of the patches, this enables use of pci-host-generic on
arm64.

This has been tested on both qemu and fast model for arm64, and on
qemu for arm32.

v1->v2
 - Address comments from Arnd Bergmann and Lorenzo Pieralisi
    - move contents of gen_pci_init to gen_pci_probe
    - assign resources only when !probe_only
 - tested on ARM32 with qemu option -M virt

Notes:
 - passing a zeroed out pci_sys_data for ARM looks ok, but I haven't
   tested it on ARM.
 - tested it on ARM64 fast model
 - Any information on how this can be tested on arm is welcome.
 - There is only one ifdef, and that can be removed when arm64 gets
   a sysdata, or when arm loses its sysdata.

 drivers/pci/host/pci-host-generic.c | 50 +++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 19 deletions(-)

Comments

Will Deacon May 5, 2015, 3:53 p.m. UTC | #1
On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> The current code in pci-host-generic.c uses pci_common_init_dev()
> from the arch/arm/ to do a part of the PCI initialization, and this
> prevents it from being used on arm64.
> 
> The initialization done by pci_common_init_dev() that is really
> needed by pci-host-generic.c can be done in the same file without
> using the hw_pci API of ARM.
> 
> The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> this is be handled by setting up 'struct gen_pci' to embed a
> pci_sys_data variable as the first element on the ARM platform.
> 
> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> ---
> Here's v2 of the patches, this enables use of pci-host-generic on
> arm64.
> 
> This has been tested on both qemu and fast model for arm64, and on
> qemu for arm32.
> 
> v1->v2
>  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
>     - move contents of gen_pci_init to gen_pci_probe
>     - assign resources only when !probe_only
>  - tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the
following errors (a 32-bit ARM kernel does seem to work):

  PCI host bridge /pci ranges:
     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000
  pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
  pci_bus 0000:00: root bus resource [bus 00-01]
  pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
  pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
  pci_bus 0000:00: scanning bus
  pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
  pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
  pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
  pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
  pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
  pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
  pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
  pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
  pci_bus 0000:00: fixups for bus
  pci_bus 0000:00: bus scan returning with max=00
  pci 0000:00:00.0: fixup irq: got 10
  pci 0000:00:00.0: assigning IRQ 10
  pci 0000:00:01.0: fixup irq: got 11
  pci 0000:00:01.0: assigning IRQ 11
  virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
  virtio-pci: probe of 0000:00:00.0 failed with error -22
  virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
  virtio-pci: probe of 0000:00:01.0 failed with error -22

Will
--
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
Arnd Bergmann May 5, 2015, 3:58 p.m. UTC | #2
On Tuesday 05 May 2015 16:53:46 Will Deacon wrote:
> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > The current code in pci-host-generic.c uses pci_common_init_dev()
> > from the arch/arm/ to do a part of the PCI initialization, and this
> > prevents it from being used on arm64.
> > 
> > The initialization done by pci_common_init_dev() that is really
> > needed by pci-host-generic.c can be done in the same file without
> > using the hw_pci API of ARM.
> > 
> > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > this is be handled by setting up 'struct gen_pci' to embed a
> > pci_sys_data variable as the first element on the ARM platform.
> > 
> > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > ---
> > Here's v2 of the patches, this enables use of pci-host-generic on
> > arm64.
> > 
> > This has been tested on both qemu and fast model for arm64, and on
> > qemu for arm32.
> > 
> > v1->v2
> >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> >     - move contents of gen_pci_init to gen_pci_probe
> >     - assign resources only when !probe_only
> >  - tested on ARM32 with qemu option -M virt
> 
> I tried this with an arm64 kernel running under kvmtool, but I get the
> following errors (a 32-bit ARM kernel does seem to work):
> 
>   PCI host bridge /pci ranges:
>      IO 0x00000000..0x0000ffff -> 0x00000000
>     MEM 0x41000000..0x7fffffff -> 0x41000000
>   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>   pci_bus 0000:00: root bus resource [bus 00-01]
>   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>   pci_bus 0000:00: scanning bus
>   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>   pci_bus 0000:00: fixups for bus
>   pci_bus 0000:00: bus scan returning with max=00
>   pci 0000:00:00.0: fixup irq: got 10
>   pci 0000:00:00.0: assigning IRQ 10
>   pci 0000:00:01.0: fixup irq: got 11
>   pci 0000:00:01.0: assigning IRQ 11
>   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>   virtio-pci: probe of 0000:00:00.0 failed with error -22
>   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>   virtio-pci: probe of 0000:00:01.0 failed with error -22

What do you see in /proc/ioport and /proc/iomem, respectively?

It sounds like the memory resource of the host did not get registered right.

	Arnd
--
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
Lorenzo Pieralisi May 5, 2015, 4:03 p.m. UTC | #3
On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > The current code in pci-host-generic.c uses pci_common_init_dev()
> > from the arch/arm/ to do a part of the PCI initialization, and this
> > prevents it from being used on arm64.
> > 
> > The initialization done by pci_common_init_dev() that is really
> > needed by pci-host-generic.c can be done in the same file without
> > using the hw_pci API of ARM.
> > 
> > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > this is be handled by setting up 'struct gen_pci' to embed a
> > pci_sys_data variable as the first element on the ARM platform.
> > 
> > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > ---
> > Here's v2 of the patches, this enables use of pci-host-generic on
> > arm64.
> > 
> > This has been tested on both qemu and fast model for arm64, and on
> > qemu for arm32.
> > 
> > v1->v2
> >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> >     - move contents of gen_pci_init to gen_pci_probe
> >     - assign resources only when !probe_only
> >  - tested on ARM32 with qemu option -M virt
> 
> I tried this with an arm64 kernel running under kvmtool, but I get the
> following errors (a 32-bit ARM kernel does seem to work):
> 
>   PCI host bridge /pci ranges:
>      IO 0x00000000..0x0000ffff -> 0x00000000
>     MEM 0x41000000..0x7fffffff -> 0x41000000
>   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>   pci_bus 0000:00: root bus resource [bus 00-01]
>   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>   pci_bus 0000:00: scanning bus
>   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>   pci_bus 0000:00: fixups for bus
>   pci_bus 0000:00: bus scan returning with max=00
>   pci 0000:00:00.0: fixup irq: got 10
>   pci 0000:00:00.0: assigning IRQ 10
>   pci 0000:00:01.0: fixup irq: got 11
>   pci 0000:00:01.0: assigning IRQ 11
>   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>   virtio-pci: probe of 0000:00:00.0 failed with error -22
>   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>   virtio-pci: probe of 0000:00:01.0 failed with error -22

Yes, I already mentioned on v1 and comment was not taken into account,
at the moment when we enable devices on arm64 we rely on the generic

pcibios_enable_device()

function that by enabling resources trigger that error on PROBE_ONLY
systems.

We could do what arm does, by checking the PROBE_ONLY flag
and call pci_enable_resources only if !PROBE_ONLY (and do that in the
generic drivers/pci/pci.c pcibios_enable_device function) but IIRC Bjorn
did not like that much.

Lorenzo
--
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
Lorenzo Pieralisi May 6, 2015, 2:18 p.m. UTC | #4
On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > The current code in pci-host-generic.c uses pci_common_init_dev()
> > from the arch/arm/ to do a part of the PCI initialization, and this
> > prevents it from being used on arm64.
> > 
> > The initialization done by pci_common_init_dev() that is really
> > needed by pci-host-generic.c can be done in the same file without
> > using the hw_pci API of ARM.
> > 
> > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > this is be handled by setting up 'struct gen_pci' to embed a
> > pci_sys_data variable as the first element on the ARM platform.
> > 
> > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > ---
> > Here's v2 of the patches, this enables use of pci-host-generic on
> > arm64.
> > 
> > This has been tested on both qemu and fast model for arm64, and on
> > qemu for arm32.
> > 
> > v1->v2
> >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> >     - move contents of gen_pci_init to gen_pci_probe
> >     - assign resources only when !probe_only
> >  - tested on ARM32 with qemu option -M virt
> 
> I tried this with an arm64 kernel running under kvmtool, but I get the
> following errors (a 32-bit ARM kernel does seem to work):
> 
>   PCI host bridge /pci ranges:
>      IO 0x00000000..0x0000ffff -> 0x00000000
>     MEM 0x41000000..0x7fffffff -> 0x41000000
>   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>   pci_bus 0000:00: root bus resource [bus 00-01]
>   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>   pci_bus 0000:00: scanning bus
>   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>   pci_bus 0000:00: fixups for bus
>   pci_bus 0000:00: bus scan returning with max=00
>   pci 0000:00:00.0: fixup irq: got 10
>   pci 0000:00:00.0: assigning IRQ 10
>   pci 0000:00:01.0: fixup irq: got 11
>   pci 0000:00:01.0: assigning IRQ 11
>   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>   virtio-pci: probe of 0000:00:00.0 failed with error -22
>   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>   virtio-pci: probe of 0000:00:01.0 failed with error -22

Ok, had a further look.

Referring to this thread:

https://lkml.org/lkml/2014/9/29/557

By looking at other architectures code, resources should be claimed
(ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC
seem to do that, in slightly different fashions.

I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used
to prevent enabling resources through a PCI command, which is what
pci_enable_resources does.

What we can do, is providing a generic PCI layer API that allows claiming
resources for a specific PCI bus, something similar if not identical
to what is done on alpha:

arch/alpha/kernel/pci.c pcibios_claim_one_bus()

that is not alpha specific at all. That way, we can use the API to claim
bus resources instead of assigning them on PCI_PROBE_ONLY (I *think*
that alpha calls pci_assign_unassigned_resources() even if
PCI_PROBE_ONLY is set, it should be safe since resources are claimed
first so IIUC the PCI layer would revert to FW BAR configuration on
assignment failure).

Bjorn, any opinion on this ? Putting together a patch is easy when
we agree on the solution.

Lorenzo
--
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 May 6, 2015, 3:18 p.m. UTC | #5
On Wed, May 6, 2015 at 9:18 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
>> On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
>> > The current code in pci-host-generic.c uses pci_common_init_dev()
>> > from the arch/arm/ to do a part of the PCI initialization, and this
>> > prevents it from being used on arm64.
>> >
>> > The initialization done by pci_common_init_dev() that is really
>> > needed by pci-host-generic.c can be done in the same file without
>> > using the hw_pci API of ARM.
>> >
>> > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
>> > this is be handled by setting up 'struct gen_pci' to embed a
>> > pci_sys_data variable as the first element on the ARM platform.
>> >
>> > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
>> > ---
>> > Here's v2 of the patches, this enables use of pci-host-generic on
>> > arm64.
>> >
>> > This has been tested on both qemu and fast model for arm64, and on
>> > qemu for arm32.
>> >
>> > v1->v2
>> >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
>> >     - move contents of gen_pci_init to gen_pci_probe
>> >     - assign resources only when !probe_only
>> >  - tested on ARM32 with qemu option -M virt
>>
>> I tried this with an arm64 kernel running under kvmtool, but I get the
>> following errors (a 32-bit ARM kernel does seem to work):
>>
>>   PCI host bridge /pci ranges:
>>      IO 0x00000000..0x0000ffff -> 0x00000000
>>     MEM 0x41000000..0x7fffffff -> 0x41000000
>>   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>>   pci_bus 0000:00: root bus resource [bus 00-01]
>>   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>>   pci_bus 0000:00: scanning bus
>>   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>>   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>>   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>>   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>>   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>>   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>>   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>>   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>>   pci_bus 0000:00: fixups for bus
>>   pci_bus 0000:00: bus scan returning with max=00
>>   pci 0000:00:00.0: fixup irq: got 10
>>   pci 0000:00:00.0: assigning IRQ 10
>>   pci 0000:00:01.0: fixup irq: got 11
>>   pci 0000:00:01.0: assigning IRQ 11
>>   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>>   virtio-pci: probe of 0000:00:00.0 failed with error -22
>>   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>>   virtio-pci: probe of 0000:00:01.0 failed with error -22
>
> Ok, had a further look.
>
> Referring to this thread:
>
> https://lkml.org/lkml/2014/9/29/557
>
> By looking at other architectures code, resources should be claimed
> (ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC
> seem to do that, in slightly different fashions.
>
> I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used
> to prevent enabling resources through a PCI command, which is what
> pci_enable_resources does.
>
> What we can do, is providing a generic PCI layer API that allows claiming
> resources for a specific PCI bus, something similar if not identical
> to what is done on alpha:
>
> arch/alpha/kernel/pci.c pcibios_claim_one_bus()
>
> that is not alpha specific at all. That way, we can use the API to claim
> bus resources instead of assigning them on PCI_PROBE_ONLY (I *think*
> that alpha calls pci_assign_unassigned_resources() even if
> PCI_PROBE_ONLY is set, it should be safe since resources are claimed
> first so IIUC the PCI layer would revert to FW BAR configuration on
> assignment failure).
>
> Bjorn, any opinion on this ? Putting together a patch is easy when
> we agree on the solution.

I would like claiming resources, i.e., pci_claim_resource(), to happen
in the core instead of in arch code because it's not inherently
arch-specific.  I don't think it should depend on PCI_PROBE_ONLY.

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 May 19, 2015, 11:09 p.m. UTC | #6
On Tue, May 12, 2015 at 05:37:47AM +0530, Jayachandran C. wrote:
> On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
> > On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > > The current code in pci-host-generic.c uses pci_common_init_dev()
> > > from the arch/arm/ to do a part of the PCI initialization, and this
> > > prevents it from being used on arm64.
> > > 
> > > The initialization done by pci_common_init_dev() that is really
> > > needed by pci-host-generic.c can be done in the same file without
> > > using the hw_pci API of ARM.
> > > 
> > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > > this is be handled by setting up 'struct gen_pci' to embed a
> > > pci_sys_data variable as the first element on the ARM platform.
> > > 
> > > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > > ---
> > > Here's v2 of the patches, this enables use of pci-host-generic on
> > > arm64.
> > > 
> > > This has been tested on both qemu and fast model for arm64, and on
> > > qemu for arm32.
> > > 
> > > v1->v2
> > >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> > >     - move contents of gen_pci_init to gen_pci_probe
> > >     - assign resources only when !probe_only
> > >  - tested on ARM32 with qemu option -M virt
> > 
> > I tried this with an arm64 kernel running under kvmtool, but I get the
> > following errors (a 32-bit ARM kernel does seem to work):
> > 
> >   PCI host bridge /pci ranges:
> >      IO 0x00000000..0x0000ffff -> 0x00000000
> >     MEM 0x41000000..0x7fffffff -> 0x41000000
> >   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
> >   pci_bus 0000:00: root bus resource [bus 00-01]
> >   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> >   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
> >   pci_bus 0000:00: scanning bus
> >   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
> >   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
> >   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
> >   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
> >   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
> >   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
> >   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
> >   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
> >   pci_bus 0000:00: fixups for bus
> >   pci_bus 0000:00: bus scan returning with max=00
> >   pci 0000:00:00.0: fixup irq: got 10
> >   pci 0000:00:00.0: assigning IRQ 10
> >   pci 0000:00:01.0: fixup irq: got 11
> >   pci 0000:00:01.0: assigning IRQ 11
> >   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
> >   virtio-pci: probe of 0000:00:00.0 failed with error -22
> >   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
> >   virtio-pci: probe of 0000:00:01.0 failed with error -22
> 
> PCI_PROBE_ONLY does not work yet for arm64, this will need further changes
> in either arm64 or pci as noted by Lorenzo. This is not due to a problem with
> this patch per se.
> 
> More importantly, this does not change the current arm32 behavior and adds
> support for arm64 for two very useful cases
>  - we can use "pci-host-ecam-generic" for a host bridge controller that is
>    initialized by firmware, without adding a custom driver.
>  - qemu virt platform works on arm64
> 
> If you think there are any furhter changes needed in this patchset please
> let me know.

I'm waiting for at least an ack from Will.

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
Will Deacon May 20, 2015, 5:29 p.m. UTC | #7
On Wed, May 20, 2015 at 12:09:18AM +0100, Bjorn Helgaas wrote:
> On Tue, May 12, 2015 at 05:37:47AM +0530, Jayachandran C. wrote:
> > On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
> > > On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > > > The current code in pci-host-generic.c uses pci_common_init_dev()
> > > > from the arch/arm/ to do a part of the PCI initialization, and this
> > > > prevents it from being used on arm64.
> > > > 
> > > > The initialization done by pci_common_init_dev() that is really
> > > > needed by pci-host-generic.c can be done in the same file without
> > > > using the hw_pci API of ARM.
> > > > 
> > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > > > this is be handled by setting up 'struct gen_pci' to embed a
> > > > pci_sys_data variable as the first element on the ARM platform.
> > > > 
> > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > > > ---
> > > > Here's v2 of the patches, this enables use of pci-host-generic on
> > > > arm64.
> > > > 
> > > > This has been tested on both qemu and fast model for arm64, and on
> > > > qemu for arm32.
> > > > 
> > > > v1->v2
> > > >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> > > >     - move contents of gen_pci_init to gen_pci_probe
> > > >     - assign resources only when !probe_only
> > > >  - tested on ARM32 with qemu option -M virt
> > > 
> > > I tried this with an arm64 kernel running under kvmtool, but I get the
> > > following errors (a 32-bit ARM kernel does seem to work):
> > > 
> > >   PCI host bridge /pci ranges:
> > >      IO 0x00000000..0x0000ffff -> 0x00000000
> > >     MEM 0x41000000..0x7fffffff -> 0x41000000
> > >   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
> > >   pci_bus 0000:00: root bus resource [bus 00-01]
> > >   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> > >   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
> > >   pci_bus 0000:00: scanning bus
> > >   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
> > >   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
> > >   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
> > >   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
> > >   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
> > >   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
> > >   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
> > >   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
> > >   pci_bus 0000:00: fixups for bus
> > >   pci_bus 0000:00: bus scan returning with max=00
> > >   pci 0000:00:00.0: fixup irq: got 10
> > >   pci 0000:00:00.0: assigning IRQ 10
> > >   pci 0000:00:01.0: fixup irq: got 11
> > >   pci 0000:00:01.0: assigning IRQ 11
> > >   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
> > >   virtio-pci: probe of 0000:00:00.0 failed with error -22
> > >   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
> > >   virtio-pci: probe of 0000:00:01.0 failed with error -22
> > 
> > PCI_PROBE_ONLY does not work yet for arm64, this will need further changes
> > in either arm64 or pci as noted by Lorenzo. This is not due to a problem with
> > this patch per se.
> > 
> > More importantly, this does not change the current arm32 behavior and adds
> > support for arm64 for two very useful cases
> >  - we can use "pci-host-ecam-generic" for a host bridge controller that is
> >    initialized by firmware, without adding a custom driver.
> >  - qemu virt platform works on arm64
> > 
> > If you think there are any furhter changes needed in this patchset please
> > let me know.
> 
> I'm waiting for at least an ack from Will.

I'm not too keen on the current state of this patch. Firstly, I'd really
like to see the PROBE_ONLY case addressed at the same time as this port
(Lorenzo and Suravee are working on this). Secondly, the hack to put
pci_sys_data as the first member of gen_pci is ugly at best and probably
makes it difficult to support MSIs on ARM.

Will
--
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 May 20, 2015, 8:46 p.m. UTC | #8
On Wed, May 20, 2015 at 12:29 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, May 20, 2015 at 12:09:18AM +0100, Bjorn Helgaas wrote:
>> On Tue, May 12, 2015 at 05:37:47AM +0530, Jayachandran C. wrote:
>> > On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
>> > > On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
>> > > > The current code in pci-host-generic.c uses pci_common_init_dev()
>> > > > from the arch/arm/ to do a part of the PCI initialization, and this
>> > > > prevents it from being used on arm64.
>> > > >
>> > > > The initialization done by pci_common_init_dev() that is really
>> > > > needed by pci-host-generic.c can be done in the same file without
>> > > > using the hw_pci API of ARM.
>> > > >
>> > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
>> > > > this is be handled by setting up 'struct gen_pci' to embed a
>> > > > pci_sys_data variable as the first element on the ARM platform.
>> > > >
>> > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
>> > > > ---
>> > > > Here's v2 of the patches, this enables use of pci-host-generic on
>> > > > arm64.
>> > > >
>> > > > This has been tested on both qemu and fast model for arm64, and on
>> > > > qemu for arm32.
>> > > >
>> > > > v1->v2
>> > > >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
>> > > >     - move contents of gen_pci_init to gen_pci_probe
>> > > >     - assign resources only when !probe_only
>> > > >  - tested on ARM32 with qemu option -M virt
>> > >
>> > > I tried this with an arm64 kernel running under kvmtool, but I get the
>> > > following errors (a 32-bit ARM kernel does seem to work):
>> > >
>> > >   PCI host bridge /pci ranges:
>> > >      IO 0x00000000..0x0000ffff -> 0x00000000
>> > >     MEM 0x41000000..0x7fffffff -> 0x41000000
>> > >   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
>> > >   pci_bus 0000:00: root bus resource [bus 00-01]
>> > >   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>> > >   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
>> > >   pci_bus 0000:00: scanning bus
>> > >   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
>> > >   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
>> > >   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
>> > >   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
>> > >   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
>> > >   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
>> > >   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
>> > >   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
>> > >   pci_bus 0000:00: fixups for bus
>> > >   pci_bus 0000:00: bus scan returning with max=00
>> > >   pci 0000:00:00.0: fixup irq: got 10
>> > >   pci 0000:00:00.0: assigning IRQ 10
>> > >   pci 0000:00:01.0: fixup irq: got 11
>> > >   pci 0000:00:01.0: assigning IRQ 11
>> > >   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
>> > >   virtio-pci: probe of 0000:00:00.0 failed with error -22
>> > >   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
>> > >   virtio-pci: probe of 0000:00:01.0 failed with error -22
>> >
>> > PCI_PROBE_ONLY does not work yet for arm64, this will need further changes
>> > in either arm64 or pci as noted by Lorenzo. This is not due to a problem with
>> > this patch per se.
>> >
>> > More importantly, this does not change the current arm32 behavior and adds
>> > support for arm64 for two very useful cases
>> >  - we can use "pci-host-ecam-generic" for a host bridge controller that is
>> >    initialized by firmware, without adding a custom driver.
>> >  - qemu virt platform works on arm64
>> >
>> > If you think there are any furhter changes needed in this patchset please
>> > let me know.
>>
>> I'm waiting for at least an ack from Will.
>
> I'm not too keen on the current state of this patch. Firstly, I'd really
> like to see the PROBE_ONLY case addressed at the same time as this port
> (Lorenzo and Suravee are working on this). Secondly, the hack to put
> pci_sys_data as the first member of gen_pci is ugly at best and probably
> makes it difficult to support MSIs on ARM.

Thanks, Will.  I'll wait for another iteration that addresses these.

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
Will Deacon May 26, 2015, 9:59 a.m. UTC | #9
On Thu, May 21, 2015 at 07:37:30AM +0100, Jayachandran C. wrote:
> On Wed, May 20, 2015 at 06:29:03PM +0100, Will Deacon wrote:
> > On Wed, May 20, 2015 at 12:09:18AM +0100, Bjorn Helgaas wrote:
> > > On Tue, May 12, 2015 at 05:37:47AM +0530, Jayachandran C. wrote:
> > > > On Tue, May 05, 2015 at 04:53:46PM +0100, Will Deacon wrote:
> > > > > On Tue, May 05, 2015 at 03:02:12AM +0100, Jayachandran C wrote:
> > > > > > The current code in pci-host-generic.c uses pci_common_init_dev()
> > > > > > from the arch/arm/ to do a part of the PCI initialization, and this
> > > > > > prevents it from being used on arm64.
> > > > > > 
> > > > > > The initialization done by pci_common_init_dev() that is really
> > > > > > needed by pci-host-generic.c can be done in the same file without
> > > > > > using the hw_pci API of ARM.
> > > > > > 
> > > > > > The ARM platform requires a pci_sys_data as sysdata for the PCI bus,
> > > > > > this is be handled by setting up 'struct gen_pci' to embed a
> > > > > > pci_sys_data variable as the first element on the ARM platform.
> > > > > > 
> > > > > > Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> > > > > > ---
> > > > > > Here's v2 of the patches, this enables use of pci-host-generic on
> > > > > > arm64.
> > > > > > 
> > > > > > This has been tested on both qemu and fast model for arm64, and on
> > > > > > qemu for arm32.
> > > > > > 
> > > > > > v1->v2
> > > > > >  - Address comments from Arnd Bergmann and Lorenzo Pieralisi
> > > > > >     - move contents of gen_pci_init to gen_pci_probe
> > > > > >     - assign resources only when !probe_only
> > > > > >  - tested on ARM32 with qemu option -M virt
> > > > > 
> > > > > I tried this with an arm64 kernel running under kvmtool, but I get the
> > > > > following errors (a 32-bit ARM kernel does seem to work):
> > > > > 
> > > > >   PCI host bridge /pci ranges:
> > > > >      IO 0x00000000..0x0000ffff -> 0x00000000
> > > > >     MEM 0x41000000..0x7fffffff -> 0x41000000
> > > > >   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
> > > > >   pci_bus 0000:00: root bus resource [bus 00-01]
> > > > >   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> > > > >   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
> > > > >   pci_bus 0000:00: scanning bus
> > > > >   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
> > > > >   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
> > > > >   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
> > > > >   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
> > > > >   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
> > > > >   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
> > > > >   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
> > > > >   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
> > > > >   pci_bus 0000:00: fixups for bus
> > > > >   pci_bus 0000:00: bus scan returning with max=00
> > > > >   pci 0000:00:00.0: fixup irq: got 10
> > > > >   pci 0000:00:00.0: assigning IRQ 10
> > > > >   pci 0000:00:01.0: fixup irq: got 11
> > > > >   pci 0000:00:01.0: assigning IRQ 11
> > > > >   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
> > > > >   virtio-pci: probe of 0000:00:00.0 failed with error -22
> > > > >   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
> > > > >   virtio-pci: probe of 0000:00:01.0 failed with error -22
> > > > 
> > > > PCI_PROBE_ONLY does not work yet for arm64, this will need further changes
> > > > in either arm64 or pci as noted by Lorenzo. This is not due to a problem with
> > > > this patch per se.
> > > > 
> > > > More importantly, this does not change the current arm32 behavior and adds
> > > > support for arm64 for two very useful cases
> > > >  - we can use "pci-host-ecam-generic" for a host bridge controller that is
> > > >    initialized by firmware, without adding a custom driver.
> > > >  - qemu virt platform works on arm64
> > > > 
> > > > If you think there are any furhter changes needed in this patchset please
> > > > let me know.
> > > 
> > > I'm waiting for at least an ack from Will.
> > 
> > I'm not too keen on the current state of this patch. Firstly, I'd really
> > like to see the PROBE_ONLY case addressed at the same time as this port
> > (Lorenzo and Suravee are working on this).
> 
> Fixing the PROBE-ONLY for arm64 is probably unrelated to the scope of this
> patch - so I would hope that we don't NACK this patch due to that issue. This
> patch is very useful without PROBE-ONLY like I noted above.

I'm not NACKing anything, I just don't see the benefit of merging half a
solution when the other half is in active development. Perhaps you could
review Lorenzo's patches to help things along?

> > Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly
> > at best
> 
> On balance, it removes the problem of using hw_pci interface which has its own
> ugliness (Arnd's mail on this http://www.spinics.net/lists/linux-pci/msg40703.html)

To me, that sounds like we should be disposing of the hw_pci interface
altogether and moving arch/arm/ over to the new architecture-independent
interface.

> > and probably makes it difficult to support MSIs on ARM.
> 
> Adding parsing of msi-parent and setting it up for arm32 and arm64 can be
> added fairly easily to the current code, I can post an RFC patch for this.

I think that would be a good addition to the series.

Will
--
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
Arnd Bergmann May 26, 2015, 10:38 a.m. UTC | #10
On Tuesday 26 May 2015 10:59:24 Will Deacon wrote:
> > > Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly
> > > at best
> > 
> > On balance, it removes the problem of using hw_pci interface which has its own
> > ugliness (Arnd's mail on this http://www.spinics.net/lists/linux-pci/msg40703.html)
> 
> To me, that sounds like we should be disposing of the hw_pci interface
> altogether and moving arch/arm/ over to the new architecture-independent
> interface.

I'd like to do that for all drivers in drivers/pci/host, but leave the
ones for the legacy platforms in arch/arm alone, in particular the ones
that use nr_controllers != 1.

We should be able to remove pci_common_init_dev() and directly probe
all devices that are using that, while we keep pci_common_init()
around for the legacy case that also does not have a parent device.

	Arnd
--
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
diff mbox

Patch

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index ba46e58..e9cc559 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -39,6 +39,9 @@  struct gen_pci_cfg_windows {
 };
 
 struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
 	struct pci_host_bridge			host;
 	struct gen_pci_cfg_windows		cfg;
 	struct list_head			resources;
@@ -48,8 +51,7 @@  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);
@@ -64,8 +66,7 @@  static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = bus->sysdata;
 	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);
@@ -198,13 +199,6 @@  static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
-{
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
 	int err;
@@ -214,13 +208,7 @@  static int gen_pci_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
 
 	if (!pci)
 		return -ENOMEM;
@@ -258,7 +246,31 @@  static int gen_pci_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	pci_common_init_dev(dev, &hw);
+	/* do not reassign resource if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC);
+
+	bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, pci, &pci->resources);
+	if (!bus) {
+		dev_err(dev, "Scanning rootbus failed");
+		return -ENODEV;
+	}
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+	}
+	pci_bus_add_devices(bus);
+
+	/* Configure PCI Express settings */
+	if (pci_has_flag(PCI_PROBE_ONLY)) {
+		struct pci_bus *child;
+
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
+	}
 	return 0;
 }