Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Wed, May 7, 2014 at 8:05 PM, Arnd Bergmann wrote:
> On Wednesday 07 May 2014 17:21:13 Srikanth Thokala wrote:
>> On Wed, Apr 30, 2014 at 9:04 PM, Arnd Bergmann wrote:
>> > On Tuesday 15 April 2014, Srikanth Thokala wrote:
>> >> +/**
>> >> + * xilinx_pcie_get_config_base - Get configuration base
>> >> + * @bus: Bus structure of current bus
>> >> + * @devfn: Device/function
>> >> + * @where: Offset from base
>> >> + *
>> >> + * Return: Base address of the configuration space needed to be
>> >> + * accessed.
>> >> + */
>> >> +static void __iomem *xilinx_pcie_get_config_base(struct pci_bus *bus,
>> >> + unsigned int devfn,
>> >> + int where)
>> >> +{
>> >> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> >> + int relbus;
>> >> +
>> >> + relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
>> >> + (devfn << ECAM_DEV_NUM_SHIFT);
>> >> +
>> >> + return port->reg_base + relbus + where;
>> >> +}
>> >
>> > Does this mean you have an ECAM-compliant config space? Nice!
>> >
>> > Would it be possible to split the config space access out into
>> > a separate file? It would be nice to share that with the generic
>> > ECAM driver that Will Deacon has sent.
>>
>> Yes, it should be possible. Is it ok, if I work on top of this driver?
>
> Do you mean as a follow-on patch? My feeling is that since we are trying
> to merge both for 3.16, it would be good to get it done right away if
> it doesn't cause too much extra work.
Sure, I will work with Will and let you know.
>
>> >> +/**
>> >> + * xilinx_pcie_enable_msi - Enable MSI support
>> >> + * @port: PCIe port information
>> >> + */
>> >> +static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
>> >> +{
>> >> + port->msg_addr = __get_free_pages(GFP_KERNEL, 0);
>> >> +
>> >> + pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
>> >> + pcie_write(port, virt_to_phys((void *)port->msg_addr),
>> >> +XILINX_PCIE_REG_MSIBASE2);
>> >> +}
>> >
>> > As a general comment about the MSI implementation, I wonder if this is
>> > actually
>> > generic enough to be shared with other host controllers. It could be moved
>> > into a separate file like the config space access in that case.
>>
>> I feel the MSI implementation is not generic by looking into the other
>> host controllers,
>> it is more specific to the hardware. Correct me, if am wrong.
>
> The other host controllers are certainly incompatible, but this one looks
> like it could be used on other controllers easily.
>
> Splitting it out would also make it easier to use another MSI implementation
> like the one in the GIC.
I need to look into this and I will come back to you.
>
>
>> >> + /* Register the device */
>> >> + pci_common_init_dev(dev, &hw);
>> >> +
>> >> + platform_set_drvdata(pdev, port);
>> >
>> > Don't you have to do the platform_set_drvdata() before
>> > pci_common_init_dev()?
>>
>> It should be fine, as I don't see any dependencies.
>
> Ah, it's only used in the remove function. It looks correct then, but I think
> it would be better to set it first anyway, in case another function starts
> using
> the drvdata later and that function may get called by the PCI initialization.
Ok.
Srikanth
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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 v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Wed, May 07 2014 at 3:53:27 pm BST, Will Deacon wrote:
> Hi all,
>
> Thanks for CC'ing me, Arnd.
>
> On Wed, May 07, 2014 at 03:35:48PM +0100, Arnd Bergmann wrote:
>> On Wednesday 07 May 2014 17:21:13 Srikanth Thokala wrote:
>> > On Wed, Apr 30, 2014 at 9:04 PM, Arnd Bergmann wrote:
>> > > Would it be possible to split the config space access out into
>> > > a separate file? It would be nice to share that with the generic
>> > > ECAM driver that Will Deacon has sent.
>> >
>> > Yes, it should be possible. Is it ok, if I work on top of this driver?
>>
>> Do you mean as a follow-on patch? My feeling is that since we are trying
>> to merge both for 3.16, it would be good to get it done right away if
>> it doesn't cause too much extra work.
>
> Do you mean something as simple as a helper for base + offset ECAM
> addressing, or something more involved that handles the mapping as well? The
> latter would need some alignment on sys->private_data, I think.
>
> Srikanth: I'll CC you on the next version of my patches (I'll send them
> now).
>
>> > > As a general comment about the MSI implementation, I wonder if
>> > > this is actually
>> > > generic enough to be shared with other host controllers. It could be
>> > > moved
>> > > into a separate file like the config space access in that case.
>> >
>> > I feel the MSI implementation is not generic by looking into the other
>> > host controllers,
>> > it is more specific to the hardware. Correct me, if am wrong.
>>
>> The other host controllers are certainly incompatible, but this one looks
>> like it could be used on other controllers easily.
>>
>> Splitting it out would also make it easier to use another MSI implementation
>> like the one in the GIC.
>
> Actually, MarcZ and I already have my driver working with GICv3 + MSI. The
> code basically amounts to implementing {add,remove}_bus callbacks to set
> the msi_chip for the pci_bus, based on what we got out of the devicetree.
> I don't think we needed anything else... Marc?
No, that's it, really. I use the "msi-parent" property to find the
msi_chip, which should have been registered as a "msi-controller".
M.
--
Jazz is not dead. It just smells funny.
--
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 v3] pcie: Add Xilinx PCIe Host Bridge IP driver
Hi all,
Thanks for CC'ing me, Arnd.
On Wed, May 07, 2014 at 03:35:48PM +0100, Arnd Bergmann wrote:
> On Wednesday 07 May 2014 17:21:13 Srikanth Thokala wrote:
> > On Wed, Apr 30, 2014 at 9:04 PM, Arnd Bergmann wrote:
> > > Would it be possible to split the config space access out into
> > > a separate file? It would be nice to share that with the generic
> > > ECAM driver that Will Deacon has sent.
> >
> > Yes, it should be possible. Is it ok, if I work on top of this driver?
>
> Do you mean as a follow-on patch? My feeling is that since we are trying
> to merge both for 3.16, it would be good to get it done right away if
> it doesn't cause too much extra work.
Do you mean something as simple as a helper for base + offset ECAM
addressing, or something more involved that handles the mapping as well? The
latter would need some alignment on sys->private_data, I think.
Srikanth: I'll CC you on the next version of my patches (I'll send them
now).
> > > As a general comment about the MSI implementation, I wonder if this is
> > > actually
> > > generic enough to be shared with other host controllers. It could be moved
> > > into a separate file like the config space access in that case.
> >
> > I feel the MSI implementation is not generic by looking into the other
> > host controllers,
> > it is more specific to the hardware. Correct me, if am wrong.
>
> The other host controllers are certainly incompatible, but this one looks
> like it could be used on other controllers easily.
>
> Splitting it out would also make it easier to use another MSI implementation
> like the one in the GIC.
Actually, MarcZ and I already have my driver working with GICv3 + MSI. The
code basically amounts to implementing {add,remove}_bus callbacks to set
the msi_chip for the pci_bus, based on what we got out of the devicetree.
I don't think we needed anything else... Marc?
Will
--
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 v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Wednesday 07 May 2014 17:21:13 Srikanth Thokala wrote:
> On Wed, Apr 30, 2014 at 9:04 PM, Arnd Bergmann wrote:
> > On Tuesday 15 April 2014, Srikanth Thokala wrote:
> >> +/**
> >> + * xilinx_pcie_get_config_base - Get configuration base
> >> + * @bus: Bus structure of current bus
> >> + * @devfn: Device/function
> >> + * @where: Offset from base
> >> + *
> >> + * Return: Base address of the configuration space needed to be
> >> + * accessed.
> >> + */
> >> +static void __iomem *xilinx_pcie_get_config_base(struct pci_bus *bus,
> >> + unsigned int devfn,
> >> + int where)
> >> +{
> >> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
> >> + int relbus;
> >> +
> >> + relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
> >> + (devfn << ECAM_DEV_NUM_SHIFT);
> >> +
> >> + return port->reg_base + relbus + where;
> >> +}
> >
> > Does this mean you have an ECAM-compliant config space? Nice!
> >
> > Would it be possible to split the config space access out into
> > a separate file? It would be nice to share that with the generic
> > ECAM driver that Will Deacon has sent.
>
> Yes, it should be possible. Is it ok, if I work on top of this driver?
Do you mean as a follow-on patch? My feeling is that since we are trying
to merge both for 3.16, it would be good to get it done right away if
it doesn't cause too much extra work.
> >> +/**
> >> + * xilinx_pcie_enable_msi - Enable MSI support
> >> + * @port: PCIe port information
> >> + */
> >> +static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
> >> +{
> >> + port->msg_addr = __get_free_pages(GFP_KERNEL, 0);
> >> +
> >> + pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
> >> + pcie_write(port, virt_to_phys((void *)port->msg_addr),
> >> +XILINX_PCIE_REG_MSIBASE2);
> >> +}
> >
> > As a general comment about the MSI implementation, I wonder if this is
> > actually
> > generic enough to be shared with other host controllers. It could be moved
> > into a separate file like the config space access in that case.
>
> I feel the MSI implementation is not generic by looking into the other
> host controllers,
> it is more specific to the hardware. Correct me, if am wrong.
The other host controllers are certainly incompatible, but this one looks
like it could be used on other controllers easily.
Splitting it out would also make it easier to use another MSI implementation
like the one in the GIC.
> >> + /* Register the device */
> >> + pci_common_init_dev(dev, &hw);
> >> +
> >> + platform_set_drvdata(pdev, port);
> >
> > Don't you have to do the platform_set_drvdata() before
> > pci_common_init_dev()?
>
> It should be fine, as I don't see any dependencies.
Ah, it's only used in the remove function. It looks correct then, but I think
it would be better to set it first anyway, in case another function starts using
the drvdata later and that function may get called by the PCI initialization.
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: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Wed, Apr 30, 2014 at 9:04 PM, Arnd Bergmann wrote:
> On Tuesday 15 April 2014, Srikanth Thokala wrote:
>> +Required properties:
>> +- #address-cells: Address representation for root ports, set to <3>
>> +- #size-cells: Size representation for root ports, set to <2>
>> +- #interrupt-cells: specifies the number of cells needed to encode an
>> + interrupt source. The value must be 1.
>> +- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
>> +- reg: Should contain AXI PCIe registers location and length
>> +- device_type: must be "pci"
>> +- interrupts: Should contain AXI PCIe interrupt
>> +- interrupt-map-mask,
>> + interrupt-map: standard PCI properties to define the mapping of the
>> + PCI interface to interrupt numbers.
>> +- ranges: ranges for the PCI memory regions (I/O space region is noti
>
> typo: noti -> not
Ok, will fix.
>
>> + supported by hardware)
>> + Please refer to the standard PCI bus binding document for a more
>> + detailed explanation
>> +
>> +Optional properties:
>> +- bus-range: PCI bus numbers covered
>> +
>> +Interrupt controller child node
>>
>> +Required properties:
>> +- interrupt-controller: identifies the node as an interrupt controller
>> +- #address-cells: specifies the number of cells needed to encode an
>> + address. The value must be 0.
>> +- #interrupt-cells: specifies the number of cells needed to encode an
>> + interrupt source. The value must be 1.
>> +
>> +NOTE:
>> +The core provides a single interrupt for both INTx/MSI messages. So,
>> +created a interrupt controller node to support 'interrupt-map' DT
>> +functionality. The driver will create an IRQ domain for this map, decode
>> +the four INTx interrupts in ISR and route them to this domain.
>
> How does this work if the pci core is combined with a GIC version that
> also has MSI support. Presumably you'd want to use that for performance
> reason rather than the integrated MSI chip.
>
> Shouldn't there be a way to pick between the two?
I will check and come back to you on this.
>
>> +/**
>> + * xilinx_pcie_get_config_base - Get configuration base
>> + * @bus: Bus structure of current bus
>> + * @devfn: Device/function
>> + * @where: Offset from base
>> + *
>> + * Return: Base address of the configuration space needed to be
>> + * accessed.
>> + */
>> +static void __iomem *xilinx_pcie_get_config_base(struct pci_bus *bus,
>> + unsigned int devfn,
>> + int where)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> + int relbus;
>> +
>> + relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
>> + (devfn << ECAM_DEV_NUM_SHIFT);
>> +
>> + return port->reg_base + relbus + where;
>> +}
>
> Does this mean you have an ECAM-compliant config space? Nice!
>
> Would it be possible to split the config space access out into
> a separate file? It would be nice to share that with the generic
> ECAM driver that Will Deacon has sent.
Yes, it should be possible. Is it ok, if I work on top of this driver?
>
>> +
>> + msg.address_hi = 0;
>> + msg.address_lo = virt_to_phys((void *)port->msg_addr);
>> + msg.data = irq;
>> +
>> + write_msi_msg(irq, &msg);
>
> It seems strange to pass the msg_addr as an integer referring to
> a virtual address. I'd suggest using phys_addr_t for the type
> and converting it at the point the page gets allocated, and then
> always assigning both the high and low part here. You'll need
> that anyway for 64-bit operation.
Ok, I will fix it. Thanks.
>
>> +/**
>> + * xilinx_pcie_enable_msi - Enable MSI support
>> + * @port: PCIe port information
>> + */
>> +static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
>> +{
>> + port->msg_addr = __get_free_pages(GFP_KERNEL, 0);
>> +
>> + pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
>> + pcie_write(port, virt_to_phys((void *)port->msg_addr),
>> +XILINX_PCIE_REG_MSIBASE2);
>> +}
>
> here too.
>
> As a general comment about the MSI implementation, I wonder if this is
> actually
> generic enough to be shared with other host controllers. It could be moved
> into a separate file like the config space access in that case.
I feel the MSI implementation is not generic by looking into the other
host controllers,
it is more specific to the hardware. Correct me, if am wrong.
>
>> +/**
>> + * xilinx_pcie_init_irq_domain - Initialize IRQ domain
>> + * @port: PCIe port information
>> + *
>> + * Return: '0' on success and error value on failure
>> + */
>> +static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
>> +{
>> + struct device *dev = port->dev;
>> + struct device_node *node = dev->of_node;
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + /* Setup MSI */
>> + int i;
>> +
>> + port->irq_domain = irq_domain_add_linear(node,
>> +
Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Thu, May 1, 2014 at 3:11 AM, Bjorn Helgaas wrote:
> On Tue, Apr 15, 2014 at 05:08:31PM +0530, Srikanth Thokala wrote:
>> This is the driver for Xilinx AXI PCIe Host Bridge Soft IP
>>
>> Signed-off-by: Srikanth Thokala
>> ---
>> Changes in v3:
>> - Rebased on v3.15.0-rc1
>> - Added support for interrupt-map DT functionality.
>> - Removed map_irq() wrapper, instead using of_irq_parse_and_map_pci().
>> - Modified resource mapping logic as per the series
>> "PCI: ARM: add support for generic PCI host controller"
>> - Modified devicetree binding documentation to update with interrupt-
>> map properties.
>> - Use devm calls wherever applicable.
>> - Fixed minor comments from Jason
>> - Thanks Jason for the review and suggestions.
>>
>> Changes in v2:
>> - Rebased on v3.14.0-rc8
>> - Removed IP specific DT properties like include-rc, axibar-num etc.,
>> as suggested by Jason and Bjorn, Thanks
>> ---
>> .../devicetree/bindings/pci/xilinx-pcie.txt| 62 ++
>> drivers/pci/host/Kconfig |7 +
>> drivers/pci/host/Makefile |1 +
>> drivers/pci/host/pci-xilinx.c | 999
>>
>> 4 files changed, 1069 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
>> create mode 100644 drivers/pci/host/pci-xilinx.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
>> b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
>> new file mode 100644
>> index 000..2fb28e0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
>> @@ -0,0 +1,62 @@
>> +* Xilinx AXI PCIe Root Port Bridge DT description
>> +
>> +Required properties:
>> +- #address-cells: Address representation for root ports, set to <3>
>> +- #size-cells: Size representation for root ports, set to <2>
>> +- #interrupt-cells: specifies the number of cells needed to encode an
>> + interrupt source. The value must be 1.
>> +- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
>> +- reg: Should contain AXI PCIe registers location and length
>> +- device_type: must be "pci"
>> +- interrupts: Should contain AXI PCIe interrupt
>> +- interrupt-map-mask,
>> + interrupt-map: standard PCI properties to define the mapping of the
>> + PCI interface to interrupt numbers.
>> +- ranges: ranges for the PCI memory regions (I/O space region is noti
>> + supported by hardware)
>> + Please refer to the standard PCI bus binding document for a more
>> + detailed explanation
>> +
>> +Optional properties:
>> +- bus-range: PCI bus numbers covered
>> +
>> +Interrupt controller child node
>>
>> +Required properties:
>> +- interrupt-controller: identifies the node as an interrupt controller
>> +- #address-cells: specifies the number of cells needed to encode an
>> + address. The value must be 0.
>> +- #interrupt-cells: specifies the number of cells needed to encode an
>> + interrupt source. The value must be 1.
>> +
>> +NOTE:
>> +The core provides a single interrupt for both INTx/MSI messages. So,
>> +created a interrupt controller node to support 'interrupt-map' DT
>> +functionality. The driver will create an IRQ domain for this map, decode
>> +the four INTx interrupts in ISR and route them to this domain.
>> +
>> +
>> +Example:
>> +
>> +
>> + pci_express: axi-pcie@5000 {
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + #interrupt-cells = <1>;
>> + compatible = "xlnx,axi-pcie-host-1.00.a";
>> + reg = < 0x5000 0x1000 >;
>> + device_type = "pci";
>> + interrupts = < 0 52 4 >;
>> + interrupt-map-mask = <0 0 0 7>;
>> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
>> + <0 0 0 2 &pcie_intc 2>,
>> + <0 0 0 3 &pcie_intc 3>,
>> + <0 0 0 4 &pcie_intc 4>;
>> + ranges = < 0x0200 0 0x6000 0x6000 0 0x1000 >;
>> +
>> + pcie_intc: interrupt-controller {
>> + interrupt-controller;
>> + #address-cells = <0>;
>> + #interrupt-cells = <1>;
>> + }
>> + };
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index a6f67ec..71afdcd 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -33,4 +33,11 @@ config PCI_RCAR_GEN2
>> There are 3 internal PCI controllers available with a single
>> built-in EHCI/OHCI host controller present on each one.
>>
>> +config PCI_XILINX
>> + bool "Xilinx AXI PCIe host bridge support"
>> + depends on ARCH_ZYNQ
>> + help
>> + Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>> + Host Bridge driver.
>> +
>> endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 13fb333.
Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Tue, Apr 15, 2014 at 05:08:31PM +0530, Srikanth Thokala wrote:
> This is the driver for Xilinx AXI PCIe Host Bridge Soft IP
>
> Signed-off-by: Srikanth Thokala
> ---
> Changes in v3:
> - Rebased on v3.15.0-rc1
> - Added support for interrupt-map DT functionality.
> - Removed map_irq() wrapper, instead using of_irq_parse_and_map_pci().
> - Modified resource mapping logic as per the series
> "PCI: ARM: add support for generic PCI host controller"
> - Modified devicetree binding documentation to update with interrupt-
> map properties.
> - Use devm calls wherever applicable.
> - Fixed minor comments from Jason
> - Thanks Jason for the review and suggestions.
>
> Changes in v2:
> - Rebased on v3.14.0-rc8
> - Removed IP specific DT properties like include-rc, axibar-num etc.,
> as suggested by Jason and Bjorn, Thanks
> ---
> .../devicetree/bindings/pci/xilinx-pcie.txt| 62 ++
> drivers/pci/host/Kconfig |7 +
> drivers/pci/host/Makefile |1 +
> drivers/pci/host/pci-xilinx.c | 999
>
> 4 files changed, 1069 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> create mode 100644 drivers/pci/host/pci-xilinx.c
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> new file mode 100644
> index 000..2fb28e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> @@ -0,0 +1,62 @@
> +* Xilinx AXI PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
> +- reg: Should contain AXI PCIe registers location and length
> +- device_type: must be "pci"
> +- interrupts: Should contain AXI PCIe interrupt
> +- interrupt-map-mask,
> + interrupt-map: standard PCI properties to define the mapping of the
> + PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is noti
> + supported by hardware)
> + Please refer to the standard PCI bus binding document for a more
> + detailed explanation
> +
> +Optional properties:
> +- bus-range: PCI bus numbers covered
> +
> +Interrupt controller child node
>
> +Required properties:
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #address-cells: specifies the number of cells needed to encode an
> + address. The value must be 0.
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +
> +NOTE:
> +The core provides a single interrupt for both INTx/MSI messages. So,
> +created a interrupt controller node to support 'interrupt-map' DT
> +functionality. The driver will create an IRQ domain for this map, decode
> +the four INTx interrupts in ISR and route them to this domain.
> +
> +
> +Example:
> +
> +
> + pci_express: axi-pcie@5000 {
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + compatible = "xlnx,axi-pcie-host-1.00.a";
> + reg = < 0x5000 0x1000 >;
> + device_type = "pci";
> + interrupts = < 0 52 4 >;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
> + <0 0 0 2 &pcie_intc 2>,
> + <0 0 0 3 &pcie_intc 3>,
> + <0 0 0 4 &pcie_intc 4>;
> + ranges = < 0x0200 0 0x6000 0x6000 0 0x1000 >;
> +
> + pcie_intc: interrupt-controller {
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + }
> + };
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index a6f67ec..71afdcd 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -33,4 +33,11 @@ config PCI_RCAR_GEN2
> There are 3 internal PCI controllers available with a single
> built-in EHCI/OHCI host controller present on each one.
>
> +config PCI_XILINX
> + bool "Xilinx AXI PCIe host bridge support"
> + depends on ARCH_ZYNQ
> + help
> + Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> + Host Bridge driver.
> +
> endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 13fb333..4e9c843 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCI_MVEBU) +=
Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Tuesday 15 April 2014, Srikanth Thokala wrote:
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
> +- reg: Should contain AXI PCIe registers location and length
> +- device_type: must be "pci"
> +- interrupts: Should contain AXI PCIe interrupt
> +- interrupt-map-mask,
> + interrupt-map: standard PCI properties to define the mapping of the
> + PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is noti
typo: noti -> not
> + supported by hardware)
> + Please refer to the standard PCI bus binding document for a more
> + detailed explanation
> +
> +Optional properties:
> +- bus-range: PCI bus numbers covered
> +
> +Interrupt controller child node
>
> +Required properties:
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #address-cells: specifies the number of cells needed to encode an
> + address. The value must be 0.
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +
> +NOTE:
> +The core provides a single interrupt for both INTx/MSI messages. So,
> +created a interrupt controller node to support 'interrupt-map' DT
> +functionality. The driver will create an IRQ domain for this map, decode
> +the four INTx interrupts in ISR and route them to this domain.
How does this work if the pci core is combined with a GIC version that
also has MSI support. Presumably you'd want to use that for performance
reason rather than the integrated MSI chip.
Shouldn't there be a way to pick between the two?
> +/**
> + * xilinx_pcie_get_config_base - Get configuration base
> + * @bus: Bus structure of current bus
> + * @devfn: Device/function
> + * @where: Offset from base
> + *
> + * Return: Base address of the configuration space needed to be
> + * accessed.
> + */
> +static void __iomem *xilinx_pcie_get_config_base(struct pci_bus *bus,
> + unsigned int devfn,
> + int where)
> +{
> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
> + int relbus;
> +
> + relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
> + (devfn << ECAM_DEV_NUM_SHIFT);
> +
> + return port->reg_base + relbus + where;
> +}
Does this mean you have an ECAM-compliant config space? Nice!
Would it be possible to split the config space access out into
a separate file? It would be nice to share that with the generic
ECAM driver that Will Deacon has sent.
> +
> + msg.address_hi = 0;
> + msg.address_lo = virt_to_phys((void *)port->msg_addr);
> + msg.data = irq;
> +
> + write_msi_msg(irq, &msg);
It seems strange to pass the msg_addr as an integer referring to
a virtual address. I'd suggest using phys_addr_t for the type
and converting it at the point the page gets allocated, and then
always assigning both the high and low part here. You'll need
that anyway for 64-bit operation.
> +/**
> + * xilinx_pcie_enable_msi - Enable MSI support
> + * @port: PCIe port information
> + */
> +static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port)
> +{
> + port->msg_addr = __get_free_pages(GFP_KERNEL, 0);
> +
> + pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1);
> + pcie_write(port, virt_to_phys((void *)port->msg_addr),
> +XILINX_PCIE_REG_MSIBASE2);
> +}
here too.
As a general comment about the MSI implementation, I wonder if this is actually
generic enough to be shared with other host controllers. It could be moved
into a separate file like the config space access in that case.
> +/**
> + * xilinx_pcie_init_irq_domain - Initialize IRQ domain
> + * @port: PCIe port information
> + *
> + * Return: '0' on success and error value on failure
> + */
> +static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port)
> +{
> + struct device *dev = port->dev;
> + struct device_node *node = dev->of_node;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + /* Setup MSI */
> + int i;
> +
> + port->irq_domain = irq_domain_add_linear(node,
> + XILINX_NUM_MSI_IRQS,
> + &msi_domain_ops,
> + &xilinx_pcie_msi_chip);
> + if (!port->irq_domain) {
> + dev_err(dev, "Failed to get a MSI IRQ domain\n");
> + return PTR_ERR(port->irq_domain);
> + }
> +
> + for (i = 0; i < XILINX_NUM_MSI_IRQS; i++)
> + irq_create_mapping(port->irq_d
Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
Hi Bjorn, On 04/15/2014 01:38 PM, Srikanth Thokala wrote: > This is the driver for Xilinx AXI PCIe Host Bridge Soft IP > > Signed-off-by: Srikanth Thokala > --- > Changes in v3: > - Rebased on v3.15.0-rc1 > - Added support for interrupt-map DT functionality. > - Removed map_irq() wrapper, instead using of_irq_parse_and_map_pci(). > - Modified resource mapping logic as per the series > "PCI: ARM: add support for generic PCI host controller" > - Modified devicetree binding documentation to update with interrupt- > map properties. > - Use devm calls wherever applicable. > - Fixed minor comments from Jason > - Thanks Jason for the review and suggestions. > > Changes in v2: > - Rebased on v3.14.0-rc8 > - Removed IP specific DT properties like include-rc, axibar-num etc., > as suggested by Jason and Bjorn, Thanks > --- > .../devicetree/bindings/pci/xilinx-pcie.txt| 62 ++ > drivers/pci/host/Kconfig |7 + > drivers/pci/host/Makefile |1 + > drivers/pci/host/pci-xilinx.c | 999 > > 4 files changed, 1069 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt > create mode 100644 drivers/pci/host/pci-xilinx.c Any comment on this patch? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
Hi,
Kindly review the driver and please let me know if you have any comments.
Thanks
Srikanth
On Tue, Apr 15, 2014 at 5:08 PM, Srikanth Thokala wrote:
> This is the driver for Xilinx AXI PCIe Host Bridge Soft IP
>
> Signed-off-by: Srikanth Thokala
> ---
> Changes in v3:
> - Rebased on v3.15.0-rc1
> - Added support for interrupt-map DT functionality.
> - Removed map_irq() wrapper, instead using of_irq_parse_and_map_pci().
> - Modified resource mapping logic as per the series
> "PCI: ARM: add support for generic PCI host controller"
> - Modified devicetree binding documentation to update with interrupt-
> map properties.
> - Use devm calls wherever applicable.
> - Fixed minor comments from Jason
> - Thanks Jason for the review and suggestions.
>
> Changes in v2:
> - Rebased on v3.14.0-rc8
> - Removed IP specific DT properties like include-rc, axibar-num etc.,
> as suggested by Jason and Bjorn, Thanks
> ---
> .../devicetree/bindings/pci/xilinx-pcie.txt| 62 ++
> drivers/pci/host/Kconfig |7 +
> drivers/pci/host/Makefile |1 +
> drivers/pci/host/pci-xilinx.c | 999
>
> 4 files changed, 1069 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> create mode 100644 drivers/pci/host/pci-xilinx.c
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> new file mode 100644
> index 000..2fb28e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> @@ -0,0 +1,62 @@
> +* Xilinx AXI PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
> +- reg: Should contain AXI PCIe registers location and length
> +- device_type: must be "pci"
> +- interrupts: Should contain AXI PCIe interrupt
> +- interrupt-map-mask,
> + interrupt-map: standard PCI properties to define the mapping of the
> + PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is noti
> + supported by hardware)
> + Please refer to the standard PCI bus binding document for a more
> + detailed explanation
> +
> +Optional properties:
> +- bus-range: PCI bus numbers covered
> +
> +Interrupt controller child node
>
> +Required properties:
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #address-cells: specifies the number of cells needed to encode an
> + address. The value must be 0.
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +
> +NOTE:
> +The core provides a single interrupt for both INTx/MSI messages. So,
> +created a interrupt controller node to support 'interrupt-map' DT
> +functionality. The driver will create an IRQ domain for this map, decode
> +the four INTx interrupts in ISR and route them to this domain.
> +
> +
> +Example:
> +
> +
> + pci_express: axi-pcie@5000 {
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + compatible = "xlnx,axi-pcie-host-1.00.a";
> + reg = < 0x5000 0x1000 >;
> + device_type = "pci";
> + interrupts = < 0 52 4 >;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
> + <0 0 0 2 &pcie_intc 2>,
> + <0 0 0 3 &pcie_intc 3>,
> + <0 0 0 4 &pcie_intc 4>;
> + ranges = < 0x0200 0 0x6000 0x6000 0 0x1000 >;
> +
> + pcie_intc: interrupt-controller {
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + }
> + };
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index a6f67ec..71afdcd 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -33,4 +33,11 @@ config PCI_RCAR_GEN2
> There are 3 internal PCI controllers available with a single
> built-in EHCI/OHCI host controller present on each one.
>
> +config PCI_XILINX
> + bool "Xilinx AXI PCIe host bridge support"
> + depends on ARCH_ZYNQ
> + help
> + Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> + Host Bridge driver.
> +
> endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 13fb333..4e9c843 100644
[PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
This is the driver for Xilinx AXI PCIe Host Bridge Soft IP
Signed-off-by: Srikanth Thokala
---
Changes in v3:
- Rebased on v3.15.0-rc1
- Added support for interrupt-map DT functionality.
- Removed map_irq() wrapper, instead using of_irq_parse_and_map_pci().
- Modified resource mapping logic as per the series
"PCI: ARM: add support for generic PCI host controller"
- Modified devicetree binding documentation to update with interrupt-
map properties.
- Use devm calls wherever applicable.
- Fixed minor comments from Jason
- Thanks Jason for the review and suggestions.
Changes in v2:
- Rebased on v3.14.0-rc8
- Removed IP specific DT properties like include-rc, axibar-num etc.,
as suggested by Jason and Bjorn, Thanks
---
.../devicetree/bindings/pci/xilinx-pcie.txt| 62 ++
drivers/pci/host/Kconfig |7 +
drivers/pci/host/Makefile |1 +
drivers/pci/host/pci-xilinx.c | 999
4 files changed, 1069 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
create mode 100644 drivers/pci/host/pci-xilinx.c
diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
new file mode 100644
index 000..2fb28e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
@@ -0,0 +1,62 @@
+* Xilinx AXI PCIe Root Port Bridge DT description
+
+Required properties:
+- #address-cells: Address representation for root ports, set to <3>
+- #size-cells: Size representation for root ports, set to <2>
+- #interrupt-cells: specifies the number of cells needed to encode an
+ interrupt source. The value must be 1.
+- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
+- reg: Should contain AXI PCIe registers location and length
+- device_type: must be "pci"
+- interrupts: Should contain AXI PCIe interrupt
+- interrupt-map-mask,
+ interrupt-map: standard PCI properties to define the mapping of the
+ PCI interface to interrupt numbers.
+- ranges: ranges for the PCI memory regions (I/O space region is noti
+ supported by hardware)
+ Please refer to the standard PCI bus binding document for a more
+ detailed explanation
+
+Optional properties:
+- bus-range: PCI bus numbers covered
+
+Interrupt controller child node
+Required properties:
+- interrupt-controller: identifies the node as an interrupt controller
+- #address-cells: specifies the number of cells needed to encode an
+ address. The value must be 0.
+- #interrupt-cells: specifies the number of cells needed to encode an
+ interrupt source. The value must be 1.
+
+NOTE:
+The core provides a single interrupt for both INTx/MSI messages. So,
+created a interrupt controller node to support 'interrupt-map' DT
+functionality. The driver will create an IRQ domain for this map, decode
+the four INTx interrupts in ISR and route them to this domain.
+
+
+Example:
+
+
+ pci_express: axi-pcie@5000 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ compatible = "xlnx,axi-pcie-host-1.00.a";
+ reg = < 0x5000 0x1000 >;
+ device_type = "pci";
+ interrupts = < 0 52 4 >;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc 1>,
+ <0 0 0 2 &pcie_intc 2>,
+ <0 0 0 3 &pcie_intc 3>,
+ <0 0 0 4 &pcie_intc 4>;
+ ranges = < 0x0200 0 0x6000 0x6000 0 0x1000 >;
+
+ pcie_intc: interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ }
+ };
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index a6f67ec..71afdcd 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -33,4 +33,11 @@ config PCI_RCAR_GEN2
There are 3 internal PCI controllers available with a single
built-in EHCI/OHCI host controller present on each one.
+config PCI_XILINX
+ bool "Xilinx AXI PCIe host bridge support"
+ depends on ARCH_ZYNQ
+ help
+ Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
+ Host Bridge driver.
+
endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 13fb333..4e9c843 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
+obj-$(CONFIG_PCI_XILINX) += pci-xilinx.o
diff --git a/drivers/pci/host/pci-xilinx.c b/drivers/pci/host/pci-xilinx.c
new file mode 100644
in
