linux-kernel - Re: [PATCH v2 1/2] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# properties

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] [day] [month] [year] [list]

Message-ID: <3wxfj3w2ilgmmmvntng4yohvorz3tn54egnyltg3dd3fwk67yq@f5p62em6sg2g>
Date: Sun, 9 Nov 2025 22:18:58 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Abraham I <kishon@...nel.org>, Bjorn Andersson <andersson@...nel.org>, 
	Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, linux-pci@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: qcom: Enforce check for PHY,
 PERST# properties

On Sat, Nov 08, 2025 at 12:59:50PM +0100, Krzysztof Kozlowski wrote:
> On Thu, Nov 06, 2025 at 04:57:16PM +0530, Manivannan Sadhasivam wrote:
> > Currently, the binding supports specifying the required PHY, PERST#
> > properties in two ways:
> > 
> > 1. Controller node (deprecated)
> > 	- phys
> > 	- perst-gpios
> > 
> > 2. Root Port node
> > 	- phys
> > 	- reset-gpios
> > 
> > But there is no check to make sure that the both variants are not mixed.
> > For instance, if the Controller node specifies 'phys', 'reset-gpios',
> 
> Schema already does not allow it, unless I missed which schema defines
> reset-gpios in controller node.
> 

'reset-gpios' is currently a valid property for both controller and Root Port
nodes. Where does the schema restricts it?

> > or if the Root Port node specifies 'phys', 'perst-gpios', then the driver
> > will fail as reported. Hence, enforce the check in the binding to catch
> > these issues.
> 
> I do not see such check.
> 

Don't you think the below required properties not enforce this check for Root
Port and Controller node? This atleast makes sure that if 'phys' is present,
'reset-gpios' would be required for Root Port and 'perst-gpios' is required for
Controller node.

> > 
> > It is also possible that DTs could have 'phys' property in Controller node
> > and 'reset-gpios' properties in the Root Port node. It will also be a
> > problem, but it is not possible to catch these cross-node issues in the
> > binding.
> 
> ... so this commit changes nothing?
> 
> The commit actually does change, but something completely different than
> you write here, so entire commit msg is describing entirely different
> cast. What you achieve here is to require perst-gpios, if controller
> node defined phys. Unfortunately your commit msg does not explain why
> perst-gpios are now required...
> 

The Qcom PCIe controller node never supported 'reset-gpios' for PERST#. It used
the 'perst-gpios' property instead. And I do not wanted to replace it with
'reset-gpios' property since we had decided to move PERST# to Root Port node,
where 'reset-gpios' is already the norm.

> > 
> > Reported-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> > Reported-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> > Closes: https://lore.kernel.org/linux-pci/8f2e0631-6c59-4298-b36e-060708970ced@oss.qualcomm.com
> > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> 
> That's too many tags. Either someone reported you bug or someone
> suggested you to do something, not both (and proposing solution is not
> suggesting a commit since you already knew you need to make the commit
> because of bug...)
> 

I disagree. Both Konrad and Krishna reported the issue in mixing up the
properties and driver ended up failing the probe. Then Dmitry suggested a schema
snippet [1] to catch these kind of mixups during DT validation. I did see it as
a valid suggestion that deserved the tag.

- Mani

[1] https://lore.kernel.org/linux-pci/qref5ooh6pl2sznf7iifrbric7hsap63ffbytkizdyrzt6mtqz@q5r27ho2sbq3/

-- 
மணிவண்ணன் சதாசிவம்