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

Mykyta Poturai posted 7 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH v2 5/7] xen/arm: rcar4: add delay after programming ATU
Posted by Mykyta Poturai 9 months, 1 week ago
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);
     /*
      * Make sure ATU enable takes effect before any subsequent config
      * and I/O accesses.
-- 
2.34.1
Re: [PATCH v2 5/7] xen/arm: rcar4: add delay after programming ATU
Posted by Grygorii Strashko 9 months, 1 week ago

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
Re: [PATCH v2 5/7] xen/arm: rcar4: add delay after programming ATU
Posted by Mykyta Poturai 9 months, 1 week ago

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.

-- 
Mykyta