diff mbox series

[v7,1/2] arm64: dts: qcom: sm8650: extend the register range for UFS ICE

Message ID 20241001-wrapped-keys-dts-v7-1-a668519b7ffe@linaro.org (mailing list archive)
State Changes Requested
Headers show
Series arm64: dts: qcom: extend the register range for ICE on sm8[56]50 | expand

Commit Message

Bartosz Golaszewski Oct. 1, 2024, 8:35 a.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
Manager (HWKM) to securely manage storage keys. Enable using this
hardware on sm8650.

This requires us to increase the register range: HWKM is an additional
piece of hardware sitting alongside ICE, and extends the old ICE's
register space.

Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
Co-developed-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Neil Armstrong Oct. 1, 2024, 9:04 a.m. UTC | #1 
On 01/10/2024 10:35, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
> Manager (HWKM) to securely manage storage keys. Enable using this
> hardware on sm8650.
> 
> This requires us to increase the register range: HWKM is an additional
> piece of hardware sitting alongside ICE, and extends the old ICE's
> register space.
> 
> Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Co-developed-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 01ac3769ffa6..5986a33ddd8b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -2595,7 +2595,7 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>   		ice: crypto@1d88000 {
>   			compatible = "qcom,sm8650-inline-crypto-engine",
>   				     "qcom,inline-crypto-engine";
> -			reg = <0 0x01d88000 0 0x8000>;
> +			reg = <0 0x01d88000 0 0x18000>;
>   
>   			clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
>   		};
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Konrad Dybcio Oct. 4, 2024, 9:51 a.m. UTC | #2 
On 1.10.2024 10:35 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
> Manager (HWKM) to securely manage storage keys. Enable using this
> hardware on sm8650.
> 
> This requires us to increase the register range: HWKM is an additional
> piece of hardware sitting alongside ICE, and extends the old ICE's
> register space.
> 
> Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Co-developed-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad
Bjorn Andersson Oct. 4, 2024, 2:06 p.m. UTC | #3 
On Tue, Oct 01, 2024 at 10:35:30AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
> Manager (HWKM) to securely manage storage keys. Enable using this
> hardware on sm8650.
> 
> This requires us to increase the register range: HWKM is an additional
> piece of hardware sitting alongside ICE, and extends the old ICE's
> register space.
> 

This commit message doesn't follow what Neil requested in v5:

https://lore.kernel.org/lkml/109b1e46-f46f-4636-87d5-66266e04ccff@linaro.org/

> Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>

I unfortunately can't find where Neil provided this.

Is this tag referring to this patch having been tested together with the
driver changes, so he's saying that HWKM works fine. Or is he saying
that the old feature set still works after the growth of the register
region (i.e. what he requested in v5)?

Regards,
Bjorn

> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Co-developed-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 01ac3769ffa6..5986a33ddd8b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -2595,7 +2595,7 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  		ice: crypto@1d88000 {
>  			compatible = "qcom,sm8650-inline-crypto-engine",
>  				     "qcom,inline-crypto-engine";
> -			reg = <0 0x01d88000 0 0x8000>;
> +			reg = <0 0x01d88000 0 0x18000>;
>  
>  			clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
>  		};
> 
> -- 
> 2.43.0
> 
>
Bartosz Golaszewski Oct. 4, 2024, 2:15 p.m. UTC | #4 
On Fri, Oct 4, 2024 at 4:06 PM Bjorn Andersson
<quic_bjorande@quicinc.com> wrote:
>
> On Tue, Oct 01, 2024 at 10:35:30AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
> > Manager (HWKM) to securely manage storage keys. Enable using this
> > hardware on sm8650.
> >
> > This requires us to increase the register range: HWKM is an additional
> > piece of hardware sitting alongside ICE, and extends the old ICE's
> > register space.
> >
>
> This commit message doesn't follow what Neil requested in v5:
>
> https://lore.kernel.org/lkml/109b1e46-f46f-4636-87d5-66266e04ccff@linaro.org/
>

Because we have dropped the new property two versions ago as per this
series' cover letter.

> > Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
>
> I unfortunately can't find where Neil provided this.
>
> Is this tag referring to this patch having been tested together with the
> driver changes, so he's saying that HWKM works fine. Or is he saying
> that the old feature set still works after the growth of the register
> region (i.e. what he requested in v5)?
>

I think Neil tested the full functionality of HWKM on sm8650 as per
Gaurav's instructions. I did the same as well.

Bart
Neil Armstrong Oct. 7, 2024, 7:11 a.m. UTC | #5 
On 04/10/2024 16:15, Bartosz Golaszewski wrote:
> On Fri, Oct 4, 2024 at 4:06 PM Bjorn Andersson
> <quic_bjorande@quicinc.com> wrote:
>>
>> On Tue, Oct 01, 2024 at 10:35:30AM +0200, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> The Inline Crypto Engine (ICE) for UFS/EMMC supports the Hardware Key
>>> Manager (HWKM) to securely manage storage keys. Enable using this
>>> hardware on sm8650.
>>>
>>> This requires us to increase the register range: HWKM is an additional
>>> piece of hardware sitting alongside ICE, and extends the old ICE's
>>> register space.
>>>
>>
>> This commit message doesn't follow what Neil requested in v5:
>>
>> https://lore.kernel.org/lkml/109b1e46-f46f-4636-87d5-66266e04ccff@linaro.org/
>>
> 
> Because we have dropped the new property two versions ago as per this
> series' cover letter.

The patch now is fine for me, the comment was applicable to v5

> 
>>> Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
>>> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
>>
>> I unfortunately can't find where Neil provided this.
>>
>> Is this tag referring to this patch having been tested together with the
>> driver changes, so he's saying that HWKM works fine. Or is he saying
>> that the old feature set still works after the growth of the register
>> region (i.e. what he requested in v5)?
>>
> 
> I think Neil tested the full functionality of HWKM on sm8650 as per
> Gaurav's instructions. I did the same as well.

Exact, I can re-test if necessary, but I trust Bartosz here.

Neil

> 
> Bart
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 01ac3769ffa6..5986a33ddd8b 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2595,7 +2595,7 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 		ice: crypto@1d88000 {
 			compatible = "qcom,sm8650-inline-crypto-engine",
 				     "qcom,inline-crypto-engine";
-			reg = <0 0x01d88000 0 0x8000>;
+			reg = <0 0x01d88000 0 0x18000>;
 
 			clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
 		};