| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+mB=1L3O6QJhDJyff2=jQm3euxUHPtPD2Zw9xKFUCYecaB3iA@mail.gmail.com>
Date: Tue, 6 May 2014 12:39:45 +0530
From: Srikanth Thokala <sthokal@...inx.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>
Cc: Srikanth Thokala <sthokal@...inx.com>,
Michal Simek <michal.simek@...inx.com>,
Grant Likely <grant.likely@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
devicetree@...r.kernel.org
Subject: Re: [PATCH v3] pcie: Add Xilinx PCIe Host Bridge IP driver
On Thu, May 1, 2014 at 3:11 AM, Bjorn Helgaas <bhelgaas@...gle.com> 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 <sthokal@...inx.com>
>> ---
>> 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 0000000..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@...00000 {
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + #interrupt-cells = <1>;
>> + compatible = "xlnx,axi-pcie-host-1.00.a";
>> + reg = < 0x50000000 0x10000000 >;
>> + 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 = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
>> +
>> + 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
>> index 0000000..d9afe66
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-xilinx.c
>> @@ -0,0 +1,999 @@
>> +/*
>> + * PCIe host controller driver for Xilinx AXI PCIe Bridge
>> + *
>> + * Copyright (c) 2012 - 2014 Xilinx, Inc.
>> + *
>> + * Based on the Tegra PCIe driver
>> + *
>> + * Bits taken from Synopsys Designware Host controller driver and
>> + * ARM PCI Host generic driver.
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/msi.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +
>> +/* Register definitions */
>> +#define XILINX_PCIE_REG_VSECC 0x00000128
>> +#define XILINX_PCIE_REG_VSECH 0x0000012c
>> +#define XILINX_PCIE_REG_BIR 0x00000130
>> +#define XILINX_PCIE_REG_BSCR 0x00000134
>> +#define XILINX_PCIE_REG_IDR 0x00000138
>> +#define XILINX_PCIE_REG_IMR 0x0000013c
>> +#define XILINX_PCIE_REG_BLR 0x00000140
>> +#define XILINX_PCIE_REG_PSCR 0x00000144
>> +#define XILINX_PCIE_REG_RPSC 0x00000148
>> +#define XILINX_PCIE_REG_MSIBASE1 0x0000014c
>> +#define XILINX_PCIE_REG_MSIBASE2 0x00000150
>> +#define XILINX_PCIE_REG_RPEFR 0x00000154
>> +#define XILINX_PCIE_REG_RPIFR1 0x00000158
>> +#define XILINX_PCIE_REG_RPIFR2 0x0000015c
>> +#define XILINX_PCIE_REG_VSECC2 0x00000200
>> +#define XILINX_PCIE_REG_VSECH2 0x00000204
>> +
>> +/* Interrupt registers definitions */
>> +#define XILINX_PCIE_INTR_LINK_DOWN BIT(0)
>> +#define XILINX_PCIE_INTR_ECRC_ERR BIT(1)
>> +#define XILINX_PCIE_INTR_STR_ERR BIT(2)
>> +#define XILINX_PCIE_INTR_HOT_RESET BIT(3)
>> +#define XILINX_PCIE_INTR_CFG_COMPL GENMASK(7, 5)
>> +#define XILINX_PCIE_INTR_CFG_TIMEOUT BIT(8)
>> +#define XILINX_PCIE_INTR_CORRECTABLE BIT(9)
>> +#define XILINX_PCIE_INTR_NONFATAL BIT(10)
>> +#define XILINX_PCIE_INTR_FATAL BIT(11)
>> +#define XILINX_PCIE_INTR_INTX BIT(16)
>> +#define XILINX_PCIE_INTR_MSI BIT(17)
>> +#define XILINX_PCIE_INTR_SLV_UNSUPP BIT(20)
>> +#define XILINX_PCIE_INTR_SLV_UNEXP BIT(21)
>> +#define XILINX_PCIE_INTR_SLV_COMPL BIT(22)
>> +#define XILINX_PCIE_INTR_SLV_ERRP BIT(23)
>> +#define XILINX_PCIE_INTR_SLV_CMPABT BIT(24)
>> +#define XILINX_PCIE_INTR_SLV_ILLBUR BIT(25)
>> +#define XILINX_PCIE_INTR_MST_DECERR BIT(26)
>> +#define XILINX_PCIE_INTR_MST_SLVERR BIT(27)
>> +#define XILINX_PCIE_INTR_MST_ERRP BIT(28)
>> +#define XILINX_PCIE_IMR_ALL_MASK 0x1FF30FED
>> +#define XILINX_PCIE_IDR_ALL_MASK 0xFFFFFFFF
>> +
>> +/* Root Port Error FIFO Read Register definitions */
>> +#define XILINX_PCIE_RPEFR_ERR_VALID BIT(18)
>> +#define XILINX_PCIE_RPEFR_REQ_ID GENMASK(15, 0)
>> +#define XILINX_PCIE_RPEFR_ALL_MASK 0xFFFFFFFF
>> +
>> +/* Root Port Interrupt FIFO Read Register 1 definitions */
>> +#define XILINX_PCIE_RPIFR1_INTR_VALID BIT(31)
>> +#define XILINX_PCIE_RPIFR1_MSI_INTR BIT(30)
>> +#define XILINX_PCIE_RPIFR1_INTR_ASSRT BIT(29)
>> +#define XILINX_PCIE_RPIFR1_INTR_MASK GENMASK(28, 27)
>> +#define XILINX_PCIE_RPIFR1_MSI_ADR_MASK GENMASK(26, 16)
>> +#define XILINX_PCIE_RPIFR1_ALL_MASK 0xFFFFFFFF
>> +#define XILINX_PCIE_RPIFR1_INTR_SHIFT 27
>> +#define XILINX_PCIE_RPIFR1_MSI_ADR_SHIFT 16
>> +
>> +/* Bridge Info Register definitions */
>> +#define XILINX_PCIE_BIR_ECAM_SZ_MASK GENMASK(18, 16)
>> +#define XILINX_PCIE_BIR_ECAM_SZ_SHIFT 16
>> +
>> +/* Root Port Interrupt FIFO Read Register 2 definitions */
>> +#define XILINX_PCIE_RPIFR2_MSG_DATA GENMASK(15, 0)
>> +
>> +/* Root Port Status/control Register definitions */
>> +#define XILINX_PCIE_REG_RPSC_BEN BIT(0)
>> +
>> +/* Phy Status/Control Register definitions */
>> +#define XILINX_PCIE_REG_PSCR_LNKUP BIT(11)
>> +
>> +/* ECAM definitions */
>> +#define ECAM_BUS_NUM_SHIFT 20
>> +#define ECAM_DEV_NUM_SHIFT 12
>> +
>> +/* PCI Configuration space Bus Number Register definitions */
>> +#define PCI_SECONDARY_BUS_NUM_SHIFT 8
>> +#define PCI_SUBORDINATE_BUS_NUM_SHIFT 16
>> +#define PCI_LATENCY_TIMER_MASK GENMASK(31, 24)
>> +
>> +/* Number of MSI IRQs */
>> +#define XILINX_NUM_MSI_IRQS 128
>> +
>> +/* Number of Memory Resources */
>> +#define XILINX_MAX_NUM_RESOURCES 3
>> +
>> +/**
>> + * struct xilinx_pcie_port - PCIe port information
>> + * @reg_base: IO Mapped Register Base
>> + * @irq: Interrupt number
>> + * @msg_addr: MSI message address
>> + * @root_busno: Root Bus number
>> + * @link_up: Link status flag
>> + * @dev: Device pointer
>> + * @irq_domain: IRQ domain pointer
>> + */
>> +struct xilinx_pcie_port {
>> + void __iomem *reg_base;
>> + u32 irq;
>> + unsigned long msg_addr;
>> + u8 root_busno;
>> + bool link_up;
>> + struct device *dev;
>> + struct irq_domain *irq_domain;
>> +};
>> +
>> +static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
>> +
>> +static inline struct xilinx_pcie_port *sys_to_pcie(struct pci_sys_data *sys)
>> +{
>> + return sys->private_data;
>> +}
>> +
>> +static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
>> +{
>> + return readl(port->reg_base + reg);
>> +}
>> +
>> +static inline void pcie_write(struct xilinx_pcie_port *port,
>> + u32 val, u32 reg)
>> +{
>> + writel(val, port->reg_base + reg);
>> +}
>> +
>> +static inline bool xilinx_pcie_is_link_up(struct xilinx_pcie_port *port)
>> +{
>> + return (pcie_read(port, XILINX_PCIE_REG_PSCR) &
>> + XILINX_PCIE_REG_PSCR_LNKUP) ? 1 : 0;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_clear_err_interrupts - Clear Error Interrupts
>> + * @port: PCIe port information
>> + */
>> +static inline void
>> +xilinx_pcie_clear_err_interrupts(struct xilinx_pcie_port *port)
>> +{
>> + u32 val = pcie_read(port, XILINX_PCIE_REG_RPEFR);
>> +
>> + if (val & XILINX_PCIE_RPEFR_ERR_VALID) {
>> + dev_dbg(port->dev, "Requester ID %d\n",
>> + val & XILINX_PCIE_RPEFR_REQ_ID);
>> + pcie_write(port, XILINX_PCIE_RPEFR_ALL_MASK,
>> + XILINX_PCIE_REG_RPEFR);
>> + }
>> +}
>> +
>> +/**
>> + * xilinx_pcie_verify_config - Verify configuration
>> + * @bus: Bus structure of current bus
>> + * @devfn: device/function
>> + *
>> + * Return: 0 on success and PCIBIOS_DEVICE_NOT_FOUND on error
>
> This comment about the return value doesn't match the code.
I will fix it, Thanks.
>
>> + */
>> +static int xilinx_pcie_verify_config(struct pci_bus *bus,
>> + unsigned int devfn)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> +
>> + /* Check if link is up when trying to access downstream ports */
>> + if (bus->number != port->root_busno)
>> + if (!xilinx_pcie_is_link_up(port))
>> + return 0;
>> +
>> + /* Only one device down on each root port */
>> + if (bus->number == port->root_busno && devfn > 0)
>> + return 0;
>> +
>> + /*
>> + * Do not read more than one device on the bus directly attached
>> + * to RC.
>> + */
>> + if (bus->primary == port->root_busno && devfn > 0)
>> + return 0;
>> +
>> + return 1;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_get_config_base - Get configuration base
>> + * @bus: Bus structure of current bus
>
> There's not really a concept of a "current" bus here. I think these
> function comments that repeat what the function and argument names already
> tell us are of minimal value. But I know some people do like them.
I will fix it.
>
>> + * @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;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_read_config - Read configuration space
>> + * @bus: Bus structure of current bus
>> + * @devfn: Device/function
>> + * @where: Offset from base
>> + * @size: Byte/word/dword
>> + * @val: Value to be read
>> + *
>> + * Return: PCIBIOS_SUCCESSFUL on success
>> + * EINVAL/PCIBIOS_DEVICE_NOT_FOUND/PCIBIOS_BAD_REGISTER_NUMBER
>> + * on failure.
>> + */
>> +static int xilinx_pcie_read_config(struct pci_bus *bus,
>> + unsigned int devfn, int where,
>> + int size, u32 *val)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> + void __iomem *addr;
>> +
>> + if (!port) {
>> + BUG();
>> + return -EINVAL;
>> + }
>> +
>> + if (!xilinx_pcie_verify_config(bus, devfn)) {
>> + *val = 0xFFFFFFFF;
>> + return PCIBIOS_DEVICE_NOT_FOUND;
>> + }
>> +
>> + addr = xilinx_pcie_get_config_base(bus, devfn, where);
>> +
>> + switch (size) {
>> + case 1:
>> + *val = readb(addr);
>> + break;
>> + case 2:
>> + *val = readw(addr);
>> + break;
>> + default:
>> + *val = readl(addr);
>> + break;
>> + }
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_write_config - Write configuration space
>> + * @bus: Bus structure of current bus
>> + * @devfn: Device/function
>> + * @where: Offset from base
>> + * @size: Byte/word/dword
>> + * @val: Value to be written to device
>> + *
>> + * Return: PCIBIOS_SUCCESSFUL on success,
>> + * EINVAL/PCIBIOS_DEVICE_NOT_FOUND/PCIBIOS_BAD_REGISTER_NUMBER
>> + * on failure.
>> + */
>> +static int xilinx_pcie_write_config(struct pci_bus *bus,
>> + unsigned int devfn, int where,
>> + int size, u32 val)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> + void __iomem *addr;
>> +
>> + if (!port) {
>> + BUG();
>> + return -EINVAL;
>> + }
>> +
>> + if (!xilinx_pcie_verify_config(bus, devfn))
>> + return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> + addr = xilinx_pcie_get_config_base(bus, devfn, where);
>> +
>> + switch (size) {
>> + case 1:
>> + writeb(val, addr);
>> + break;
>> + case 2:
>> + writew(val, addr);
>> + break;
>> + default:
>> + writel(val, addr);
>> + break;
>> + }
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +/* PCIe operations */
>> +static struct pci_ops xilinx_pcie_ops = {
>> + .read = xilinx_pcie_read_config,
>> + .write = xilinx_pcie_write_config,
>> +};
>> +
>> +/* MSI functions */
>> +
>> +/**
>> + * xilinx_pcie_destroy_msi - Free MSI number
>> + * @irq: IRQ to be freed
>> + */
>> +static void xilinx_pcie_destroy_msi(unsigned int irq)
>> +{
>> + struct irq_desc *desc;
>> + struct msi_desc *msi;
>> + struct xilinx_pcie_port *port;
>> +
>> + desc = irq_to_desc(irq);
>> + msi = irq_desc_get_msi_desc(desc);
>> + port = sys_to_pcie(msi->dev->bus->sysdata);
>> + if (!port) {
>> + BUG();
>> + return;
>> + }
>> +
>> + if (!test_bit(irq, msi_irq_in_use))
>> + dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
>> + else
>> + clear_bit(irq, msi_irq_in_use);
>> +}
>> +
>> +/**
>> + * xilinx_pcie_assign_msi - Allocate MSI number
>> + * @port: PCIe port structure
>> + *
>> + * Return: A valid IRQ on success and error value on failure.
>> + */
>> +static int xilinx_pcie_assign_msi(struct xilinx_pcie_port *port)
>> +{
>> + int pos;
>> +
>> + pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
>> + if (pos < XILINX_NUM_MSI_IRQS)
>> + set_bit(pos, msi_irq_in_use);
>> + else
>> + return -ENOSPC;
>> +
>> + return irq_find_mapping(port->irq_domain, pos);
>> +}
>> +
>> +/**
>> + * xilinx_msi_teardown_irq - Destroy the MSI
>> + * @chip: MSI Chip descriptor
>> + * @irq: MSI IRQ to destroy
>> + */
>> +static void xilinx_msi_teardown_irq(struct msi_chip *chip, unsigned int irq)
>> +{
>> + xilinx_pcie_destroy_msi(irq);
>> +}
>> +
>> +/**
>> + * xilinx_pcie_msi_setup_irq - Setup MSI request
>> + * @chip: MSI chip pointer
>> + * @pdev: PCIe device pointer
>> + * @desc: MSI descriptor pointer
>> + *
>> + * Return: '0' on success and error value on failure
>> + */
>> +static int xilinx_pcie_msi_setup_irq(struct msi_chip *chip,
>> + struct pci_dev *pdev,
>> + struct msi_desc *desc)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(pdev->bus->sysdata);
>> + int irq;
>> + struct msi_msg msg;
>> +
>> + if (!port) {
>> + BUG();
>> + return -EINVAL;
>> + }
>> +
>> + irq = xilinx_pcie_assign_msi(port);
>> + if (irq < 0)
>> + return irq;
>> +
>> + irq_set_msi_desc(irq, desc);
>> +
>> + msg.address_hi = 0;
>> + msg.address_lo = virt_to_phys((void *)port->msg_addr);
>> + msg.data = irq;
>> +
>> + write_msi_msg(irq, &msg);
>> +
>> + return 0;
>> +}
>> +
>> +/* MSI Chip Descriptor */
>> +static struct msi_chip xilinx_pcie_msi_chip = {
>> + .setup_irq = xilinx_pcie_msi_setup_irq,
>> + .teardown_irq = xilinx_msi_teardown_irq,
>> +};
>> +
>> +/* HW Interrupt Chip Descriptor */
>> +static struct irq_chip xilinx_msi_irq_chip = {
>> + .name = "Xilinx PCIe MSI",
>> + .irq_enable = unmask_msi_irq,
>> + .irq_disable = mask_msi_irq,
>> + .irq_mask = mask_msi_irq,
>> + .irq_unmask = unmask_msi_irq,
>> +};
>> +
>> +/**
>> + * xilinx_pcie_msi_map - Set the handler for the MSI and mark IRQ as valid
>> + * @domain: IRQ domain
>> + * @irq: Virtual IRQ number
>> + * @hwirq: HW interrupt number
>> + *
>> + * Return: Always returns 0.
>> + */
>> +static int xilinx_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
>> + irq_hw_number_t hwirq)
>> +{
>> + irq_set_chip_and_handler(irq, &xilinx_msi_irq_chip, handle_simple_irq);
>> + irq_set_chip_data(irq, domain->host_data);
>> + set_irq_flags(irq, IRQF_VALID);
>> +
>> + return 0;
>> +}
>> +
>> +/* IRQ Domain operations */
>> +static const struct irq_domain_ops msi_domain_ops = {
>> + .map = xilinx_pcie_msi_map,
>> +};
>> +
>> +/**
>> + * 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);
>> +}
>> +
>> +/**
>> + * xilinx_pcie_add_bus - Add MSI chip info to PCIe bus
>> + * @bus: PCIe bus
>> + */
>> +static void xilinx_pcie_add_bus(struct pci_bus *bus)
>> +{
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
>> +
>> + if (!port)
>> + return;
>> +
>> + xilinx_pcie_msi_chip.dev = port->dev;
>> + bus->msi = &xilinx_pcie_msi_chip;
>> + }
>> +}
>> +
>> +/* INTx Functions */
>> +
>> +/**
>> + * xilinx_pcie_intx_map - Set the handler for the INTx and mark IRQ as valid
>> + * @domain: IRQ domain
>> + * @irq: Virtual IRQ number
>> + * @hwirq: HW interrupt number
>> + *
>> + * Return: Always returns 0.
>> + */
>> +static int xilinx_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
>> + irq_hw_number_t hwirq)
>> +{
>> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
>> + irq_set_chip_data(irq, domain->host_data);
>> + set_irq_flags(irq, IRQF_VALID);
>> +
>> + return 0;
>> +}
>> +
>> +/* INTx IRQ Domain operations */
>> +static const struct irq_domain_ops intx_domain_ops = {
>> + .map = xilinx_pcie_intx_map,
>> +};
>> +
>> +/* PCIe HW Functions */
>> +
>> +/**
>> + * xilinx_pcie_intr_handler - Interrupt Service Handler
>> + * @irq: IRQ number
>> + * @data: PCIe port information
>> + *
>> + * Return: IRQ_HANDLED on success and IRQ_NONE on failure
>> + */
>> +static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data)
>> +{
>> + struct xilinx_pcie_port *port = (struct xilinx_pcie_port *)data;
>> + u32 val, mask, status, msi_data;
>> +
>> + /* Read interrupt decode and mask registers */
>> + val = pcie_read(port, XILINX_PCIE_REG_IDR);
>> + mask = pcie_read(port, XILINX_PCIE_REG_IMR);
>> +
>> + status = val & mask;
>> + if (!status)
>> + return IRQ_NONE;
>> +
>> + if (status & XILINX_PCIE_INTR_LINK_DOWN)
>> + dev_warn(port->dev, "Link Down\n");
>> +
>> + if (status & XILINX_PCIE_INTR_ECRC_ERR)
>> + dev_warn(port->dev, "ECRC failed\n");
>> +
>> + if (status & XILINX_PCIE_INTR_STR_ERR)
>> + dev_warn(port->dev, "Streaming error\n");
>> +
>> + if (status & XILINX_PCIE_INTR_HOT_RESET)
>> + dev_info(port->dev, "Hot reset\n");
>> +
>> + if (status & XILINX_PCIE_INTR_CFG_TIMEOUT)
>> + dev_warn(port->dev, "ECAM access timeout\n");
>> +
>> + if (status & XILINX_PCIE_INTR_CORRECTABLE) {
>> + dev_warn(port->dev, "Correctable error message\n");
>> + xilinx_pcie_clear_err_interrupts(port);
>> + }
>> +
>> + if (status & XILINX_PCIE_INTR_NONFATAL) {
>> + dev_warn(port->dev, "Non fatal error message\n");
>> + xilinx_pcie_clear_err_interrupts(port);
>> + }
>> +
>> + if (status & XILINX_PCIE_INTR_FATAL) {
>> + dev_warn(port->dev, "Fatal error message\n");
>> + xilinx_pcie_clear_err_interrupts(port);
>> + }
>> +
>> + if (status & XILINX_PCIE_INTR_INTX) {
>> + /* INTx interrupt received */
>> + val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
>> +
>> + /* Check whether interrupt valid */
>> + if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
>> + dev_warn(port->dev, "RP Intr FIFO1 read error\n");
>> + return IRQ_HANDLED;
>> + }
>> +
>> + /* Clear interrupt FIFO register 1 */
>> + pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
>> + XILINX_PCIE_REG_RPIFR1);
>> +
>> + /* Handle INTx Interrupt */
>> + val = ((val & XILINX_PCIE_RPIFR1_INTR_MASK) >>
>> + XILINX_PCIE_RPIFR1_INTR_SHIFT) + 1;
>> + generic_handle_irq(irq_find_mapping(port->irq_domain, val));
>> + }
>> +
>> + if (status & XILINX_PCIE_INTR_MSI) {
>> + /* MSI Interrupt */
>> + val = pcie_read(port, XILINX_PCIE_REG_RPIFR1);
>> +
>> + if (!(val & XILINX_PCIE_RPIFR1_INTR_VALID)) {
>> + dev_warn(port->dev, "RP Intr FIFO1 read error\n");
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (val & XILINX_PCIE_RPIFR1_MSI_INTR) {
>> + msi_data = pcie_read(port, XILINX_PCIE_REG_RPIFR2) &
>> + XILINX_PCIE_RPIFR2_MSG_DATA;
>> +
>> + /* Clear interrupt FIFO register 1 */
>> + pcie_write(port, XILINX_PCIE_RPIFR1_ALL_MASK,
>> + XILINX_PCIE_REG_RPIFR1);
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + /* Handle MSI Interrupt */
>> + generic_handle_irq(msi_data);
>> + }
>> + }
>> + }
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_UNSUPP)
>> + dev_warn(port->dev, "Slave unsupported request\n");
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_UNEXP)
>> + dev_warn(port->dev, "Slave unexpected completion\n");
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_COMPL)
>> + dev_warn(port->dev, "Slave completion timeout\n");
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_ERRP)
>> + dev_warn(port->dev, "Slave Error Poison\n");
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_CMPABT)
>> + dev_warn(port->dev, "Slave Completer Abort\n");
>> +
>> + if (status & XILINX_PCIE_INTR_SLV_ILLBUR)
>> + dev_warn(port->dev, "Slave Illegal Burst\n");
>> +
>> + if (status & XILINX_PCIE_INTR_MST_DECERR)
>> + dev_warn(port->dev, "Master decode error\n");
>> +
>> + if (status & XILINX_PCIE_INTR_MST_SLVERR)
>> + dev_warn(port->dev, "Master slave error\n");
>> +
>> + if (status & XILINX_PCIE_INTR_MST_ERRP)
>> + dev_warn(port->dev, "Master error poison\n");
>> +
>> + /* Clear the Interrupt Decode register */
>> + pcie_write(port, status, XILINX_PCIE_REG_IDR);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_free_irq_domain - Free IRQ domain
>> + * @port: PCIe port information
>> + */
>> +static void xilinx_pcie_free_irq_domain(struct xilinx_pcie_port *port)
>> +{
>> + int i;
>> + u32 irq, num_irqs;
>> +
>> + /* Free IRQ Domain */
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + free_pages(port->msg_addr, 0);
>> +
>> + num_irqs = XILINX_NUM_MSI_IRQS;
>> + } else {
>> + /* INTx */
>> + num_irqs = 4;
>> + }
>> +
>> + for (i = 0; i < num_irqs; i++) {
>> + irq = irq_find_mapping(port->irq_domain, i);
>> + if (irq > 0)
>> + irq_dispose_mapping(irq);
>> + }
>> +
>> + irq_domain_remove(port->irq_domain);
>> +}
>> +
>> +/**
>> + * 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_domain, i);
>> +
>> + xilinx_pcie_enable_msi(port);
>> + } else {
>
> This means that you can't set up INTx if CONFIG_PCI_MSI is defined. Is
> that what you want? The hardware doesn't know whether we have
> CONFIG_PCI_MSI defined, of course, so I would think it would still support
> INTx either way.
Thanks for pointing this. I will fix it in my v4.
>
>> + /* Setup INTx */
>> + struct device_node *pcie_intc_node =
>> + of_get_next_child(node, NULL);
>> +
>> + if (!pcie_intc_node) {
>> + dev_err(dev, "No PCIe Intc node found\n");
>> + return PTR_ERR(pcie_intc_node);
>> + }
>> +
>> + port->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
>> + &intx_domain_ops,
>> + port);
>> + if (!port->irq_domain) {
>> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
>> + return PTR_ERR(port->irq_domain);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_init_port - Initialize hardware
>> + * @port: PCIe port information
>> + */
>> +static void xilinx_pcie_init_port(struct xilinx_pcie_port *port)
>> +{
>> + /* Check if PCIe link is up? */
>
> Superfluous comment, since it's obvious from the function name and dev_info
> text.
Ok.
>
>> + port->link_up = xilinx_pcie_is_link_up(port);
>> + if (!port->link_up)
>> + dev_info(port->dev, "PCIe Link is DOWN\n");
>> + else
>> + dev_info(port->dev, "PCIe Link is UP\n");
>> +
>> + /* Disable all interrupts*/
>> + pcie_write(port, ~XILINX_PCIE_IDR_ALL_MASK,
>> + XILINX_PCIE_REG_IMR);
>> +
>> + /* Clear pending interrupts */
>> + pcie_write(port, pcie_read(port, XILINX_PCIE_REG_IDR) &
>> + XILINX_PCIE_IMR_ALL_MASK,
>> + XILINX_PCIE_REG_IDR);
>> +
>> + /* Enable all interrupts*/
>> + pcie_write(port, XILINX_PCIE_IMR_ALL_MASK, XILINX_PCIE_REG_IMR);
>> +
>> + /* Enable the Bridge enable bit */
>> + pcie_write(port, pcie_read(port, XILINX_PCIE_REG_RPSC) |
>> + XILINX_PCIE_REG_RPSC_BEN,
>> + XILINX_PCIE_REG_RPSC);
>> +}
>> +
>> +/**
>> + * xilinx_pcie_setup - Setup memory resources
>> + * @nr: Bus number
>> + * @sys: Per controller structure
>> + *
>> + * Return: '1' on success and error value on failure
>> + */
>> +static int xilinx_pcie_setup(int nr, struct pci_sys_data *sys)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(sys);
>> + struct device *dev = port->dev;
>> + struct device_node *node = dev->of_node;
>> + struct resource *mem;
>> + resource_size_t offset;
>> + struct of_pci_range_parser parser;
>> + struct of_pci_range range;
>> + struct pci_host_bridge_window *win;
>> + int err = 0, mem_resno = 0;
>> +
>> + /* Get the ranges */
>> + if (of_pci_range_parser_init(&parser, node)) {
>> + dev_err(dev, "missing \"ranges\" property\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Parse the ranges and add the resources found to the list */
>> + for_each_of_pci_range(&parser, &range) {
>> +
>> + if (mem_resno >= XILINX_MAX_NUM_RESOURCES) {
>> + dev_err(dev, "Maximum memory resources exceeded\n");
>> + return -EINVAL;
>> + }
>> +
>> + mem = devm_kmalloc(dev, sizeof(*mem), GFP_KERNEL);
>> + if (!mem) {
>> + err = -ENOMEM;
>> + goto free_resources;
>> + }
>> +
>> + of_pci_range_to_resource(&range, node, mem);
>> +
>> + switch (mem->flags & IORESOURCE_TYPE_BITS) {
>> + case IORESOURCE_MEM:
>> + offset = range.cpu_addr - range.pci_addr;
>> + mem_resno++;
>> + break;
>> + default:
>> + err = -EINVAL;
>> + break;
>> + }
>> +
>> + if (err < 0) {
>> + dev_warn(dev, "Invalid resource found\n");
>
> It'd be nice for debugging to print something useful here, e.g,
> %pR of "mem".
Ok.
>
>> + continue;
>> + }
>> +
>> + err = request_resource(&iomem_resource, mem);
>> + if (err)
>> + goto free_resources;
>> +
>> + pci_add_resource_offset(&sys->resources, mem, offset);
>> + }
>> +
>> + return 1;
>> +
>> +free_resources:
>> + release_child_resources(&iomem_resource);
>> + list_for_each_entry(win, &sys->resources, list)
>> + devm_kfree(dev, win->res);
>> + pci_free_resource_list(&sys->resources);
>> +
>> + return err;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_scan_bus - Scan PCIe bus for devices
>> + * @nr: Bus number
>> + * @sys: Per controller structure
>> + *
>> + * Return: Valid Bus pointer on success and NULL on failure
>> + */
>> +static struct pci_bus __init *xilinx_pcie_scan_bus(int nr,
>> + struct pci_sys_data *sys)
>> +{
>> + struct xilinx_pcie_port *port = sys_to_pcie(sys);
>> + struct pci_bus *bus;
>> +
>> + if (port) {
>> + port->root_busno = sys->busnr;
>> + bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
>> + sys, &sys->resources);
>
> I don't think you're supplying a bus number resource here, and you should.
> There's a discussion about this issue in a different driver here:
>
> http://lkml.kernel.org/r/1396261856-22465-2-git-send-email-phil.edworthy@renesas.com
Ok.
>
>> + } else {
>> + bus = NULL;
>> + BUG();
>> + }
>> +
>> + return bus;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_parse_dt - Parse Device tree
>> + * @port: PCIe port information
>> + *
>> + * Return: '0' on success and error value on failure
>> + */
>> +static int xilinx_pcie_parse_dt(struct xilinx_pcie_port *port)
>> +{
>> + struct device *dev = port->dev;
>> + struct device_node *node = dev->of_node;
>> + struct resource regs, busn;
>> + const char *type;
>> + int err;
>> +
>> + /* Check for device type */
>> + type = of_get_property(node, "device_type", NULL);
>> + if (!type || strcmp(type, "pci")) {
>> + dev_err(dev, "invalid \"device_type\" %s\n", type);
>> + return -EINVAL;
>> + }
>> +
>> + /* Map the register space */
>> + err = of_address_to_resource(node, 0, ®s);
>> + if (err) {
>> + dev_err(dev, "missing \"reg\" property\n");
>> + return err;
>> + }
>> +
>> + port->reg_base = devm_ioremap_resource(dev, ®s);
>> + if (IS_ERR(port->reg_base))
>> + return PTR_ERR(port->reg_base);
>> +
>> + /* Map IRQ */
>> + port->irq = irq_of_parse_and_map(node, 0);
>> + err = devm_request_irq(dev, port->irq, xilinx_pcie_intr_handler,
>> + IRQF_SHARED, "xilinx-pcie", port);
>> + if (err) {
>> + dev_err(dev, "unable to request irq %d\n", port->irq);
>> + return err;
>> + }
>> +
>> + /* Get the bus range */
>> + if (of_pci_parse_bus_range(node, &busn)) {
>> + u32 val = pcie_read(port, XILINX_PCIE_REG_BIR);
>> + u8 last;
>> +
>> + last = (val & XILINX_PCIE_BIR_ECAM_SZ_MASK) >>
>> + XILINX_PCIE_BIR_ECAM_SZ_SHIFT;
>> +
>> + busn = (struct resource) {
>> + .name = node->name,
>> + .start = 0,
>> + .end = last,
>> + .flags = IORESOURCE_BUS,
>> + };
>
> This doesn't actually *do* anything, does it? You're filling in "busn",
> but that's just a local variable, and I don't see that you save the
> information anywhere.
Will fix.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_probe - Probe function
>> + * @pdev: Platform device pointer
>> + *
>> + * Return: '0' on success and error value on failure
>> + */
>> +static int xilinx_pcie_probe(struct platform_device *pdev)
>> +{
>> + struct xilinx_pcie_port *port;
>> + struct hw_pci hw;
>> + struct device *dev = &pdev->dev;
>> + int err;
>> +
>> + if (!dev->of_node)
>> + return -ENODEV;
>> +
>> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>> + if (!port)
>> + return -ENOMEM;
>> +
>> + port->dev = dev;
>> +
>> + /* Parse the device tree */
>
> All the comments here are superfluous.
Ok, will remove.
>
>> + err = xilinx_pcie_parse_dt(port);
>> + if (err) {
>> + dev_err(dev, "Parsing DT failed\n");
>> + return err;
>> + }
>> +
>> + /* Initialize the hardware */
>> + xilinx_pcie_init_port(port);
>> +
>> + /* Setup IRQ Domain */
>> + err = xilinx_pcie_init_irq_domain(port);
>> + if (err) {
>> + dev_err(dev, "Failed creating IRQ Dommain\n");
>> + return err;
>> + }
>> +
>> + /* Initialize hw_pci structure */
>> + memset(&hw, 0, sizeof(hw));
>> + hw = (struct hw_pci) {
>> + .nr_controllers = 1,
>> + .private_data = (void **)&port,
>> + .setup = xilinx_pcie_setup,
>> + .map_irq = of_irq_parse_and_map_pci,
>> + .add_bus = xilinx_pcie_add_bus,
>> + .scan = xilinx_pcie_scan_bus,
>> + .ops = &xilinx_pcie_ops,
>> + };
>> +
>> + /* Register the device */
>> + pci_common_init_dev(dev, &hw);
>> +
>> + platform_set_drvdata(pdev, port);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * xilinx_pcie_remove - Remove function
>> + * @pdev: Platform device pointer
>> + *
>> + * Return: '0' always
>> + */
>> +static int xilinx_pcie_remove(struct platform_device *pdev)
>> +{
>> + struct xilinx_pcie_port *port = platform_get_drvdata(pdev);
>> +
>> + xilinx_pcie_free_irq_domain(port);
>> +
>> + return 0;
>> +}
>> +
>> +static struct of_device_id xilinx_pcie_of_match[] = {
>> + { .compatible = "xlnx,axi-pcie-host-1.00.a" ,},
>
> The "," inside the closing brace look unnecessary, but if you want to keep
> it, the space should go after the comma instead of before it.
Ok, I will fix all these comments in my v4.
Thanks for the review,
Srikanth
>
>> + {}
>> +};
>> +
>> +static struct platform_driver xilinx_pcie_driver = {
>> + .driver = {
>> + .name = "xilinx-pcie",
>> + .owner = THIS_MODULE,
>> + .of_match_table = xilinx_pcie_of_match,
>> + .suppress_bind_attrs = true,
>> + },
>> + .probe = xilinx_pcie_probe,
>> + .remove = xilinx_pcie_remove,
>> +};
>> +module_platform_driver(xilinx_pcie_driver);
>> +
>> +MODULE_AUTHOR("Xilinx Inc");
>> +MODULE_DESCRIPTION("Xilinx AXI PCIe driver");
>> +MODULE_LICENSE("GPLv2");
>> --
>> 1.7.9.5
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@...r.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists