| Message ID | 20250125-topic-x1p4_dts-v1-2-02659a08b044@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | X1P42100 DT and PCIe PHY bits | expand |
On Sat, Jan 25, 2025 at 04:31:18AM +0100, Konrad Dybcio wrote: > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > (Almost?) all QMP PHYs come with both a "full reset" ("phy") and a > "retain certain registers" one ("phy_nocsr"). > > Drop the maxItems=1 constraint for resets and reset_names as we go > ahead and straighten out the DT usage. After that's done (which > will involve modifying some clock drivers etc.), we may set > *min*Items to 2, bar some possible exceptions. You drop minItems now, so that's a bit confusing. If all devices have two resets, just change in top-level resets the minItems -> 2 now and mention that it does not affect the ABI, because Linux will support missing reset and it describes the hardware more accurately. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > --- > .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 24 ---------------------- > 1 file changed, 24 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > index f1ffc3d5cae44b8a9c96cdcd749a6e54533c94f6..c42143bd139e30d1beabc9099d0dde17128413bf 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > @@ -204,30 +204,6 @@ allOf: > clock-names: > minItems: 7 > > - - if: > - properties: > - compatible: > - contains: > - enum: > - - qcom,sm8550-qmp-gen4x2-pcie-phy > - - qcom,sm8650-qmp-gen4x2-pcie-phy > - - qcom,x1e80100-qmp-gen4x2-pcie-phy > - - qcom,x1e80100-qmp-gen4x4-pcie-phy > - - qcom,x1e80100-qmp-gen4x8-pcie-phy > - - qcom,x1p42100-qmp-gen4x4-pcie-phy You just added this line, so this patch should be #1. Best regards, Krzysztof
On 27.01.2025 9:26 AM, Krzysztof Kozlowski wrote: > On Sat, Jan 25, 2025 at 04:31:18AM +0100, Konrad Dybcio wrote: >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >> >> (Almost?) all QMP PHYs come with both a "full reset" ("phy") and a >> "retain certain registers" one ("phy_nocsr"). >> >> Drop the maxItems=1 constraint for resets and reset_names as we go >> ahead and straighten out the DT usage. After that's done (which >> will involve modifying some clock drivers etc.), we may set >> *min*Items to 2, bar some possible exceptions. > > You drop minItems now, so that's a bit confusing. If all devices have > two resets, just change in top-level resets the minItems -> 2 now and > mention that it does not affect the ABI, because Linux will support > missing reset and it describes the hardware more accurately. This will generate a ton of warnings and resolving them may take an additional cycle, as I'd need to get things merged through clk too, so I thought this is a good transitional solution Konrad
On 01/02/2025 16:56, Konrad Dybcio wrote: > On 27.01.2025 9:26 AM, Krzysztof Kozlowski wrote: >> On Sat, Jan 25, 2025 at 04:31:18AM +0100, Konrad Dybcio wrote: >>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>> >>> (Almost?) all QMP PHYs come with both a "full reset" ("phy") and a >>> "retain certain registers" one ("phy_nocsr"). >>> >>> Drop the maxItems=1 constraint for resets and reset_names as we go >>> ahead and straighten out the DT usage. After that's done (which >>> will involve modifying some clock drivers etc.), we may set >>> *min*Items to 2, bar some possible exceptions. >> >> You drop minItems now, so that's a bit confusing. If all devices have >> two resets, just change in top-level resets the minItems -> 2 now and >> mention that it does not affect the ABI, because Linux will support >> missing reset and it describes the hardware more accurately. > > This will generate a ton of warnings and resolving them may take an > additional cycle, as I'd need to get things merged through clk too, > so I thought this is a good transitional solution I still don't understand why existing devices now get 1 reset, while previously they had minItems:2. Best regards, Krzysztof
On 2.02.2025 3:35 PM, Krzysztof Kozlowski wrote: > On 01/02/2025 16:56, Konrad Dybcio wrote: >> On 27.01.2025 9:26 AM, Krzysztof Kozlowski wrote: >>> On Sat, Jan 25, 2025 at 04:31:18AM +0100, Konrad Dybcio wrote: >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>> >>>> (Almost?) all QMP PHYs come with both a "full reset" ("phy") and a >>>> "retain certain registers" one ("phy_nocsr"). >>>> >>>> Drop the maxItems=1 constraint for resets and reset_names as we go >>>> ahead and straighten out the DT usage. After that's done (which >>>> will involve modifying some clock drivers etc.), we may set >>>> *min*Items to 2, bar some possible exceptions. >>> >>> You drop minItems now, so that's a bit confusing. If all devices have >>> two resets, just change in top-level resets the minItems -> 2 now and >>> mention that it does not affect the ABI, because Linux will support >>> missing reset and it describes the hardware more accurately. >> >> This will generate a ton of warnings and resolving them may take an >> additional cycle, as I'd need to get things merged through clk too, >> so I thought this is a good transitional solution > > I still don't understand why existing devices now get 1 reset, while > previously they had minItems:2. Hm, right.. Would it make sense to just remove the else: branch? Konrad
On 03/02/2025 14:03, Konrad Dybcio wrote: > On 2.02.2025 3:35 PM, Krzysztof Kozlowski wrote: >> On 01/02/2025 16:56, Konrad Dybcio wrote: >>> On 27.01.2025 9:26 AM, Krzysztof Kozlowski wrote: >>>> On Sat, Jan 25, 2025 at 04:31:18AM +0100, Konrad Dybcio wrote: >>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>> >>>>> (Almost?) all QMP PHYs come with both a "full reset" ("phy") and a >>>>> "retain certain registers" one ("phy_nocsr"). >>>>> >>>>> Drop the maxItems=1 constraint for resets and reset_names as we go >>>>> ahead and straighten out the DT usage. After that's done (which >>>>> will involve modifying some clock drivers etc.), we may set >>>>> *min*Items to 2, bar some possible exceptions. >>>> >>>> You drop minItems now, so that's a bit confusing. If all devices have >>>> two resets, just change in top-level resets the minItems -> 2 now and >>>> mention that it does not affect the ABI, because Linux will support >>>> missing reset and it describes the hardware more accurately. >>> >>> This will generate a ton of warnings and resolving them may take an >>> additional cycle, as I'd need to get things merged through clk too, >>> so I thought this is a good transitional solution >> >> I still don't understand why existing devices now get 1 reset, while >> previously they had minItems:2. > > Hm, right.. > > Would it make sense to just remove the else: branch? Yes, I guess that's what you want to achieve here. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml index f1ffc3d5cae44b8a9c96cdcd749a6e54533c94f6..c42143bd139e30d1beabc9099d0dde17128413bf 100644 --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml @@ -204,30 +204,6 @@ allOf: clock-names: minItems: 7 - - if: - properties: - compatible: - contains: - enum: - - qcom,sm8550-qmp-gen4x2-pcie-phy - - qcom,sm8650-qmp-gen4x2-pcie-phy - - qcom,x1e80100-qmp-gen4x2-pcie-phy - - qcom,x1e80100-qmp-gen4x4-pcie-phy - - qcom,x1e80100-qmp-gen4x8-pcie-phy - - qcom,x1p42100-qmp-gen4x4-pcie-phy - then: - properties: - resets: - minItems: 2 - reset-names: - minItems: 2 - else: - properties: - resets: - maxItems: 1 - reset-names: - maxItems: 1 - - if: properties: compatible: