[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <lopqatqgd4ql5qdtlph2irw2b4ed6dbnjsgncd7idlsz2tjlhn@rdvfj2ma2wx5>
Date: Tue, 15 Apr 2025 12:48:34 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Sai Krishna Musham <sai.krishna.musham@....com>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kw@...ux.com,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, cassel@...nel.org,
linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
michal.simek@....com, bharat.kumar.gogada@....com, thippeswamy.havalige@....com
Subject: Re: [LINUX PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
PERST# signal
On Mon, Apr 14, 2025 at 08:38:42AM +0530, Sai Krishna Musham wrote:
> Add support for handling the PCIe Root Port (RP) PERST# signal using
> the GPIO framework, along with the PCIe IP reset. This reset is
> managed by the driver and occurs after the Initial Power Up sequence
> (PCIe CEM r6.0, 2.2.1) is handled in hardware before the driver's probe
> function is called.
>
> This reset mechanism is particularly useful in warm reset scenarios,
> where the power rails remain stable and only PERST# signal is toggled
> through the driver. Applying both the PCIe IP reset and the PERST#
> improves the reliability of the reset process by ensuring that both
> the Root Port controller and the Endpoint are reset synchronously
> and avoid lane errors.
>
> Adapt the implementation to use the GPIO framework for reset signal
> handling and make this reset handling optional, along with the
> `cpm_crx` property, to maintain backward compatibility with existing
> device tree binaries (DTBs).
>
> Additionally, clear Firewall after the link reset for CPM5NC to allow
> further PCIe transactions.
>
> Signed-off-by: Sai Krishna Musham <sai.krishna.musham@....com>
> ---
> Changes for v7:
> - Use platform_get_resource_byname() to make cpm_crx and cpm5nc_fw_attr
> optional
> - Use 100us delay T_PERST as per PCIe spec before PERST# deassert.
>
> Changes for v6:
> - Correct version check condition of CPM5NC_HOST.
>
> Changes for v5:
> - Handle probe defer for reset_gpio.
> - Resolve ABI break.
>
> Changes for v4:
> - Add PCIe PERST# support for CPM5NC.
> - Add PCIe IP reset along with PERST# to avoid Link Training Errors.
> - Remove PCIE_T_PVPERL_MS define and PCIE_T_RRS_READY_MS after
> PERST# deassert.
> - Move PCIe PERST# assert and deassert logic to
> xilinx_cpm_pcie_init_port() before cpm_pcie_link_up(), since
> Interrupts enable and PCIe RP bridge enable should be done after
> Link up.
> - Update commit message.
>
> Changes for v3:
> - Use PCIE_T_PVPERL_MS define.
>
> Changes for v2:
> - Make the request GPIO optional.
> - Correct the reset sequence as per PERST#
> - Update commit message
> ---
> drivers/pci/controller/pcie-xilinx-cpm.c | 97 +++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
> index 13ca493d22bd..c46642417d52 100644
> --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -6,6 +6,8 @@
> */
>
> #include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/irqchip.h>
> @@ -21,6 +23,13 @@
> #include "pcie-xilinx-common.h"
>
> /* Register definitions */
> +#define XILINX_CPM_PCIE0_RST 0x00000308
> +#define XILINX_CPM5_PCIE0_RST 0x00000318
> +#define XILINX_CPM5_PCIE1_RST 0x0000031C
> +#define XILINX_CPM5NC_PCIE0_RST 0x00000324
> +
> +#define XILINX_CPM5NC_PCIE0_FRWALL 0x00000140
> +
> #define XILINX_CPM_PCIE_REG_IDR 0x00000E10
> #define XILINX_CPM_PCIE_REG_IMR 0x00000E14
> #define XILINX_CPM_PCIE_REG_PSCR 0x00000E1C
> @@ -93,12 +102,16 @@ enum xilinx_cpm_version {
> * @ir_status: Offset for the error interrupt status register
> * @ir_enable: Offset for the CPM5 local error interrupt enable register
> * @ir_misc_value: A bitmask for the miscellaneous interrupt status
> + * @cpm_pcie_rst: Offset for the PCIe IP reset
> + * @cpm5nc_fw_rst: Offset for the CPM5NC Firewall
> */
> struct xilinx_cpm_variant {
> enum xilinx_cpm_version version;
> u32 ir_status;
> u32 ir_enable;
> u32 ir_misc_value;
> + u32 cpm_pcie_rst;
> + u32 cpm5nc_fw_rst;
> };
>
> /**
> @@ -106,6 +119,8 @@ struct xilinx_cpm_variant {
> * @dev: Device pointer
> * @reg_base: Bridge Register Base
> * @cpm_base: CPM System Level Control and Status Register(SLCR) Base
> + * @crx_base: CPM Clock and Reset Control Registers Base
> + * @cpm5nc_fw_base: CPM5NC Firewall Attribute Base
> * @intx_domain: Legacy IRQ domain pointer
> * @cpm_domain: CPM IRQ domain pointer
> * @cfg: Holds mappings of config space window
> @@ -118,6 +133,8 @@ struct xilinx_cpm_pcie {
> struct device *dev;
> void __iomem *reg_base;
> void __iomem *cpm_base;
> + void __iomem *crx_base;
> + void __iomem *cpm5nc_fw_base;
> struct irq_domain *intx_domain;
> struct irq_domain *cpm_domain;
> struct pci_config_window *cfg;
> @@ -475,12 +492,57 @@ static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie *port)
> * xilinx_cpm_pcie_init_port - Initialize hardware
> * @port: PCIe port information
> */
> -static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port)
> +static int xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port)
> {
> const struct xilinx_cpm_variant *variant = port->variant;
> + struct device *dev = port->dev;
> + struct gpio_desc *reset_gpio;
> + bool do_reset = false;
> +
> + if (port->crx_base && (variant->version < CPM5NC_HOST ||
> + (variant->version == CPM5NC_HOST &&
> + port->cpm5nc_fw_base))) {
> + /* Request the GPIO for PCIe reset signal and assert */
> + reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(reset_gpio),
> + "Failed to request reset GPIO\n");
> + if (reset_gpio)
> + do_reset = true;
> + }
> +
> + if (do_reset) {
> + /* Assert the PCIe IP reset */
> + writel_relaxed(0x1, port->crx_base + variant->cpm_pcie_rst);
> +
> + /*
> + * "PERST# active time", as per Table 2-10: Power Sequencing
> + * and Reset Signal Timings of the PCIe Electromechanical
> + * Specification, Revision 6.0, symbol "T_PERST".
> + */
> + udelay(100);
You don't need this delay unless the above 'cpm_pcie_rst' controls refclk.
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists