linux-kernel - Re: [PATCH 1/4] PCI: qcom: Reshuffle reset logic in 2_7

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 29 Dec 2023 15:04:23 +0100
From: Johan Hovold <johan@...nel.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: Manivannan Sadhasivam <mani@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Stanimir Varbanov <svarbanov@...sol.com>,
	Andrew Murray <amurray@...goodpenguin.co.uk>,
	Vinod Koul <vkoul@...nel.org>,
	Marijn Suijten <marijn.suijten@...ainline.org>,
	linux-arm-msm@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] PCI: qcom: Reshuffle reset logic in 2_7_0 .init

On Wed, Dec 27, 2023 at 11:17:19PM +0100, Konrad Dybcio wrote:
> At least on SC8280XP, if the PCIe reset is asserted, the corresponding
> AUX_CLK will be stuck at 'off'.

No, this path is exercised on every boot without the aux clock ever
being stuck at off. So something is clearly missing in this description.

> Assert the reset (which may end up being a NOP if it was previously
> asserted) and de-assert it back *before* turning on the clocks to avoid
> such cases.
> 
> In addition to that, in case the clock bulk enable fails, assert the
> RC reset back, as the hardware is in an unknown state at best.

This is arguably a separate change, and not necessarily one that is
correct either, so should at least go in a separate patch if it should
be done at all.

> Fixes: ed8cc3b1fc84 ("PCI: qcom: Add support for SDM845 PCIe controller")

I think you're being way to liberal with your use of Fixes tags. To
claim that this is a bug, you need to make a more convincing case for
why you think so.

Also note Qualcomm's vendor driver is similarly asserting reset after
enabling the clocks.

That driver does not seem to reset the controller on resume, though, in
case that is relevant for your current experiments.

Johan