Re: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-04 Thread Arnd Bergmann
On Monday 03 February 2014, Jason Gunthorpe wrote:
> On Mon, Feb 03, 2014 at 09:12:32PM +0100, Arnd Bergmann wrote:
> 
> > Are you sure that is true for the root bridge as well? I don't
> > remember the details, but I though that for the host bridge,
> > we don't actually look at the BARs at all.
> 
> That is right, but this isn't a host bridge device, it is a PCI-PCI
> bridge with root complex registers. The root complex bridge is not the
> same as the host bridge.
> 
> Unfortunately the implementation is non-conforming. :(

Ok, I see. I was probably asking the wrong question then earlier
when I tried to find out what this is.

Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-04 Thread Arnd Bergmann
On Monday 03 February 2014, Jason Gunthorpe wrote:
 On Mon, Feb 03, 2014 at 09:12:32PM +0100, Arnd Bergmann wrote:
 
  Are you sure that is true for the root bridge as well? I don't
  remember the details, but I though that for the host bridge,
  we don't actually look at the BARs at all.
 
 That is right, but this isn't a host bridge device, it is a PCI-PCI
 bridge with root complex registers. The root complex bridge is not the
 same as the host bridge.
 
 Unfortunately the implementation is non-conforming. :(

Ok, I see. I was probably asking the wrong question then earlier
when I tried to find out what this is.

Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Jason Gunthorpe
On Mon, Feb 03, 2014 at 09:12:32PM +0100, Arnd Bergmann wrote:

> Are you sure that is true for the root bridge as well? I don't
> remember the details, but I though that for the host bridge,
> we don't actually look at the BARs at all.

That is right, but this isn't a host bridge device, it is a PCI-PCI
bridge with root complex registers. The root complex bridge is not the
same as the host bridge.

Unfortunately the implementation is non-conforming. :(

Jason
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Tanmay Inamdar
On Mon, Feb 3, 2014 at 12:12 PM, Arnd Bergmann  wrote:
> On Monday 03 February 2014 11:42:22 Tanmay Inamdar wrote:
>> On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann  wrote:
>> > On Friday 24 January 2014, Tanmay Inamdar wrote:
>> >
>> >> +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
>> >> +{
>> >> + int i;
>> >> +
>> >> + /* Hide the PCI host BARs from the kernel as their content doesn't
>> >> +  * fit well in the resource management
>> >> +  */
>> >> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>> >> + dev->resource[i].start = dev->resource[i].end = 0;
>> >> + dev->resource[i].flags = 0;
>> >> + }
>> >> + dev_info(>dev, "Hiding X-Gene pci host bridge resources %s\n",
>> >> +  pci_name(dev));
>> >> +}
>> >> +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
>> >> +  xgene_pcie_fixup_bridge);
>> >
>> > Shouldn't this be gone now that the host bridge is correctly shown
>> > at the domain root?
>>
>> In inbound region configuration, whole DDR space is mapped into the
>> BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
>> size of BAR of RC and tries to fit it into the memory resource. First
>> thing is that the outbound memory is not enough to map the inbound BAR
>> space. This creates problem with the resource management logic and
>> second thing is that, it is not required to map inbound BAR space RC
>> bar as no one will be accessing it further.
>>
>> As Jason suggested, Bridge BAR's should be 0 size unless the bridge
>> itself has registers. However this is not the case with XGene PCIe
>> controller. It may have been inherited from the legacy design.
>> 'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.
>
> Are you sure that is true for the root bridge as well? I don't
> remember the details, but I though that for the host bridge,
> we don't actually look at the BARs at all.
>
>> > If you want to try out the I/O space, I'd suggest using an Intel
>> > e1000 network card, which has both memory and i/o space. There
>> > is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
>> > that lets you check the I/O registers on it, or you can go
>> > through /dev/port from user space.
>> >
>> > I also haven't seen your patch that adds pci_ioremap_io() for
>> > arm64. It would be helpful to keep it in the same patch
>> > series, since it won't build without this patch.
>>
>> I will post the arm64 pci patch along with next revision of this
>> driver. That will cover the 'pci_ioremap_io' as well.
>
> Please note that today, Liviu Dudau has also posted patches for this,
> so you should coordinate a bit.

Yes. Just looking at his patches. Looks similar to what I have. I will
wait till your solution shapes up.
>
> Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Arnd Bergmann
On Monday 03 February 2014 11:42:22 Tanmay Inamdar wrote:
> On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann  wrote:
> > On Friday 24 January 2014, Tanmay Inamdar wrote:
> >
> >> +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
> >> +{
> >> + int i;
> >> +
> >> + /* Hide the PCI host BARs from the kernel as their content doesn't
> >> +  * fit well in the resource management
> >> +  */
> >> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> >> + dev->resource[i].start = dev->resource[i].end = 0;
> >> + dev->resource[i].flags = 0;
> >> + }
> >> + dev_info(>dev, "Hiding X-Gene pci host bridge resources %s\n",
> >> +  pci_name(dev));
> >> +}
> >> +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
> >> +  xgene_pcie_fixup_bridge);
> >
> > Shouldn't this be gone now that the host bridge is correctly shown
> > at the domain root?
> 
> In inbound region configuration, whole DDR space is mapped into the
> BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
> size of BAR of RC and tries to fit it into the memory resource. First
> thing is that the outbound memory is not enough to map the inbound BAR
> space. This creates problem with the resource management logic and
> second thing is that, it is not required to map inbound BAR space RC
> bar as no one will be accessing it further.
> 
> As Jason suggested, Bridge BAR's should be 0 size unless the bridge
> itself has registers. However this is not the case with XGene PCIe
> controller. It may have been inherited from the legacy design.
> 'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.

