Re: [PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller
Hi Pratyush,
Thanks for the review.
Best regards
Gabriel
On 27 August 2015 at 19:31, Pratyush Anand wrote:
> Hi Gabriel,
>
> Looks good to me.
>
> On Thu, Aug 27, 2015 at 6:04 PM, Gabriel Fernandez
> wrote:
>> sti pcie is built around a Synopsis Designware PCIe IP.
>>
>> Signed-off-by: Fabrice Gasnier
>> Signed-off-by: Gabriel Fernandez
>
>
>> +static int st_pcie_link_up(struct pcie_port *pp)
>> +{
>> + u32 status;
>> + int link_up;
>
> nit: why not bool
i prefer to keep it as 'int' because the prototype of link_up callback
is an 'int'.
>
>> + int count = 0;
>
> [...]
>
>> +static void st_pcie_board_reset(struct pcie_port *pp)
>> +{
>> + struct st_pcie *pcie = to_st_pcie(pp);
>> +
>> + if (!gpio_is_valid(pcie->reset_gpio))
>> + return;
>> +
>> + if (gpio_direction_output(pcie->reset_gpio, 0)) {
>> + dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
>> + pcie->reset_gpio);
>> + return;
>> + }
>> +
>> + /* From PCIe spec */
>> + msleep(2);
>> + gpio_direction_output(pcie->reset_gpio, 1);
>> +
>> + /*
>> +* PCIe specification states that you should not issue any config
>> +* requests until 100ms after asserting reset, so we enforce that
>> here
>> +*/
>> + msleep(100);
>
> IIRC, specification says to wait after link training completes. So
> shouldn't it be after st_pcie_enable_ltssm. Moreover, I wonder why
> others do not need it.
>
Ok i will fix it.
> Reviewed-by: Pratyush Anand
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller
Hi Gabriel,
Looks good to me.
On Thu, Aug 27, 2015 at 6:04 PM, Gabriel Fernandez
wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier
> Signed-off-by: Gabriel Fernandez
> +static int st_pcie_link_up(struct pcie_port *pp)
> +{
> + u32 status;
> + int link_up;
nit: why not bool
> + int count = 0;
[...]
> +static void st_pcie_board_reset(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + if (!gpio_is_valid(pcie->reset_gpio))
> + return;
> +
> + if (gpio_direction_output(pcie->reset_gpio, 0)) {
> + dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
> + pcie->reset_gpio);
> + return;
> + }
> +
> + /* From PCIe spec */
> + msleep(2);
> + gpio_direction_output(pcie->reset_gpio, 1);
> +
> + /*
> +* PCIe specification states that you should not issue any config
> +* requests until 100ms after asserting reset, so we enforce that here
> +*/
> + msleep(100);
IIRC, specification says to wait after link training completes. So
shouldn't it be after st_pcie_enable_ltssm. Moreover, I wonder why
others do not need it.
Reviewed-by: Pratyush Anand
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] PCI: st: Provide support for the sti PCIe controller
sti pcie is built around a Synopsis Designware PCIe IP. Signed-off-by: Fabrice Gasnier Signed-off-by: Gabriel Fernandez --- drivers/pci/host/Kconfig | 9 + drivers/pci/host/Makefile | 1 + drivers/pci/host/pci-st.c | 583 ++ 3 files changed, 593 insertions(+) create mode 100644 drivers/pci/host/pci-st.c diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index c132bdd..db56b8f 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -145,4 +145,13 @@ config PCIE_IPROC_BCMA Say Y here if you want to use the Broadcom iProc PCIe controller through the BCMA bus interface +config PCI_ST + bool "ST PCIe controller" + depends on ARCH_STI || (ARM && COMPILE_TEST) + select PCIE_DW + help + Enable PCIe controller support on ST Socs. This controller is based + on Designware hardware and therefore the driver re-uses the + Designware core functions to implement the driver. + endmenu diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 140d66f..c4024fa 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o +obj-$(CONFIG_PCI_ST) += pci-st.o diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c new file mode 100644 index 000..0e7aaa2 --- /dev/null +++ b/drivers/pci/host/pci-st.c @@ -0,0 +1,583 @@ +/* + * Copyright (C) 2014 STMicroelectronics + * + * STMicroelectronics PCI express Driver for sti SoCs. + * ST PCIe IPs are built around a Synopsys IP Core. + * + * Author: Fabrice Gasnier + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, as + * published by the Free Software Foundation. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "pcie-designware.h" + +#define TRANSLATION_CONTROL0x900 +/* Controls if area is inclusive or exclusive */ +#define RC_PASS_ADDR_RANGE BIT(1) + +/* Base of area reserved for config accesses. Fixed size of 64K. */ +#define CFG_BASE_ADDRESS 0x92c +#define CFG_REGION_SIZE65536 +#define CFG_SPACE1_OFFSET 0x1000 + +/* First 4K of config space has this BDF (bus,device,function) */ +#define FUNC0_BDF_NUM 0x930 + +/* Mem regions */ +#define IN0_MEM_ADDR_START 0x964 +#define IN0_MEM_ADDR_LIMIT 0x968 +#define IN1_MEM_ADDR_START 0x974 +#define IN1_MEM_ADDR_LIMIT 0x978 + +/* This actually contains the LTSSM state machine state */ +#define PORT_LOGIC_DEBUG_REG_0 0x728 + +/* LTSSM state machine values */ +#define DEBUG_REG_0_LTSSM_MASK 0x1f +#define S_DETECT_QUIET 0x00 +#define S_DETECT_ACT 0x01 +#define S_POLL_ACTIVE 0x02 +#define S_POLL_COMPLIANCE 0x03 +#define S_POLL_CONFIG 0x04 +#define S_PRE_DETECT_QUIET 0x05 +#define S_DETECT_WAIT 0x06 +#define S_CFG_LINKWD_START 0x07 +#define S_CFG_LINKWD_ACEPT 0x08 +#define S_CFG_LANENUM_WAIT 0x09 +#define S_CFG_LANENUM_ACEPT0x0A +#define S_CFG_COMPLETE 0x0B +#define S_CFG_IDLE 0x0C +#define S_RCVRY_LOCK 0x0D +#define S_RCVRY_SPEED 0x0E +#define S_RCVRY_RCVRCFG0x0F +#define S_RCVRY_IDLE 0x10 +#define S_L0 0x11 +#define S_L0S 0x12 +#define S_L123_SEND_EIDLE 0x13 +#define S_L1_IDLE 0x14 +#define S_L2_IDLE 0x15 +#define S_L2_WAKE 0x16 +#define S_DISABLED_ENTRY 0x17 +#define S_DISABLED_IDLE0x18 +#define S_DISABLED 0x19 +#define S_LPBK_ENTRY 0x1A +#define S_LPBK_ACTIVE 0x1B +#define S_LPBK_EXIT0x1C +#define S_LPBK_EXIT_TIMEOUT0x1D +#define S_HOT_RESET_ENTRY 0x1E +#define S_HOT_RESET0x1F + +/* syscfg bits */ +#define PCIE_SYS_INT BIT(5) +#define PCIE_APP_REQ_RETRY_EN BIT(3) +#define PCIE_APP_LTSSM_ENABLE BIT(2) +#define PCIE_APP_INIT_RST BIT(1) +#define PCIE_DEVICE_TYPE BIT(0) +#define PCIE_DEFAULT_VAL PCIE_DEVICE_TYPE + +/* Time to wait between testing the link in msecs (hardware poll interval) */ +#define LINK_LOOP_DELAY_MS 1 +/* Total amount of time to wait for the link to come up in msecs */
