Re: [PATCH v5 1/2] dt-bindings: PCI: rockchip,rk3399-pcie: Add PERST# inactive delay properties

From: Krzysztof Kozlowski <krzk@kernel.org>
To: "Enrique Hernández Bello" <ehbello@gmail.com>
Cc: shawn.lin@rock-chips.com, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	heiko@sntech.de, dlemoal@kernel.org, iav@iav.lv,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/2] dt-bindings: PCI: rockchip,rk3399-pcie: Add PERST# inactive delay properties
Date: Thu, 5 Nov 2026 19:24:15 +0100	[thread overview]
Message-ID: <b865a3ef-3a69-4bc0-be25-29d3ddfb3385@kernel.org> (raw)
In-Reply-To: <20261105174042.2307139-1-ehbello@gmail.com>

On 05/11/2026 18:40, Enrique Hernández Bello wrote:
> On Thu, Nov 05, 2026, Krzysztof Kozlowski wrote:
>> Vendor driver does not matter, it is antipattern. Do not mention it here
>> as reason to do such change.
>>
>> No need to repeat the diff.
> 
> Understood, both go in the next version.
> 
>> us or ms?
> 
> ms. The 0 case is badly worded: the driver never holds PERST# for less
> than 100 us, whatever the value, and 0 just ends up at that floor. I will
> describe the floor instead of giving 0 a meaning of its own.
> 
>> What is the nature of this timing? 100 ms is a lot. Are you sure you are
>> not describing RC delay (we have bindings for that already)
> 
> The 100 ms is not something the board needs. It is the driver's current
> wait, T_PVPERL from the CEM spec, applied on every board. Because PERST#
> is already asserted when that wait starts, it is also the width of the
> reset pulse.
> 
> On my board (ROCK Pi 4C) the slot supply is regulator-always-on, so power
> has been stable for seconds before probe and there is nothing left to
> settle. The problem goes the other way: the JMB585 behind it stops

So maybe that's JMB585 issue? Arguments so far are rather against
controller-based property.

> answering configuration reads when it is held in reset that long. A wait
> of 0 to 15 ms enumerated 17 times out of 17, 20 ms once out of 3, and
> 100 ms never in 5. So the board needs a shorter value, not a longer one.
> I do not know why this endpoint behaves this way.
> 
> Which binding did you have in mind for the RC delay? If an existing one
> fits, I would rather use it.

The RC delay binding, gpio-delay.

> 
> I also plan to drop rockchip,s2r-perst-inactive-ms, since it describes
> an OS power state rather than the hardware.
> 
> Thanks,
> Enrique

Best regards,
Krzysztof