Re: [PATCH v2 4/8] PCI: rzg3s-host: Add Initial PCIe Host Driver for Renesas RZ/G3S SoC

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, May 30, 2025 at 02:19:13PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The Renesas RZ/G3S features a PCIe IP that complies with the PCI Express
> Base Specification 4.0 and supports speeds of up to 5 GT/s. It functions
> only as a root complex, with a single-lane (x1) configuration. The
> controller includes Type 1 configuration registers, as well as IP
> specific registers (called AXI registers) required for various adjustments.

> +/* Timeouts */
> +#define RZG3S_REQ_ISSUE_TIMEOUT_US		2500
> +#define RZG3S_LTSSM_STATE_TIMEOUT_US		1000
> +#define RZG3S_LS_CHANGE_TIMEOUT_US		1000
> +#define RZG3S_LINK_UP_TIMEOUT_US		500000

Are any of these timeouts related to values in the PCIe spec?  If so,
use #defines from drivers/pci/pci.h, or add a new one if needed.

If they come from the RZ/G3S spec, can you include citations?

> +static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host, bool probe)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/* Initialize the PCIe related registers */
> +	ret = rzg3s_pcie_config_init(host);
> +	if (ret)
> +		return ret;
> +
> +	/* Initialize the interrupts */
> +	rzg3s_pcie_irq_init(host);
> +
> +	ret = reset_control_bulk_deassert(host->data->num_cfg_resets,
> +					  host->cfg_resets);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for link up */
> +	ret = readl_poll_timeout(host->axi + RZG3S_PCI_PCSTAT1, val,
> +				 !(val & RZG3S_PCI_PCSTAT1_DL_DOWN_STS), 5000,
> +				 RZG3S_LINK_UP_TIMEOUT_US);

Where do we wait for PCIE_T_RRS_READY_MS before pci_host_probe()
starts issuing config requests to enumerate devices?

> +	if (ret) {
> +		reset_control_bulk_assert(host->data->num_cfg_resets,
> +					  host->cfg_resets);
> +		return ret;
> +	}
> +
> +	val = readl(host->axi + RZG3S_PCI_PCSTAT2);
> +	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
> +
> +	val = FIELD_GET(RZG3S_PCI_PCSTAT2_STATE_RX_DETECT, val);
> +	dev_info(host->dev, "PCIe x%d: link up\n", hweight32(val));
> +
> +	if (probe) {
> +		ret = devm_add_action_or_reset(host->dev,
> +					       rzg3s_pcie_cfg_resets_action,
> +					       host);
> +	}
> +
> +	return ret;
> +}

> +		 * According to the RZ/G3S HW manual (Rev.1.10, section
> +		 * 34.3.1.71 AXI Window Mask (Lower) Registers) HW expects first
> +		 * 12 LSB bits to be 0xfff. Extract 1 from size for this.

s/Extract/Subtract/

> +		 */
> +		size = roundup_pow_of_two(size) - 1;