Are you sure that is true for the root bridge as well? I don't
remember the details, but I though that for the host bridge,
we don't actually look at the BARs at all.

> > If you want to try out the I/O space, I'd suggest using an Intel
> > e1000 network card, which has both memory and i/o space. There
> > is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
> > that lets you check the I/O registers on it, or you can go
> > through /dev/port from user space.
> >
> > I also haven't seen your patch that adds pci_ioremap_io() for
> > arm64. It would be helpful to keep it in the same patch
> > series, since it won't build without this patch.
> 
> I will post the arm64 pci patch along with next revision of this
> driver. That will cover the 'pci_ioremap_io' as well.

Please note that today, Liviu Dudau has also posted patches for this,
so you should coordinate a bit.

Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Tanmay Inamdar
On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann  wrote:
> On Friday 24 January 2014, Tanmay Inamdar wrote:
>
>> +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
>> +{
>> + int i;
>> +
>> + /* Hide the PCI host BARs from the kernel as their content doesn't
>> +  * fit well in the resource management
>> +  */
>> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>> + dev->resource[i].start = dev->resource[i].end = 0;
>> + dev->resource[i].flags = 0;
>> + }
>> + dev_info(>dev, "Hiding X-Gene pci host bridge resources %s\n",
>> +  pci_name(dev));
>> +}
>> +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
>> +  xgene_pcie_fixup_bridge);
>
> Shouldn't this be gone now that the host bridge is correctly shown
> at the domain root?

In inbound region configuration, whole DDR space is mapped into the
BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
size of BAR of RC and tries to fit it into the memory resource. First
thing is that the outbound memory is not enough to map the inbound BAR
space. This creates problem with the resource management logic and
second thing is that, it is not required to map inbound BAR space RC
bar as no one will be accessing it further.

As Jason suggested, Bridge BAR's should be 0 size unless the bridge
itself has registers. However this is not the case with XGene PCIe
controller. It may have been inherited from the legacy design.
'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.

>
>> +static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
>> +{
>> + struct xgene_pcie_port *pp = sys->private_data;
>> + struct resource *io = >realio;
>> +
>> + io->start = sys->domain * SZ_64K;
>> + io->end = io->start + SZ_64K;
>> + io->flags = pp->io.res.flags;
>> + io->name = "PCI IO";
>> + pci_ioremap_io(io->start, pp->io.res.start);
>> +
>> + pci_add_resource_offset(>resources, io, sys->io_offset);
>> + sys->mem_offset = pp->mem.res.start - pp->mem.pci_addr;
>> + pci_add_resource_offset(>resources, >mem.res,
>> + sys->mem_offset);
>> + return 1;
>> +}
>
> Thanks for bringing back the I/O space handling.
>
> You don't seem to set sys->io_offset anywhere, but each of the
> ports listed in your DT starts a local bus I/O register range
> at port 0.
>
> AFAICT, you need to add (somewhere)
>
> sys->io_offset = pp->realio.start - pp->io.pci_addr;
>
> but there could be something else missing. You clearly haven't
> tested if the I/O space actually works.

