diff mbox

[RESEND,4/4] crypto: rk_crypto - add DT bindings documentation

Message ID 1446193369-4453-5-git-send-email-zain.wang@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zain Wang Oct. 30, 2015, 8:22 a.m. UTC
Add DT bindings documentation for the rk3288 crypto drivers.

Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
---
 .../devicetree/bindings/crypto/rk-crypto.txt       | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/rk-crypto.txt

Comments

Mark Rutland Oct. 30, 2015, 2:03 p.m. UTC | #1
On Fri, Oct 30, 2015 at 04:22:49PM +0800, Zain Wang wrote:
> Add DT bindings documentation for the rk3288 crypto drivers.
> 
> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
> ---
>  .../devicetree/bindings/crypto/rk-crypto.txt       | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/rk-crypto.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/rk-crypto.txt b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
> new file mode 100644
> index 0000000..1e50768
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
> @@ -0,0 +1,31 @@
> +Rockchip Electronics And Security Accelerator
> +
> +Required properties:
> +- compatible: should be "rockchip,crypto"

Choose a more specific name. Rockchip could easily come up with another
crypto accelerator later.

> +- reg: base physical address of the engine and length of memory mapped
> +       region.
> +- interrupts: interrupt number
> +- clocks: clock specifiers
> +- clock-names: "aclk_crypto" used to clock data
> +			   "hclk_crypto" used to clock data
> +			   "srst_crypto" used to clock crypto accelerator
> +			   "apb_pclk"    used to clock dma
> +-status: Enable

No need to mention the status property.

Thanks,
Mark.

> +
> +Examples:
> +
> +	crypto: cypto-controller@ff8a0000 {
> +		compatible = "rockchip,crypto";
> +		reg = <0xff8a0000 0x4000>;
> +		interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cru ACLK_CRYPTO>,
> +			 <&cru HCLK_CRYPTO>,
> +			 <&cru SRST_CRYPTO>,
> +			 <&cru ACLK_DMAC1>;
> +
> +		clock-names = "aclk_crypto",
> +			      "hclk_crypto",
> +			      "srst_crypto",
> +				  "apb_pclk";
> +		status = "okay";
> +	};
> -- 
> 1.9.1
> 
>
Heiko Stübner Oct. 30, 2015, 2:08 p.m. UTC | #2
Hi,

Am Freitag, 30. Oktober 2015, 16:22:49 schrieb Zain Wang:
> Add DT bindings documentation for the rk3288 crypto drivers.
> 
> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
> ---
>  .../devicetree/bindings/crypto/rk-crypto.txt       | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/rk-crypto.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/rk-crypto.txt b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
> new file mode 100644
> index 0000000..1e50768
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
> @@ -0,0 +1,31 @@
> +Rockchip Electronics And Security Accelerator
> +
> +Required properties:
> +- compatible: should be "rockchip,crypto"
> +- reg: base physical address of the engine and length of memory mapped
> +       region.
> +- interrupts: interrupt number
> +- clocks: clock specifiers
> +- clock-names: "aclk_crypto" used to clock data
> +			   "hclk_crypto" used to clock data
> +			   "srst_crypto" used to clock crypto accelerator
> +			   "apb_pclk"    used to clock dma

The TRM area of the crypto IP block does not specifiy any special naming for
its clocks, but as a general rule clock names are in the scope of the ip block
so there is no need to add _crypto to every one of them :-) .

I'd suggest clock-names as "aclk", "hclk", "crypto" for the devicetree, with
crypto being the clock that actually drives the operation.


Secondly, why do you need to drive the clock of the peripheral dma-
controller itself (your apb_pclk)? The documentation in the TRM is
quite sparse, so this might very well be justified, but it looks odd that
you need to control the dmac1-clock when it looks like the crypto block
is doing its own dma and neither system-dma has any crypto-related
channel.

So I'd really like some explanation for this :-)


Thanks
Heiko
Zain Wang Nov. 2, 2015, 9:13 a.m. UTC | #3
Hi Mark

