linux-kernel - Re: [PATCH v6 05/13] PCI: brcmstb: Use bridge reset if available

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 for Android: free password hash cracker in your pocket

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

Message-ID: <3bb5c6db-11d9-4e65-a581-1a7f6945450a@suse.de>
Date: Mon, 26 Aug 2024 13:42:59 +0300
From: Stanimir Varbanov <svarbanov@...e.de>
To: Jim Quinlan <james.quinlan@...adcom.com>,
 Stanimir Varbanov <svarbanov@...e.de>
Cc: linux-pci@...r.kernel.org, Nicolas Saenz Julienne <nsaenz@...nel.org>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
 Cyril Brulebois <kibi@...ian.org>,
 Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Krzysztof Kozlowski <krzk@...nel.org>,
 bcm-kernel-feedback-list@...adcom.com, jim2101024@...il.com,
 Florian Fainelli <florian.fainelli@...adcom.com>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Rob Herring <robh@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>,
 "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
 <linux-rpi-kernel@...ts.infradead.org>,
 "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
 <linux-arm-kernel@...ts.infradead.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 05/13] PCI: brcmstb: Use bridge reset if available

Hi Jim,

<cut>

> 
> Hi Stan,
> 
> Most of the clocks on the STB chips come up active so one does not
> have to turn them on and off to have the device function.  It helps
> power savings to do this although I'm not sure it is significant.
>>
>>>
>>> Perhaps you don't see the dependence on the PCIe clocks if the 2712
>>> does not give the PCIe node a clock property and instead keeps its
>>> clocks on all of the time.  In that case I would think that your
>>> solution would be fine.
>>
>> What you mean by my solution? The one where avoiding assert of
>> bridge_reset at link [1] bellow?
> 
> Yes.
>>
>> If so, I still cannot understand the relation between bridge_reset and
>> rescal as the comment mentions:
>>
>> "Shutting down this bridge on pcie1 means accesses to rescal block will
>> hang the chip if another RC wants to assert/deassert rescal".
> 
> I was just describing my observations; this should not be happening.
> I would say it is a HW bug for the 2712.  I can file a bug against the
> 2712 but that will not help us right now.  From what I was told by HW,
> asserting the PCIe1 bridge reset does not affect the rescal settings,
> but it does freeze access to the rescal registers, and that is game
> over for the other PCIe controllers accessing the rescal registers.

Good findings, thank you.

The problem comes from this snippet from brcm_pcie_probe() :

	ret = pci_host_probe(bridge);
	if (!ret && !brcm_pcie_link_up(pcie))
		ret = -ENODEV;

	if (ret) {
		brcm_pcie_remove(pdev);
		return ret;
	}

Even when pci_host_probe() is successful the .probe will fail if there
are no endpoint devices on this root port bus. This is the case when
probing pcie1 port which is the one with external connector. Cause the
probe is failing we call reset_control_rearm(rescal) from
brcm_pcie_remove(), after that during .probe of pcie2 (the root port
where RP1 south-bridge is attached) reset_control_reset(rescal) will
issue rescal reset thus rescal-reset driver will stuck on read/write
registers.

I think we have to drop this link-up check and allow the probe to finish
successfully. Even that there no PCI devices attached to bus we want the
root port to be visible by lspci tool. This will solve partially the
issue with accessing rescal reset-controller registers after asserting
bridge_reset. The other part of the problem will be solved by remove the
invocation of reset_control_rearm(rescal) from __brcm_pcie_remove().
That way only the first probed root port will issue rescal reset and
every next probe will not try to reset rescal because we do not call
_rearm(rescal).

What do you think?

~Stan