Re: [PATCH] PCI: rcar-gen4: Fix inverted break condition in PHY initialization

[Marek Vasut <marek.vasut@mailbox.org>]

On 9/16/25 11:59 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hello Geert,

> On Tue, 16 Sept 2025 at 01:59, Marek Vasut
> <marek.vasut+renesas@mailbox.org> wrote:
>> R-Car V4H Reference Manual R19UH0186EJ0130 Rev.1.30 Apr. 21, 2025 page 4581
>> Figure 104.3b Initial Setting of PCIEC(example), third quarter of the figure
>> indicates that register 0xf8 should be polled until bit 18 becomes set to 1.
>>
>> Register 0xf8 bit 18 is 0 immediately after write to PCIERSTCTRL1 and is set
>> to 1 in less than 1 ms afterward. The current readl_poll_timeout() break
>> condition is inverted and returns when register 0xf8 bit 18 is set to 0,
>> which in most cases means immediately. In case CONFIG_DEBUG_LOCK_ALLOC=y ,
>> the timing changes just enough for the first readl_poll_timeout() poll to
>> already read register 0xf8 bit 18 as 1 and afterward never read register
>> 0xf8 bit 18 as 0, which leads to timeout and failure to start the PCIe
>> controller.
>>
>> Fix this by inverting the poll condition to match the reference manual
>> initialization sequence.
>>
>> Fixes: faf5a975ee3b ("PCI: rcar-gen4: Add support for R-Car V4H")
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> 
> Thanks for your patch!
> 
>> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>> @@ -711,7 +711,7 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
>>          val &= ~APP_HOLD_PHY_RST;
>>          writel(val, rcar->base + PCIERSTCTRL1);
>>
>> -       ret = readl_poll_timeout(rcar->phy_base + 0x0f8, val, !(val & BIT(18)), 100, 10000);
>> +       ret = readl_poll_timeout(rcar->phy_base + 0x0f8, val, val & BIT(18), 100, 10000);
>>          if (ret < 0)
>>                  return ret;
>>
> 
> This change looks correct, and fixes the timeout seen on White Hawk
> with CONFIG_DEBUG_LOCK_ALLOC=y.
> However, it causes a crash when CONFIG_DEBUG_LOCK_ALLOC=n:
> 
>      SError Interrupt on CPU0, code 0x00000000be000011 -- SError

...

>       el1h_64_error_handler+0x2c/0x40
>       el1h_64_error+0x70/0x74
>       dw_pcie_read+0x20/0x74 (P)
>       rcar_gen4_pcie_additional_common_init+0x1c/0x6c

SError in rcar_gen4_pcie_additional_common_init , this is unrelated to 
this fix.

Does the following patch make this SError go away ?

"
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c 
b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index d787f2a364d5..5932f83996f0 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -204,6 +204,8 @@ static int rcar_gen4_pcie_common_init(struct 
rcar_gen4_pcie *rcar)
         if (ret)
                 goto err_unprepare;

+mdelay(1);
+
         if (rcar->drvdata->additional_common_init)
                 rcar->drvdata->additional_common_init(rcar);

"