| 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
| ||
|
Message-ID: <Z8/Dto1fZWvemiY5@hu-varada-blr.qualcomm.com>
Date: Tue, 11 Mar 2025 10:31:42 +0530
From: Varadarajan Narayanan <quic_varada@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC: Krzysztof Kozlowski <krzk@...nel.org>, <bhelgaas@...gle.com>,
<lpieralisi@...nel.org>, <kw@...ux.com>,
<manivannan.sadhasivam@...aro.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <vkoul@...nel.org>,
<kishon@...nel.org>, <andersson@...nel.org>, <konradybcio@...nel.org>,
<p.zabel@...gutronix.de>, <quic_nsekar@...cinc.com>,
<dmitry.baryshkov@...aro.org>, <linux-arm-msm@...r.kernel.org>,
<linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-phy@...ts.infradead.org>
Subject: Re: [PATCH v11 3/7] dt-bindings: PCI: qcom: Use sdx55 reg
description for ipq9574
On Mon, Mar 10, 2025 at 12:37:28PM +0100, Krzysztof Kozlowski wrote:
> On 10/03/2025 08:44, Varadarajan Narayanan wrote:
> > On Thu, Mar 06, 2025 at 01:06:13PM +0100, Krzysztof Kozlowski wrote:
> >> On 06/03/2025 12:52, Krzysztof Kozlowski wrote:
> >>> On 20/02/2025 10:42, Varadarajan Narayanan wrote:
> >>>> All DT entries except "reg" is similar between ipq5332 and ipq9574. ipq9574
> >>>> has 5 registers while ipq5332 has 6. MHI is the additional (i.e. sixth
> >>>> entry). Since this matches with the sdx55's "reg" definition which allows
> >>>> for 5 or 6 registers, combine ipq9574 with sdx55.
> >>>>
> >>>> This change is to prepare ipq9574 to be used as ipq5332's fallback
> >>>> compatible.
> >>>>
> >>>> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> >>>
> >>> Unreviewed.
> >>>
> >>>> Signed-off-by: Varadarajan Narayanan <quic_varada@...cinc.com>
> >>>> ---
> >>>> v8: Add 'Reviewed-by: Krzysztof Kozlowski'
> >>>> ---
> >>>> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> index 7235d6554cfb..4b4927178abc 100644
> >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >>>> @@ -169,7 +169,6 @@ allOf:
> >>>> enum:
> >>>> - qcom,pcie-ipq6018
> >>>> - qcom,pcie-ipq8074-gen3
> >>>> - - qcom,pcie-ipq9574
> >>>
> >>> Why you did not explain that you are going to affect users of DTS?
> >>>
> >>> NAK
> >
> > Sorry for not explicitly calling this out. I thought that would be seen from the
> > following DTS related patches.
> >
> >> I did not connect the dots, but I pointed out that you break users and
> >> your DTS is wrong:
> >> https://lore.kernel.org/all/f7551daa-cce5-47b3-873f-21b9c5026ed2@kernel.org/
> >>
> >> so you should come back with questions to clarify what to do, not keep
> >> pushing this incorrect patchset.
> >>
> >> My bad, I should really have zero trust.
> >
> > It looks like it is not possible to have ipq9574 as fallback (for ipq5332)
> > without making changes to ipq9574 since the "reg" constraint is different
> > between the two. And this in turn would break the ABI w.r.t. ipq9574.
>
> I don't get why this is not possible. You have one list for ipq9574 and
> existing compatible devices, and you add second list for new device.
>
> ... or you just keep existing order. Why you need to keep changing order
> every time you add new device?
Presently, sdx55 and ipq9574 have the following reg/reg-names constraints.
compatible | qcom,pcie-sdx55 | qcom,pcie-ipq9574
----------------+-----------------------+------------------
reg minItems| 5 | 5
maxItems| 6 | 5
----------------+-----------------------+------------------
reg-names | |
minItems| 5 | 5
----------------+-----------------------+------------------
maxItems| | 5 (6 for ipq5332)
----------------+-----------------------+------------------
items | |
| parf | dbi
| dbi | elbi
| elbi | atu
| atu | parf
| config | config
| mhi | (add mhi for ipq5332)
----------------+-----------------------+------------------
To make ipq9574 as fallback for ipq5332, have to add "mhi" to reg-names of
ipq9574. Once I add that, the sdx55 and ipq9574 is the same list but in
different order.
If this would not be considered as duplication of the same constraint, then I
can club ipq5332 with ipq9574.
If this would be considered as duplication, then sdx55 and ipq9574 would have to
use the same reg-names list and sdx55 or ipq9574 reg-names order would change.
> > To overcome this, two approaches seem to be availabe
> >
> > 1. Document that ipq9574 is impacted and rework these patches to
> > minimize the impact as much as possible
>
> What impact? What is the reason to impact ipq9574? What is the actual issue?
By impact, I meant the change in the reg-names order as mentioned above (for
considered as duplication).
> > (or)
> >
> > 2. Handle ipq5332 as a separate compatible (without fallback) and reuse
> > the constraints of sdx55 for "reg" and ipq9574 for the others (like
> > clock etc.). This approach will also have to revert [1], as it
> > assumes ipq9574 as fallback.
> >
> > Please advice which of the above would be appropriate. If there is a better 3rd
> > alternative please let me know, will align with that approach.
>
> Keep existing order. Why every time we see new device, it comes up with
> a different order?
Will be able to do that based on the answer to 'duplication' question and how to
handle that.
if (adding mhi to ipq9574 reg-names != duplication)
/* Keep existing order */
* Append "mhi" to ipq9574
* use ipq9574 reg-names order for ipq5332
else
* combine ipq9574 & sdx55 reg-names
if (use sdx55 reg-names order)
/* patchset v11 is using this approach */
* change ipq9574
* follow the same for ipq5332
else if (use ipq9574 order)
* change sdx55
* follow the same for ipq5332
Please advice.
Thanks
Varada
Powered by blists - more mailing lists