Re: [PATCH] PCI: dwc: rcar-gen4: Fix potential unclocked access in rcar_gen4_pcie_ep_deinit()

On 8/19/26 10:48 AM, Koichiro Den wrote:

Hello Den-san,

>> Finally, if dw_pcie_ep_init() call in rcar_gen4_add_dw_pcie_ep() fails, then
>> with the aforementioned deinit implementation, I think it will be possible
>> to simply do the following to cover (**).
>>
>> ret = dw_pcie_ep_init(ep);
>> if (ret)
>>    return ret;
>>
>> (I am also attaching the entire example as a diff, compile tested only thus
>> far)
>>
>> What do you think ?
> 
> Hi Marek,
> 
> Thanks for sharing your thoughts.
> 
> I think the callback idea is much cleaner. I wonder if .post_deinit might
> describe its role more clearly.

I think it does, and I will include that in V2.

> P.S. I took a quick look through the related call paths, and it made me wonder
> if we could take this a little further. Perhaps dw_pcie_ep_deinit() could call
> the same hook at the end (I use .post_deinit as a tentative name below):
> 
>    dw_pcie_ep_init()
>      -> .pre_init()
>      -> generic intialization
>      error:
>        -> generic unwind
>        -> .post_deinit()
> 
>    dw_pcie_ep_deinit()
>      -> generic teardown
>      -> .post_deinit()
> 
> Then would let the generic init/deinit pair own both hooks, and the
> pcie-rcar-gen4 driver would no longer need to call rcar_gen4_pcie_ep_deinit()
> separately. After dropping those explicit calls, the function could naturally be
> renamed rcar_gen4_pcie_ep_post_deinit() and be used only as the hook.

That is really good.

> This might be too much for this small fix. Even without this wider change,
> however, I think rcar_gen4_pcie_ep_pre_deinit() should do the same operations as
> rcar_gen4_pcie_ep_deinit(), rather than only calling common_deinit(). For
> example, if pci_epc_mem_init() fails after .pre_init() succeeds, PCIEDMAINTSTSEN
> is already 0xffff, and common_deinit() does not clear it.

I will send a proper V2 shortly to cater for this. Thank you !

-- 
Best regards,
Marek Vasut