diff mbox series

[v2,5/7] xen/arm: rcar4: add delay after programming ATU

Message ID 9cd78a64dde2e0a039919a08025abaa89d63966c.1741596512.git.mykyta_poturai@epam.com (mailing list archive)
State New
Headers show
Series Add support for R-Car Gen4 PCI host controller | expand

Commit Message

Mykyta Poturai March 11, 2025, 10:24 a.m. UTC
From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

For some reason, we need a delay before accessing ATU region after
we programmed it. Otherwise, we'll get erroneous TLP.

There is a code below, which should do this in proper way, by polling
CTRL2 register, but according to documentation, hardware does not
change this ATU_ENABLE bit at all.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
v1->v2:
* rebased
---
 xen/arch/arm/pci/pci-designware.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Grygorii Strashko March 12, 2025, 10:12 a.m. UTC | #1
On 11.03.25 12:24, Mykyta Poturai wrote:
> From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> For some reason, we need a delay before accessing ATU region after
> we programmed it. Otherwise, we'll get erroneous TLP.
> 
> There is a code below, which should do this in proper way, by polling
> CTRL2 register, but according to documentation, hardware does not
> change this ATU_ENABLE bit at all.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> ---
> v1->v2:
> * rebased
> ---
>   xen/arch/arm/pci/pci-designware.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/pci/pci-designware.c b/xen/arch/arm/pci/pci-designware.c
> index 6ab03cf9b0..def2c12d63 100644
> --- a/xen/arch/arm/pci/pci-designware.c
> +++ b/xen/arch/arm/pci/pci-designware.c
> @@ -194,6 +194,11 @@ static void dw_pcie_prog_outbound_atu_unroll(struct pci_host_bridge *pci,
>       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>                                PCIE_ATU_ENABLE);
>   
> +    /*
> +     * HACK: We need to delay there, because the next code does not
> +     * work as expected on S4
> +     */
> +    mdelay(1);

It seems like a HACK to WA some platform issue, but in its current form it
will affect all DW PCI compatible platforms.

So, it is required a proper solution for the affected platform to be found, or
some sort of DW PCI "quirk"s processing code be introduced.

I'd recommend to drop this patch for now from this series.

>       /*
>        * Make sure ATU enable takes effect before any subsequent config
>        * and I/O accesses.

BR,
-grygorii
Mykyta Poturai March 12, 2025, 11:29 a.m. UTC | #2
On 12.03.25 12:12, Grygorii Strashko wrote:
> 
> 
> On 11.03.25 12:24, Mykyta Poturai wrote:
>> From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>
>> For some reason, we need a delay before accessing ATU region after
>> we programmed it. Otherwise, we'll get erroneous TLP.
>>
>> There is a code below, which should do this in proper way, by polling
>> CTRL2 register, but according to documentation, hardware does not
>> change this ATU_ENABLE bit at all.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
>> ---
>> v1->v2:
>> * rebased
>> ---
>>   xen/arch/arm/pci/pci-designware.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/xen/arch/arm/pci/pci-designware.c 
>> b/xen/arch/arm/pci/pci-designware.c
>> index 6ab03cf9b0..def2c12d63 100644
>> --- a/xen/arch/arm/pci/pci-designware.c
>> +++ b/xen/arch/arm/pci/pci-designware.c
>> @@ -194,6 +194,11 @@ static void 
>> dw_pcie_prog_outbound_atu_unroll(struct pci_host_bridge *pci,
>>       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>>                                PCIE_ATU_ENABLE);
>> +    /*
>> +     * HACK: We need to delay there, because the next code does not
>> +     * work as expected on S4
>> +     */
>> +    mdelay(1);
> 
> It seems like a HACK to WA some platform issue, but in its current form it
> will affect all DW PCI compatible platforms.
> 
> So, it is required a proper solution for the affected platform to be 
> found, or
> some sort of DW PCI "quirk"s processing code be introduced.
> 
> I'd recommend to drop this patch for now from this series.
> 
>>       /*
>>        * Make sure ATU enable takes effect before any subsequent config
>>        * and I/O accesses.
> 
> BR,
> -grygorii

Hi Grygorii

After some further investigations I have retested this on V4H, and it 
seems to work fine without delays. Considering this and the issue with 
static variables and multiple PCI hosts I think I will drop ATU related 
workarounds until some proper quirks handling system is established.
diff mbox series

Patch

diff --git a/xen/arch/arm/pci/pci-designware.c b/xen/arch/arm/pci/pci-designware.c
index 6ab03cf9b0..def2c12d63 100644
--- a/xen/arch/arm/pci/pci-designware.c
+++ b/xen/arch/arm/pci/pci-designware.c
@@ -194,6 +194,11 @@  static void dw_pcie_prog_outbound_atu_unroll(struct pci_host_bridge *pci,
     dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
                              PCIE_ATU_ENABLE);
 
+    /*
+     * HACK: We need to delay there, because the next code does not
+     * work as expected on S4
+     */
+    mdelay(1);
     /*
      * Make sure ATU enable takes effect before any subsequent config
      * and I/O accesses.