On 2015?10?30? 22:03, Mark Rutland wrote:
> On Fri, Oct 30, 2015 at 04:22:49PM +0800, Zain Wang wrote:
>> Add DT bindings documentation for the rk3288 crypto drivers.
>>
>> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
>> ---
>>  .../devicetree/bindings/crypto/rk-crypto.txt       | 31 ++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/crypto/rk-crypto.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/rk-crypto.txt b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
>> new file mode 100644
>> index 0000000..1e50768
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
>> @@ -0,0 +1,31 @@
>> +Rockchip Electronics And Security Accelerator
>> +
>> +Required properties:
>> +- compatible: should be "rockchip,crypto"
> Choose a more specific name. Rockchip could easily come up with another
> crypto accelerator later.
ok! done.
rk3288_crypto may be better than crypto.
>> +- reg: base physical address of the engine and length of memory mapped
>> +       region.
>> +- interrupts: interrupt number
>> +- clocks: clock specifiers
>> +- clock-names: "aclk_crypto" used to clock data
>> +			   "hclk_crypto" used to clock data
>> +			   "srst_crypto" used to clock crypto accelerator
>> +			   "apb_pclk"    used to clock dma
>> +-status: Enable
> No need to mention the status property.
ok! done!
> Thanks,
> Mark.
>
>> +
>> +Examples:
>> +
>> +	crypto: cypto-controller@ff8a0000 {
>> +		compatible = "rockchip,crypto";
>> +		reg = <0xff8a0000 0x4000>;
>> +		interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
>> +		clocks = <&cru ACLK_CRYPTO>,
>> +			 <&cru HCLK_CRYPTO>,
>> +			 <&cru SRST_CRYPTO>,
>> +			 <&cru ACLK_DMAC1>;
>> +
>> +		clock-names = "aclk_crypto",
>> +			      "hclk_crypto",
>> +			      "srst_crypto",
>> +				  "apb_pclk";
>> +		status = "okay";
>> +	};
>> -- 
>> 1.9.1
>>
>>
>
Thanks
Zain
Rob Herring Nov. 2, 2015, 3:39 p.m. UTC | #4
On Mon, Nov 2, 2015 at 3:13 AM, Zain <zain.wang@rock-chips.com> wrote:
> Hi Mark
>
> On 2015?10?30? 22:03, Mark Rutland wrote:
>> On Fri, Oct 30, 2015 at 04:22:49PM +0800, Zain Wang wrote:
>>> Add DT bindings documentation for the rk3288 crypto drivers.
>>>
>>> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
>>> ---

>>> +Required properties:
>>> +- compatible: should be "rockchip,crypto"
>> Choose a more specific name. Rockchip could easily come up with another
>> crypto accelerator later.
> ok! done.
> rk3288_crypto may be better than crypto.

Yes, but use '-' not '_'.

Rob
Zain Wang Nov. 3, 2015, 12:29 a.m. UTC | #5
Hi Mark,

On 2015?11?02? 23:39, Rob Herring wrote:
> On Mon, Nov 2, 2015 at 3:13 AM, Zain <zain.wang@rock-chips.com> wrote:
>> Hi Mark
>>
>> On 2015?10?30? 22:03, Mark Rutland wrote:
>>> On Fri, Oct 30, 2015 at 04:22:49PM +0800, Zain Wang wrote:
>>>> Add DT bindings documentation for the rk3288 crypto drivers.
>>>>
>>>> Signed-off-by: Zain Wang <zain.wang@rock-chips.com>
>>>> ---
>>>> +Required properties:
>>>> +- compatible: should be "rockchip,crypto"
>>> Choose a more specific name. Rockchip could easily come up with another
>>> crypto accelerator later.
>> ok! done.
>> rk3288_crypto may be better than crypto.
> Yes, but use '-' not '_'.
ok! done!
>
> Rob
>
>
>
Thanks
Zain
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/crypto/rk-crypto.txt b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
new file mode 100644
index 0000000..1e50768
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/rk-crypto.txt
@@ -0,0 +1,31 @@ 
+Rockchip Electronics And Security Accelerator
+
+Required properties:
+- compatible: should be "rockchip,crypto"
+- reg: base physical address of the engine and length of memory mapped
+       region.
+- interrupts: interrupt number
+- clocks: clock specifiers
+- clock-names: "aclk_crypto" used to clock data
+			   "hclk_crypto" used to clock data
+			   "srst_crypto" used to clock crypto accelerator
+			   "apb_pclk"    used to clock dma
+-status: Enable
+
+Examples:
+
+	crypto: cypto-controller@ff8a0000 {
+		compatible = "rockchip,crypto";
+		reg = <0xff8a0000 0x4000>;
+		interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru ACLK_CRYPTO>,
+			 <&cru HCLK_CRYPTO>,
+			 <&cru SRST_CRYPTO>,
+			 <&cru ACLK_DMAC1>;
+
+		clock-names = "aclk_crypto",
+			      "hclk_crypto",
+			      "srst_crypto",
+				  "apb_pclk";
+		status = "okay";
+	};