diff mbox series

[2/3] dt-bindings: Add ICE device specific parameters

Message ID 1539789476-6098-3-git-send-email-anilc@codeaurora.org
State Changes Requested, archived
Headers show
Series Add Inline Crypto Engine (ICE) driver | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

AnilKumar Chimata Oct. 17, 2018, 3:17 p.m. UTC
Add dt parameters information specific to the Inline
Crypto Engine (ICE) device.

Signed-off-by: AnilKumar Chimata <anilc@codeaurora.org>
---
 .../devicetree/bindings/crypto/msm/ice.txt         | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/msm/ice.txt

Comments

Rob Herring (Arm) Oct. 25, 2018, 6:15 p.m. UTC | #1
On Wed, Oct 17, 2018 at 08:47:55PM +0530, AnilKumar Chimata wrote:
> Add dt parameters information specific to the Inline
> Crypto Engine (ICE) device.
> 
> Signed-off-by: AnilKumar Chimata <anilc@codeaurora.org>
> ---
>  .../devicetree/bindings/crypto/msm/ice.txt         | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/msm/ice.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/msm/ice.txt b/Documentation/devicetree/bindings/crypto/msm/ice.txt
> new file mode 100644
> index 0000000..86eed5e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/msm/ice.txt
> @@ -0,0 +1,34 @@
> +* Inline Crypto Engine (ICE)
> +
> +Required properties:
> +  - compatible			: should be "qcom,ice"

Only 1 version ever? Probably not and this needs an SoC specific 
compatible string. Does this follow any standard or ICE is a QCom thing?

> +  - reg 			: <register mapping>

No need to define standard properties. You need to say how many register 
ranges.

> +
> +Optional properties:
> +  - interrupt-names     	: name describing the interrupts for ICE IRQ

No point to this if there is only 1 IRQ.

> +  - interrupts          	: <interrupt mapping for ICE IRQ>
> +  - qcom,enable-ice-clk 	: should enable clocks for ICE HW

This shouldn't be needed.

> +  - clocks              	: List of phandle and clock specifier pairs
> +  - clock-names         	: List of clock input name strings sorted in the same
> +				  order as the clocks property.

How many?  You need to give the 
> +  - qcom,op-freq-hz     	: max clock speed sorted in the same order as the clocks
> +				  property.

Use the assigned-clocks properties for this.

> +  - qcom,instance-type  	: describe the storage type for which ICE node is defined
> +				  currently, only "ufs" and "sdcc" are supported storage type

What if there is more than one instance of ufs or SD? Do you need to 
know which ICE goes with which controller?

> +  - power-domains		: regulator supply to be used by ICE HW
> +
> +Example:
> +	ufs_ice: ufsice@1d90000 {

crytpo@...

> +		compatible = "qcom,ice";
> +		reg = <0x1d90000 0x8000>;
> +		qcom,enable-ice-clk;
> +		clock-names = "ufs_core_clk", "bus_clk",
> +				"iface_clk", "ice_core_clk";
> +		clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> +			 <&gcc GCC_UFS_MEM_CLKREF_CLK>,
> +			 <&gcc GCC_UFS_PHY_AHB_CLK>,
> +			 <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
> +		qcom,op-freq-hz = <0>, <0>, <0>, <300000000>;
> +		power-domains = <&gcc UFS_PHY_GDSC>;
> +		qcom,instance-type = "ufs";
> +	};
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
AnilKumar Chimata Oct. 29, 2018, 1:30 p.m. UTC | #2
Hi Rob,

Thanks for the comments,

On 2018-10-25 23:45, Rob Herring wrote:
> On Wed, Oct 17, 2018 at 08:47:55PM +0530, AnilKumar Chimata wrote:
>> Add dt parameters information specific to the Inline
>> Crypto Engine (ICE) device.
>> 
>> Signed-off-by: AnilKumar Chimata <anilc@codeaurora.org>
>> ---
>>  .../devicetree/bindings/crypto/msm/ice.txt         | 34 
>> ++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/crypto/msm/ice.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/crypto/msm/ice.txt 
>> b/Documentation/devicetree/bindings/crypto/msm/ice.txt
>> new file mode 100644
>> index 0000000..86eed5e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/msm/ice.txt
>> @@ -0,0 +1,34 @@
>> +* Inline Crypto Engine (ICE)
>> +
>> +Required properties:
>> +  - compatible			: should be "qcom,ice"
> 
> Only 1 version ever? Probably not and this needs an SoC specific
> compatible string. Does this follow any standard or ICE is a QCom 
> thing?

