On 11/03/2025 06:01, Varadarajan Narayanan wrote: > 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@xxxxxxxxxx> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>>>> >>>>> Unreviewed. >>>>> >>>>>> Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> >>>>>> --- >>>>>> 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@xxxxxxxxxx/ >>>> >>>> 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. only ipq5332 gets additional item, not ipq9574. Your sentence is not correct. You do not have to add mhi to ipq9574. Neither we, nor schema asked you to do this. > Once I add that, the sdx55 and ipq9574 is the same list but in > different order. > You cannot change the order in existing devices. > 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). Then you must eliminate the impact, not minimize it. > >>> (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. I don't understand what is duplication of something here. > > if (adding mhi to ipq9574 reg-names != duplication) > > /* Keep existing order */ > > * Append "mhi" to ipq9574 ipq9574 does not have mhi, does it? If it has, it should be separate patch with its own explanation of the hardware. Best regards, Krzysztof
