| Message ID | 20230408214041.533749-2-abel.vesa@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add dedicated Qcom ICE driver | expand |
On 08/04/2023 23:40, Abel Vesa wrote: > Starting with SM8550, the ICE will have its own devicetree node > so add the qcom,ice property to reference it. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 08/04/2023 23:40, Abel Vesa wrote: > Starting with SM8550, the ICE will have its own devicetree node > so add the qcom,ice property to reference it. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > > The v6 is here: > https://lore.kernel.org/all/20230407105029.2274111-3-abel.vesa@linaro.org/ > > Changes since v6: > * Dropped the minItems for both the qcom,ice and the reg in the > qcom,ice compatile subschema, like Krzysztof suggested > > Changes since v5: > * dropped the sm8550 specific subschema and replaced it with one that > mutually excludes the qcom,ice vs both the ICE specific reg range > and the ICE clock > > Changes since v4: > * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce > it while making sure none of the other platforms are allowed to use it > > Changes since v3: > * dropped the "and drop core clock" part from subject line > > Changes since v2: > * dropped all changes except the qcom,ice property > > > .../devicetree/bindings/ufs/qcom,ufs.yaml | 24 +++++++++++++++++++ > 1 file changed, 24 insertions(+) > I see dt_binding_check errors after applying this patch. Are you sure this was tested? Best regards, Krzysztof
On 05/05/2023 20:47, Krzysztof Kozlowski wrote: > On 08/04/2023 23:40, Abel Vesa wrote: >> Starting with SM8550, the ICE will have its own devicetree node >> so add the qcom,ice property to reference it. >> >> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >> --- >> >> The v6 is here: >> https://lore.kernel.org/all/20230407105029.2274111-3-abel.vesa@linaro.org/ >> >> Changes since v6: >> * Dropped the minItems for both the qcom,ice and the reg in the >> qcom,ice compatile subschema, like Krzysztof suggested >> >> Changes since v5: >> * dropped the sm8550 specific subschema and replaced it with one that >> mutually excludes the qcom,ice vs both the ICE specific reg range >> and the ICE clock >> >> Changes since v4: >> * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce >> it while making sure none of the other platforms are allowed to use it >> >> Changes since v3: >> * dropped the "and drop core clock" part from subject line >> >> Changes since v2: >> * dropped all changes except the qcom,ice property >> >> >> .../devicetree/bindings/ufs/qcom,ufs.yaml | 24 +++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> > > I see dt_binding_check errors after applying this patch. Are you sure > this was tested? False alarm, it was other patch in my tree. This one is good. Best regards, Krzysztof
On 08/04/2023 23:40, Abel Vesa wrote: > Starting with SM8550, the ICE will have its own devicetree node > so add the qcom,ice property to reference it. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > > The v6 is here: > https://lore.kernel.org/all/20230407105029.2274111-3-abel.vesa@linaro.org/ > > Changes since v6: > * Dropped the minItems for both the qcom,ice and the reg in the > qcom,ice compatile subschema, like Krzysztof suggested > > Changes since v5: > * dropped the sm8550 specific subschema and replaced it with one that > mutually excludes the qcom,ice vs both the ICE specific reg range > and the ICE clock > > Changes since v4: > * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce > it while making sure none of the other platforms are allowed to use it > > Changes since v3: > * dropped the "and drop core clock" part from subject line > > Changes since v2: > * dropped all changes except the qcom,ice property > > > .../devicetree/bindings/ufs/qcom,ufs.yaml | 24 +++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml > index c5a06c048389..10d426ba1959 100644 > --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml > +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml > @@ -70,6 +70,10 @@ properties: > power-domains: > maxItems: 1 > > + qcom,ice: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle to the Inline Crypto Engine node > + > reg: > minItems: 1 > maxItems: 2 > @@ -187,6 +191,26 @@ allOf: > > # TODO: define clock bindings for qcom,msm8994-ufshc > > + - if: > + properties: > + qcom,ice: Un-reviewed. This is broken and was never tested. After applying this patch, I can see many new warnings in all DTBs (so it is easy to spot that it was not actually tested). Your probably meant here: if: required: Best regards, Krzysztof
Abel, > Un-reviewed. This is broken and was never tested. After applying this > patch, I can see many new warnings in all DTBs (so it is easy to spot > that it was not actually tested). > > Your probably meant here: > if: > required: Please provide a fix for this. I don't want to rebase this late in the cycle. Thanks!
On 22/06/2023 03:19, Martin K. Petersen wrote: > > Abel, > >> Un-reviewed. This is broken and was never tested. After applying this >> patch, I can see many new warnings in all DTBs (so it is easy to spot >> that it was not actually tested). >> >> Your probably meant here: >> if: >> required: > > Please provide a fix for this. I don't want to rebase this late in the > cycle. AFAIK, this was not applied. At least as of next 20210621 and I commented on this few days ago. Anything changed here? Best regards, Krzysztof
On 23-06-22 08:07:51, Krzysztof Kozlowski wrote: > On 22/06/2023 03:19, Martin K. Petersen wrote: > > > > Abel, > > > >> Un-reviewed. This is broken and was never tested. After applying this > >> patch, I can see many new warnings in all DTBs (so it is easy to spot > >> that it was not actually tested). > >> > >> Your probably meant here: > >> if: > >> required: > > > > Please provide a fix for this. I don't want to rebase this late in the > > cycle. > > AFAIK, this was not applied. At least as of next 20210621 and I > commented on this few days ago. Anything changed here? Check this one: https://lore.kernel.org/all/yq1a5x1wl4g.fsf@ca-mkp.ca.oracle.com/ I'll send a fix today. > > Best regards, > Krzysztof >
On 22/06/2023 09:02, Abel Vesa wrote: > On 23-06-22 08:07:51, Krzysztof Kozlowski wrote: >> On 22/06/2023 03:19, Martin K. Petersen wrote: >>> >>> Abel, >>> >>>> Un-reviewed. This is broken and was never tested. After applying this >>>> patch, I can see many new warnings in all DTBs (so it is easy to spot >>>> that it was not actually tested). >>>> >>>> Your probably meant here: >>>> if: >>>> required: >>> >>> Please provide a fix for this. I don't want to rebase this late in the >>> cycle. >> >> AFAIK, this was not applied. At least as of next 20210621 and I >> commented on this few days ago. Anything changed here? > > Check this one: > https://lore.kernel.org/all/yq1a5x1wl4g.fsf@ca-mkp.ca.oracle.com/ > So staging tree is not in next? If it cannot be rebased "that late in the cycle", this means it should be in the next. :/ Best regards, Krzysztof
Hi Krzysztof! > AFAIK, this was not applied. At least as of next 20210621 and I > commented on this few days ago. Anything changed here? It's definitely there in 20230621: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml?h=next-20230621 I merged the series on the 16th prior to you withdrawing your Reviewed-by: tag. But let's just get the bindings fixed.
diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml index c5a06c048389..10d426ba1959 100644 --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml @@ -70,6 +70,10 @@ properties: power-domains: maxItems: 1 + qcom,ice: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the Inline Crypto Engine node + reg: minItems: 1 maxItems: 2 @@ -187,6 +191,26 @@ allOf: # TODO: define clock bindings for qcom,msm8994-ufshc + - if: + properties: + qcom,ice: + maxItems: 1 + then: + properties: + reg: + maxItems: 1 + clocks: + minItems: 8 + maxItems: 8 + else: + properties: + reg: + minItems: 2 + maxItems: 2 + clocks: + minItems: 9 + maxItems: 11 + unevaluatedProperties: false examples:
Starting with SM8550, the ICE will have its own devicetree node so add the qcom,ice property to reference it. Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- The v6 is here: https://lore.kernel.org/all/20230407105029.2274111-3-abel.vesa@linaro.org/ Changes since v6: * Dropped the minItems for both the qcom,ice and the reg in the qcom,ice compatile subschema, like Krzysztof suggested Changes since v5: * dropped the sm8550 specific subschema and replaced it with one that mutually excludes the qcom,ice vs both the ICE specific reg range and the ICE clock Changes since v4: * Added check for sm8550 compatible w.r.t. qcom,ice in order to enforce it while making sure none of the other platforms are allowed to use it Changes since v3: * dropped the "and drop core clock" part from subject line Changes since v2: * dropped all changes except the qcom,ice property .../devicetree/bindings/ufs/qcom,ufs.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+)