Hi, Bjorn,
On 06.06.2025 01:57, Bjorn Helgaas wrote:
> On Fri, May 30, 2025 at 02:19:13PM +0300, Claudiu wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>>
>> 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?
The values here were retrieved by experimenting. They are not present in
RZ/G3S specification. I'll look though the header you pointed and use any
defines if they match.
>
>> +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?
I missed adding it as RZ/G3S manual don't mention this delay.
>
>> + 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/
OK.
Thank you for your review,
Claudiu