That is correct :-). Could not find the card. Thanks for the patch below.
>
> If you want to try out the I/O space, I'd suggest using an Intel
> e1000 network card, which has both memory and i/o space. There
> is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
> that lets you check the I/O registers on it, or you can go
> through /dev/port from user space.
>
> I also haven't seen your patch that adds pci_ioremap_io() for
> arm64. It would be helpful to keep it in the same patch
> series, since it won't build without this patch.

I will post the arm64 pci patch along with next revision of this
driver. That will cover the 'pci_ioremap_io' as well.

>
> Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Tanmay Inamdar
On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann a...@arndb.de wrote:
 On Friday 24 January 2014, Tanmay Inamdar wrote:

 +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
 +{
 + int i;
 +
 + /* Hide the PCI host BARs from the kernel as their content doesn't
 +  * fit well in the resource management
 +  */
 + for (i = 0; i  DEVICE_COUNT_RESOURCE; i++) {
 + dev-resource[i].start = dev-resource[i].end = 0;
 + dev-resource[i].flags = 0;
 + }
 + dev_info(dev-dev, Hiding X-Gene pci host bridge resources %s\n,
 +  pci_name(dev));
 +}
 +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
 +  xgene_pcie_fixup_bridge);

 Shouldn't this be gone now that the host bridge is correctly shown
 at the domain root?

In inbound region configuration, whole DDR space is mapped into the
BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
size of BAR of RC and tries to fit it into the memory resource. First
thing is that the outbound memory is not enough to map the inbound BAR
space. This creates problem with the resource management logic and
second thing is that, it is not required to map inbound BAR space RC
bar as no one will be accessing it further.

As Jason suggested, Bridge BAR's should be 0 size unless the bridge
itself has registers. However this is not the case with XGene PCIe
controller. It may have been inherited from the legacy design.
'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.


 +static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
 +{
 + struct xgene_pcie_port *pp = sys-private_data;
 + struct resource *io = pp-realio;
 +
 + io-start = sys-domain * SZ_64K;
 + io-end = io-start + SZ_64K;
 + io-flags = pp-io.res.flags;
 + io-name = PCI IO;
 + pci_ioremap_io(io-start, pp-io.res.start);
 +
 + pci_add_resource_offset(sys-resources, io, sys-io_offset);
 + sys-mem_offset = pp-mem.res.start - pp-mem.pci_addr;
 + pci_add_resource_offset(sys-resources, pp-mem.res,
 + sys-mem_offset);
 + return 1;
 +}

 Thanks for bringing back the I/O space handling.

 You don't seem to set sys-io_offset anywhere, but each of the
 ports listed in your DT starts a local bus I/O register range
 at port 0.

 AFAICT, you need to add (somewhere)

 sys-io_offset = pp-realio.start - pp-io.pci_addr;

 but there could be something else missing. You clearly haven't
 tested if the I/O space actually works.

That is correct :-). Could not find the card. Thanks for the patch below.

 If you want to try out the I/O space, I'd suggest using an Intel
 e1000 network card, which has both memory and i/o space. There
 is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
 that lets you check the I/O registers on it, or you can go
 through /dev/port from user space.

 I also haven't seen your patch that adds pci_ioremap_io() for
 arm64. It would be helpful to keep it in the same patch
 series, since it won't build without this patch.

I will post the arm64 pci patch along with next revision of this
driver. That will cover the 'pci_ioremap_io' as well.


 Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Arnd Bergmann
On Monday 03 February 2014 11:42:22 Tanmay Inamdar wrote:
 On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann a...@arndb.de wrote:
  On Friday 24 January 2014, Tanmay Inamdar wrote:
 
  +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
  +{
  + int i;
  +
  + /* Hide the PCI host BARs from the kernel as their content doesn't
  +  * fit well in the resource management
  +  */
  + for (i = 0; i  DEVICE_COUNT_RESOURCE; i++) {
  + dev-resource[i].start = dev-resource[i].end = 0;
  + dev-resource[i].flags = 0;
  + }
  + dev_info(dev-dev, Hiding X-Gene pci host bridge resources %s\n,
  +  pci_name(dev));
  +}
  +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
  +  xgene_pcie_fixup_bridge);
 
  Shouldn't this be gone now that the host bridge is correctly shown
  at the domain root?
 
 In inbound region configuration, whole DDR space is mapped into the
 BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
 size of BAR of RC and tries to fit it into the memory resource. First
 thing is that the outbound memory is not enough to map the inbound BAR
 space. This creates problem with the resource management logic and
 second thing is that, it is not required to map inbound BAR space RC
 bar as no one will be accessing it further.
 
 As Jason suggested, Bridge BAR's should be 0 size unless the bridge
 itself has registers. However this is not the case with XGene PCIe
 controller. It may have been inherited from the legacy design.
 'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.

