RE: [PATCH v5 13/15] dt-bindings: crypto: ice: document the hwkm property

["Gaurav Kashyap (QUIC)" <quic_gaurkash@quicinc.com>]

On 06/17/2024 11:31 PM PDT, Krzysztof Kozlowski wrote:
> On 18/06/2024 02:35, Gaurav Kashyap (QUIC) wrote:
> > Hello Krzysztof
> >
> > On   06/17/2024 12:17 AM PDT, Krzysztof Kozlowski wrote:
> >> On 17/06/2024 02:51, Gaurav Kashyap wrote:
> >>> +  qcom,ice-use-hwkm:
> >>> +    type: boolean
> >>> +    description:
> >>> +      Use the supported Hardware Key Manager (HWKM) in Qualcomm
> ICE
> >>> +      to support wrapped keys. Having this entry helps scenarios where
> >>> +      the ICE hardware supports HWKM, but the Trustzone firmware does
> >>> +      not have the full capability to use this HWKM and support wrapped
> >>> +      keys. Not having this entry enabled would make ICE function in
> >>> +      non-HWKM mode supporting standard keys.
> >>
> >> No changelog, previous comments and discussion ignored.
> >>
> >> NAK
> >
> > Apologies for not addressing the previous comments.
> > https://lore.kernel.org/all/9892c541ba4e4b5d975faaa4b49c92ba@quicinc.c
> > om/
> >
> > Maybe we can continue our discussion here; " SM8450 and SM8350 QCOM
> > ICE both support HWKM in their ICE hardware.
> > However, wrapped keys can not be enabled on those targets due to
> > certain missing trustzone support. If we solely rely on hardware
> > version to decide if ICE has to use wrapped keys for data encryption,
> > then it becomes untestable on those chipsets.
> 
> That does not make any sense to me. You enable it for SM8550 and SM8650
> not SM8450 and SM8350.
> 
> >
> > So, we want another way to distinguish this scenario, and hence I
> > chose a DT vendor property
> 
> What scenario? Show it in your patches.
> 
> > to explicitly mention if we have to use the supported HWKM.
> > If there is another way, I am open to exploring that as well."
> 
> That property is just entirely redundant. If you claim otherwise, show it
> through patches.
> 
> To be clear, so you will not resend the same ignoring comments: NAK.
> 

Ack, next set of patches will have the property removed.

> Best regards,
> Krzysztof

Regards,
Gaurav