Re: [PATCH v2] PCI: dw-rockchip: Add system PM support

[Niklas Cassel <cassel@kernel.org>]

Hello Nicolas,

On Thu, Apr 17, 2025 at 03:24:34PM +0200, Nicolas Frattaroli wrote:
> On Tuesday, 15 April 2025 15:09:29 Central European Summer Time Niklas Cassel wrote:
> > On Fri, Apr 11, 2025 at 02:14:08PM +0800, Shawn Lin wrote:
> > > [...]
> > > +      rockchip_pcie_ltssm_enable_control_mode(rockchip, PCIE_CLIENT_RC_MODE);
> > 
> > Here you are setting PCIE_CLIENT_RC_MODE unconditionally.
> > 
> > I really don't think that you have tested these callbacks with EP mode.
> 
> Hi Niklas,
> 
> I may be reading too much into your tone here, but I think it'd be good if
> you didn't formulate this in such a passive-aggressive accusatory way. You
> can just express your concern as a question about whether this was tested
> with EP mode.

I provided a suggestion further down in the same email
(perhaps split the driver to RC and EP part like the qcom driver).

I also provided a further suggestion here:
https://lore.kernel.org/linux-pci/aADdI7ByEImYy3Pq@ryzen/
(perhaps do like the tegra driver and let the callback return -ENOTSUPP
if running in EP mode.)


> 
> After all, I'm giving you specifically the same benefit of the doubt with
> RC mode that has broken BAR resource mapping on RK3588 in timing-related
> ways in v6.15-rc that has already taken me about a day of unreliable
> bisects to try and track down, and may in fact end up bisecting to one of
> your recent commits touching that part.

It would have been nice if you waited until your bisect was done,
so you could have provided some facts, rather than speculation.

Nonetheless, may I suggest that you try with Shawn's recent patch:
https://lore.kernel.org/linux-pci/aACaupQvmiiBE29l@ryzen/T/#m5ec27d0ee4d2345dd4539385b3c7c8f6b98ee72e
which fixes the .link_up() callback.

The link_up() callback is used e.g. by dw_pcie_other_conf_map_bus():
https://github.com/torvalds/linux/blob/v6.15-rc2/drivers/pci/controller/dwc/pcie-designware-host.c#L622-L631

So bad things could happen if this callback is not implemented correctly.


Kind regards,
Niklas