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

[Zain <zain.wang@rock-chips.com>]

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

Hi Heiko,

On 2015年10月30日 22:08, Heiko Stuebner wrote:
> 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.
>
ok! done!
> 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 :-)
Because dmac1_clk(apb_pclk) is connected to NOC. NOC is the bus
inter-connect. Gating dmac1_clock will block this part of NOC transfer.
And crypto master is using this part of NOC transfer to access DDR/SRAM,
it would be wrong result without dmac1_clk. That's why dmac1_clk should
be refered here.
>
>
> Thanks
> Heiko
>
>
>
Thanks
Zain

[-- Attachment #2: Type: text/html, Size: 3122 bytes --]