Re: [PATCH v3] PCI: rockchip: Don't lengthen PERST# when power is already valid

[Shawn Lin]

在 2026/10/29 星期四 1:11, Enrique Hernández Bello 写道:
> On Wed, 28 Oct 2026 02:21:14 +0000, Shawn Lin wrote:
>
> > Was it possible to add a DT property for configurable delay? I came
> > across several EPs need quite a longer wait time, so add it in the
> > vendor tree for dwc-based IP[1]. That could be applied here to slove
> > your cases and others', and might avoid regression.
> >
> > [1]
> > https://github.com/rockchip-linux/kernel/blob/develop-6.12/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L865
>
> Thanks -- that is a better shape than mine, and I will do it.
>
> I will mirror the vendor driver: rockchip,perst-inactive-ms for the pulse, and
> rockchip,s2r-perst-inactive-ms for the resume path, since the two paths want
> different values here as well.
>
> One thing I would rather ask than guess: should the default stay at
> PCIE_T_PVPERL_MS, with affected boards setting the property, or should the
> default be the spec minimum, with the property there for endpoints that need
> longer?

 I'd suggest the affected boards to set it via DT.

> What makes me ask is where T_PVPERL is measured from. It runs from power
> stable to PERST# inactive, so on a board whose slot rails are regulator-
> always-on it is already satisfied seconds before the driver touches the
> GPIO, and the 100 ms then only widens the pulse. The one pulse-width floor
> the spec gives is T_PERST, 100 us.
>
> On my board that difference decides whether the JMB585 answers config reads at
> all: 0 of 5 boots with a 100 ms pulse, 5 of 5 with a short one, with the
> power-settling time held constant across both.
>
> Keeping the current default is the safer change and I am happy to do that. I
> would just rather we did it knowingly, instead of carrying a number that was
> only ever right for boards which power the slot at the moment they assert
> PERST#.
>
> Secondary question: do you want the vendor-prefixed names, or would the DT
> binding maintainers prefer something generic, given the timing comes from the
> CEM spec rather than from Rockchip?

I think this kind of settings should keep vendor prefix, documented in
vendor driver's dt-binding, unless other drivers would like to share
this which I'm not sure now.