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. 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