Yes, currently one version and in future we might have to add if there 
are any hardware specific changes and this is Qualcomm specific hardware.

> 
>> +  - reg 			: <register mapping>
> 
> No need to define standard properties. You need to say how many 
> register
> ranges.

Changed.

> 
>> +
>> +Optional properties:
>> +  - interrupt-names     	: name describing the interrupts for ICE IRQ
> 
> No point to this if there is only 1 IRQ.

There are two IRQ lines which hardware can support one for non-secure 
operating system and another for secure operating system.

> 
>> +  - interrupts          	: <interrupt mapping for ICE IRQ>
>> +  - qcom,enable-ice-clk 	: should enable clocks for ICE HW
> 
> This shouldn't be needed.

Changed.

> 
>> +  - clocks              	: List of phandle and clock specifier pairs
>> +  - clock-names         	: List of clock input name strings sorted in 
>> the same
>> +				  order as the clocks property.
> 
> How many?  You need to give the

Currently four clocks are needed, details updated accordingly.

>> +  - qcom,op-freq-hz     	: max clock speed sorted in the same order 
>> as the clocks
>> +				  property.
> 
> Use the assigned-clocks properties for this.

Actually that is not a max clock speed, its any array of operating 
frequencies which ICE can support.

> 
>> +  - qcom,instance-type  	: describe the storage type for which ICE 
>> node is defined
>> +				  currently, only "ufs" and "sdcc" are supported storage type
> 
> What if there is more than one instance of ufs or SD? Do you need to
> know which ICE goes with which controller?

That is right, needs to have multiple device node entries which 
differentiate between storage instances.

> 
>> +  - power-domains		: regulator supply to be used by ICE HW
>> +
>> +Example:
>> +	ufs_ice: ufsice@1d90000 {
> 
> crytpo@...
> 
>> +		compatible = "qcom,ice";
>> +		reg = <0x1d90000 0x8000>;
>> +		qcom,enable-ice-clk;
>> +		clock-names = "ufs_core_clk", "bus_clk",
>> +				"iface_clk", "ice_core_clk";
>> +		clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
>> +			 <&gcc GCC_UFS_MEM_CLKREF_CLK>,
>> +			 <&gcc GCC_UFS_PHY_AHB_CLK>,
>> +			 <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
>> +		qcom,op-freq-hz = <0>, <0>, <0>, <300000000>;
>> +		power-domains = <&gcc UFS_PHY_GDSC>;
>> +		qcom,instance-type = "ufs";
>> +	};
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/crypto/msm/ice.txt b/Documentation/devicetree/bindings/crypto/msm/ice.txt
new file mode 100644
index 0000000..86eed5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/msm/ice.txt
@@ -0,0 +1,34 @@ 
+* Inline Crypto Engine (ICE)
+
+Required properties:
+  - compatible			: should be "qcom,ice"
+  - reg 			: <register mapping>
+
+Optional properties:
+  - interrupt-names     	: name describing the interrupts for ICE IRQ
+  - interrupts          	: <interrupt mapping for ICE IRQ>
+  - qcom,enable-ice-clk 	: should enable clocks for ICE HW
+  - clocks              	: List of phandle and clock specifier pairs
+  - clock-names         	: List of clock input name strings sorted in the same
+				  order as the clocks property.
+  - qcom,op-freq-hz     	: max clock speed sorted in the same order as the clocks
+				  property.
+  - qcom,instance-type  	: describe the storage type for which ICE node is defined
+				  currently, only "ufs" and "sdcc" are supported storage type
+  - power-domains		: regulator supply to be used by ICE HW
+
+Example:
+	ufs_ice: ufsice@1d90000 {
+		compatible = "qcom,ice";
+		reg = <0x1d90000 0x8000>;
+		qcom,enable-ice-clk;
+		clock-names = "ufs_core_clk", "bus_clk",
+				"iface_clk", "ice_core_clk";
+		clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
+			 <&gcc GCC_UFS_MEM_CLKREF_CLK>,
+			 <&gcc GCC_UFS_PHY_AHB_CLK>,
+			 <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
+		qcom,op-freq-hz = <0>, <0>, <0>, <300000000>;
+		power-domains = <&gcc UFS_PHY_GDSC>;
+		qcom,instance-type = "ufs";
+	};