Are you sure that is true for the root bridge as well? I don't
remember the details, but I though that for the host bridge,
we don't actually look at the BARs at all.

  If you want to try out the I/O space, I'd suggest using an Intel
  e1000 network card, which has both memory and i/o space. There
  is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
  that lets you check the I/O registers on it, or you can go
  through /dev/port from user space.
 
  I also haven't seen your patch that adds pci_ioremap_io() for
  arm64. It would be helpful to keep it in the same patch
  series, since it won't build without this patch.
 
 I will post the arm64 pci patch along with next revision of this
 driver. That will cover the 'pci_ioremap_io' as well.

Please note that today, Liviu Dudau has also posted patches for this,
so you should coordinate a bit.

Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Tanmay Inamdar
On Mon, Feb 3, 2014 at 12:12 PM, Arnd Bergmann a...@arndb.de wrote:
 On Monday 03 February 2014 11:42:22 Tanmay Inamdar wrote:
 On Thu, Jan 30, 2014 at 6:16 AM, Arnd Bergmann a...@arndb.de wrote:
  On Friday 24 January 2014, Tanmay Inamdar wrote:
 
  +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
  +{
  + int i;
  +
  + /* Hide the PCI host BARs from the kernel as their content doesn't
  +  * fit well in the resource management
  +  */
  + for (i = 0; i  DEVICE_COUNT_RESOURCE; i++) {
  + dev-resource[i].start = dev-resource[i].end = 0;
  + dev-resource[i].flags = 0;
  + }
  + dev_info(dev-dev, Hiding X-Gene pci host bridge resources %s\n,
  +  pci_name(dev));
  +}
  +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
  +  xgene_pcie_fixup_bridge);
 
  Shouldn't this be gone now that the host bridge is correctly shown
  at the domain root?

 In inbound region configuration, whole DDR space is mapped into the
 BAR of RC. When Linux PCI mid-layer starts enumerating, it reads the
 size of BAR of RC and tries to fit it into the memory resource. First
 thing is that the outbound memory is not enough to map the inbound BAR
 space. This creates problem with the resource management logic and
 second thing is that, it is not required to map inbound BAR space RC
 bar as no one will be accessing it further.

 As Jason suggested, Bridge BAR's should be 0 size unless the bridge
 itself has registers. However this is not the case with XGene PCIe
 controller. It may have been inherited from the legacy design.
 'arch/powerpc/sysdev/ppc4xx_pci.c' has similar fixup function.

 Are you sure that is true for the root bridge as well? I don't
 remember the details, but I though that for the host bridge,
 we don't actually look at the BARs at all.

  If you want to try out the I/O space, I'd suggest using an Intel
  e1000 network card, which has both memory and i/o space. There
  is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
  that lets you check the I/O registers on it, or you can go
  through /dev/port from user space.
 
  I also haven't seen your patch that adds pci_ioremap_io() for
  arm64. It would be helpful to keep it in the same patch
  series, since it won't build without this patch.

 I will post the arm64 pci patch along with next revision of this
 driver. That will cover the 'pci_ioremap_io' as well.

 Please note that today, Liviu Dudau has also posted patches for this,
 so you should coordinate a bit.

Yes. Just looking at his patches. Looks similar to what I have. I will
wait till your solution shapes up.

 Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-02-03 Thread Jason Gunthorpe
On Mon, Feb 03, 2014 at 09:12:32PM +0100, Arnd Bergmann wrote:

 Are you sure that is true for the root bridge as well? I don't
 remember the details, but I though that for the host bridge,
 we don't actually look at the BARs at all.

That is right, but this isn't a host bridge device, it is a PCI-PCI
bridge with root complex registers. The root complex bridge is not the
same as the host bridge.

Unfortunately the implementation is non-conforming. :(

Jason
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-01-30 Thread Arnd Bergmann
On Friday 24 January 2014, Tanmay Inamdar wrote:

> +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
> +{
> + int i;
> +
> + /* Hide the PCI host BARs from the kernel as their content doesn't
> +  * fit well in the resource management
> +  */
> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> + dev->resource[i].start = dev->resource[i].end = 0;
> + dev->resource[i].flags = 0;
> + }
> + dev_info(>dev, "Hiding X-Gene pci host bridge resources %s\n",
> +  pci_name(dev));
> +}
> +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
> +  xgene_pcie_fixup_bridge);

Shouldn't this be gone now that the host bridge is correctly shown
at the domain root?

> +static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
> +{
> + struct xgene_pcie_port *pp = sys->private_data;
> + struct resource *io = >realio;
> +
> + io->start = sys->domain * SZ_64K;
> + io->end = io->start + SZ_64K;
> + io->flags = pp->io.res.flags;
> + io->name = "PCI IO";
> + pci_ioremap_io(io->start, pp->io.res.start);
> +
> + pci_add_resource_offset(>resources, io, sys->io_offset);
> + sys->mem_offset = pp->mem.res.start - pp->mem.pci_addr;
> + pci_add_resource_offset(>resources, >mem.res,
> + sys->mem_offset);
> + return 1;
> +}

Thanks for bringing back the I/O space handling.

You don't seem to set sys->io_offset anywhere, but each of the
ports listed in your DT starts a local bus I/O register range
at port 0.

AFAICT, you need to add (somewhere)

sys->io_offset = pp->realio.start - pp->io.pci_addr;

but there could be something else missing. You clearly haven't
tested if the I/O space actually works.

If you want to try out the I/O space, I'd suggest using an Intel
e1000 network card, which has both memory and i/o space. There
is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
that lets you check the I/O registers on it, or you can go
through /dev/port from user space.

I also haven't seen your patch that adds pci_ioremap_io() for
arm64. It would be helpful to keep it in the same patch
series, since it won't build without this patch.

Arnd
--
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: [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver

2014-01-30 Thread Arnd Bergmann
On Friday 24 January 2014, Tanmay Inamdar wrote:

 +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
 +{
 + int i;
 +
 + /* Hide the PCI host BARs from the kernel as their content doesn't
 +  * fit well in the resource management
 +  */
 + for (i = 0; i  DEVICE_COUNT_RESOURCE; i++) {
 + dev-resource[i].start = dev-resource[i].end = 0;
 + dev-resource[i].flags = 0;
 + }
 + dev_info(dev-dev, Hiding X-Gene pci host bridge resources %s\n,
 +  pci_name(dev));
 +}
 +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
 +  xgene_pcie_fixup_bridge);

Shouldn't this be gone now that the host bridge is correctly shown
at the domain root?

 +static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
 +{
 + struct xgene_pcie_port *pp = sys-private_data;
 + struct resource *io = pp-realio;
 +
 + io-start = sys-domain * SZ_64K;
 + io-end = io-start + SZ_64K;
 + io-flags = pp-io.res.flags;
 + io-name = PCI IO;
 + pci_ioremap_io(io-start, pp-io.res.start);
 +
 + pci_add_resource_offset(sys-resources, io, sys-io_offset);
 + sys-mem_offset = pp-mem.res.start - pp-mem.pci_addr;
 + pci_add_resource_offset(sys-resources, pp-mem.res,
 + sys-mem_offset);
 + return 1;
 +}

Thanks for bringing back the I/O space handling.

You don't seem to set sys-io_offset anywhere, but each of the
ports listed in your DT starts a local bus I/O register range
at port 0.

AFAICT, you need to add (somewhere)

sys-io_offset = pp-realio.start - pp-io.pci_addr;

but there could be something else missing. You clearly haven't
tested if the I/O space actually works.

If you want to try out the I/O space, I'd suggest using an Intel
e1000 network card, which has both memory and i/o space. There
is a patch at http://www.spinics.net/lists/linux-pci/msg27684.html
that lets you check the I/O registers on it, or you can go
through /dev/port from user space.

I also haven't seen your patch that adds pci_ioremap_io() for
arm64. It would be helpful to keep it in the same patch
series, since it won't build without this patch.

Arnd
--
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/