Add documentation to describe Starfive crypto
driver bindings.
Signed-off-by: Jia Jie Ho <[email protected]>
Signed-off-by: Huan Feng <[email protected]>
---
.../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
1 file changed, 109 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
diff --git a/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
new file mode 100644
index 000000000000..6b852f774c32
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/starfive-crypto.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive Crypto Controller Device Tree Bindings
+
+maintainers:
+ - Jia Jie Ho <[email protected]>
+ - William Qiu <[email protected]>
+
+properties:
+ compatible:
+ const: starfive,jh7110-crypto
+
+ reg:
+ maxItems: 1
+
+ reg-names:
+ items:
+ - const: secreg
+
+ clocks:
+ items:
+ - description: Hardware reference clock
+ - description: AHB reference clock
+
+ clock-names:
+ items:
+ - const: sec_hclk
+ - const: sec_ahb
+
+ interrupts:
+ items:
+ - description: Interrupt pin for algo completion
+ - description: Interrupt pin for DMA transfer completion
+
+ interrupt-names:
+ items:
+ - const: secirq
+ - const: dmairq
+
+ resets:
+ items:
+ - description: STG domain reset line
+
+ reset-names:
+ items:
+ - const: sec_hre
+
+ enable-side-channel-mitigation:
+ description: Enable side-channel-mitigation feature for AES module.
+ Enabling this feature will affect the speed performance of
+ crypto engine.
+ type: boolean
+
+ enable-dma:
+ description: Enable data transfer using dedicated DMA controller.
+ type: boolean
+
+ dmas:
+ items:
+ - description: TX DMA channel
+ - description: RX DMA channel
+
+ dma-names:
+ items:
+ - const: sec_m
+ - const: sec_p
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/starfive-jh7110.h>
+ #include <dt-bindings/reset/starfive-jh7110.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ crypto: crypto@16000000 {
+ compatible = "starfive,jh7110-crypto";
+ reg = <0x0 0x16000000 0x0 0x4000>;
+ reg-names = "secreg";
+ clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
+ <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
+ interrupts = <28>, <29>;
+ interrupt-names = "secirq", "dmairq";
+ clock-names = "sec_hclk","sec_ahb";
+ resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
+ reset-names = "sec_hre";
+ enable-side-channel-mitigation;
+ enable-dma;
+ dmas = <&sec_dma 1 2>,
+ <&sec_dma 0 2>;
+ dma-names = "sec_m","sec_p";
+ };
+ };
--
2.25.1
On 30/11/2022 06:52, Jia Jie Ho wrote:
> Add documentation to describe Starfive crypto
> driver bindings.
Please wrap commit message according to Linux coding style / submission
process:
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
Subject: drop second, redundant "bindings".
>
> Signed-off-by: Jia Jie Ho <[email protected]>
> Signed-off-by: Huan Feng <[email protected]>
> ---
> .../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
> 1 file changed, 109 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
>
> diff --git a/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
> new file mode 100644
> index 000000000000..6b852f774c32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
Filename based on compatible, so starfive,jh7110-crypto.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/starfive-crypto.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive Crypto Controller Device Tree Bindings
Drop "Device Tree Bindings"
> +
> +maintainers:
> + - Jia Jie Ho <[email protected]>
> + - William Qiu <[email protected]>
> +
> +properties:
> + compatible:
> + const: starfive,jh7110-crypto
> +
> + reg:
> + maxItems: 1
> +
> + reg-names:
> + items:
> + - const: secreg
Why do you need reg-names for one entry?
> +
> + clocks:
> + items:
> + - description: Hardware reference clock
> + - description: AHB reference clock
> +
> + clock-names:
> + items:
> + - const: sec_hclk
> + - const: sec_ahb
sec seems redundant, so just "ahb". The first clock then "hclk" or "ref"?
> +
> + interrupts:
> + items:
> + - description: Interrupt pin for algo completion
> + - description: Interrupt pin for DMA transfer completion
> +
> + interrupt-names:
> + items:
> + - const: secirq
> + - const: dmairq
Drop "irq" from both.
> +
> + resets:
> + items:
> + - description: STG domain reset line
> +
> + reset-names:
> + items:
> + - const: sec_hre
Drop "sec". Why do you need the names for one entry?
> +
> + enable-side-channel-mitigation:
> + description: Enable side-channel-mitigation feature for AES module.
> + Enabling this feature will affect the speed performance of
> + crypto engine.
> + type: boolean
Why exactly this is a hardware (DT) property, not runtime?
> +
> + enable-dma:
> + description: Enable data transfer using dedicated DMA controller.
> + type: boolean
Usually the presence of dmas indicates whether to use or not to use DMA.
Do you expect a case where DMA channels are provided by you don't want
DMA? Explain such case and describe why it is a hardware/system
integration property.
> +
> + dmas:
> + items:
> + - description: TX DMA channel
> + - description: RX DMA channel
> +
> + dma-names:
> + items:
> + - const: sec_m
tx
> + - const: sec_p
rx
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/starfive-jh7110.h>
> + #include <dt-bindings/reset/starfive-jh7110.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
Use 4 spaces for example indentation.
> +
Best regards,
Krzysztof
On Wed, 30 Nov 2022 13:52:13 +0800, Jia Jie Ho wrote:
> Add documentation to describe Starfive crypto
> driver bindings.
>
> Signed-off-by: Jia Jie Ho <[email protected]>
> Signed-off-by: Huan Feng <[email protected]>
> ---
> .../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
> 1 file changed, 109 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/crypto/starfive-crypto.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
21 | #include <dt-bindings/clock/starfive-jh7110.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/crypto/starfive-crypto.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Wednesday, November 30, 2022 9:21 PM
> To: JiaJie Ho <[email protected]>; Herbert Xu
> <[email protected]>; David S . Miller <[email protected]>;
> Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] dt-bindings: crypto: Add bindings for Starfive crypto
> driver
>
> On 30/11/2022 06:52, Jia Jie Ho wrote:
> > Add documentation to describe Starfive crypto driver bindings.
>
> Please wrap commit message according to Linux coding style / submission
> process:
> https://elixir.bootlin.com/linux/v5.18-
> rc4/source/Documentation/process/submitting-patches.rst#L586
>
>
> Subject: drop second, redundant "bindings".
>
>
I'll fix the commit title and message in the next version.
> >
> > Signed-off-by: Jia Jie Ho <[email protected]>
> > Signed-off-by: Huan Feng <[email protected]>
> > ---
> > .../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
> > 1 file changed, 109 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
> > b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
> > new file mode 100644
> > index 000000000000..6b852f774c32
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
>
> Filename based on compatible, so starfive,jh7110-crypto.yaml
>
Will update filename.
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/starfive-crypto.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive Crypto Controller Device Tree Bindings
>
> Drop "Device Tree Bindings"
>
Will update title.
> > +
> > +maintainers:
> > + - Jia Jie Ho <[email protected]>
> > + - William Qiu <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: starfive,jh7110-crypto
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + reg-names:
> > + items:
> > + - const: secreg
>
> Why do you need reg-names for one entry?
>
Will remove need of reg-names and update probe function accordingly.
> > +
> > + clocks:
> > + items:
> > + - description: Hardware reference clock
> > + - description: AHB reference clock
> > +
> > + clock-names:
> > + items:
> > + - const: sec_hclk
> > + - const: sec_ahb
>
> sec seems redundant, so just "ahb". The first clock then "hclk" or "ref"?
>
Will fix in next version.
> > +
> > + interrupts:
> > + items:
> > + - description: Interrupt pin for algo completion
> > + - description: Interrupt pin for DMA transfer completion
> > +
> > + interrupt-names:
> > + items:
> > + - const: secirq
> > + - const: dmairq
>
> Drop "irq" from both.
>
Will fix.
> > +
> > + resets:
> > + items:
> > + - description: STG domain reset line
> > +
> > + reset-names:
> > + items:
> > + - const: sec_hre
>
> Drop "sec". Why do you need the names for one entry?
>
Will remove names.
> > +
> > + enable-side-channel-mitigation:
> > + description: Enable side-channel-mitigation feature for AES module.
> > + Enabling this feature will affect the speed performance of
> > + crypto engine.
> > + type: boolean
>
> Why exactly this is a hardware (DT) property, not runtime?
>
This is a hardware setting provided in StarFive crypto engine only.
The crypto API doesn't control this setting during runtime and leaving this always on will impact speed performance.
So, I added this property to allow user to control this in dtb.
> > +
> > + enable-dma:
> > + description: Enable data transfer using dedicated DMA controller.
> > + type: boolean
>
> Usually the presence of dmas indicates whether to use or not to use DMA.
> Do you expect a case where DMA channels are provided by you don't want
> DMA? Explain such case and describe why it is a hardware/system integration
> property.
>
I'll remove this property as DMA is always enabled.
> > +
> > + dmas:
> > + items:
> > + - description: TX DMA channel
> > + - description: RX DMA channel
> > +
> > + dma-names:
> > + items:
> > + - const: sec_m
>
> tx
>
Will fix.
> > + - const: sec_p
>
> rx
>
Will fix.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - clocks
> > + - clock-names
> > + - resets
> > + - reset-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/starfive-jh7110.h>
> > + #include <dt-bindings/reset/starfive-jh7110.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
>
> Use 4 spaces for example indentation.
I'll fix the indentation.
Thank you for taking time to review and provide helpful comments for this patch.
Best regards,
Jia Jie
On 01/12/2022 10:01, JiaJie Ho wrote:
>>> +
>>> + enable-side-channel-mitigation:
>>> + description: Enable side-channel-mitigation feature for AES module.
>>> + Enabling this feature will affect the speed performance of
>>> + crypto engine.
>>> + type: boolean
>>
>> Why exactly this is a hardware (DT) property, not runtime?
>>
>
> This is a hardware setting provided in StarFive crypto engine only.
> The crypto API doesn't control this setting during runtime and leaving this always on will impact speed performance.
> So, I added this property to allow user to control this in dtb.
Devicetree should not describe policies, so without justification it
does not look like hardware property. Drop.
Best regards,
Krzysztof
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Wednesday, November 30, 2022 9:48 PM
> To: JiaJie Ho <[email protected]>
> Cc: [email protected]; [email protected]; Rob
> Herring <[email protected]>; Herbert Xu
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; David S . Miller <[email protected]>
> Subject: Re: [PATCH 5/6] dt-bindings: crypto: Add bindings for Starfive crypto
> driver
>
>
> On Wed, 30 Nov 2022 13:52:13 +0800, Jia Jie Ho wrote:
> > Add documentation to describe Starfive crypto driver bindings.
> >
> > Signed-off-by: Jia Jie Ho <[email protected]>
> > Signed-off-by: Huan Feng <[email protected]>
> > ---
> > .../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
> > 1 file changed, 109 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/crypto/starfive-
> crypto.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No
> such file or directory
> 21 | #include <dt-bindings/clock/starfive-jh7110.h>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406:
> Documentation/devicetree/bindings/crypto/starfive-crypto.example.dtb]
> Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-
> bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
Hi Rob Herring,
The #include in example have dependencies on the following patches:
https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
I've noted them in the cover letter.
How do I add them in this patch?
Thanks,
Jia Jie
On 06/12/2022 04:48, JiaJie Ho wrote:
>
>
>> -----Original Message-----
>> From: Rob Herring <[email protected]>
>> Sent: Wednesday, November 30, 2022 9:48 PM
>> To: JiaJie Ho <[email protected]>
>> Cc: [email protected]; [email protected]; Rob
>> Herring <[email protected]>; Herbert Xu
>> <[email protected]>; Krzysztof Kozlowski
>> <[email protected]>; [email protected];
>> [email protected]; David S . Miller <[email protected]>
>> Subject: Re: [PATCH 5/6] dt-bindings: crypto: Add bindings for Starfive crypto
>> driver
>>
>>
>> On Wed, 30 Nov 2022 13:52:13 +0800, Jia Jie Ho wrote:
>>> Add documentation to describe Starfive crypto driver bindings.
>>>
>>> Signed-off-by: Jia Jie Ho <[email protected]>
>>> Signed-off-by: Huan Feng <[email protected]>
>>> ---
>>> .../bindings/crypto/starfive-crypto.yaml | 109 ++++++++++++++++++
>>> 1 file changed, 109 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/crypto/starfive-crypto.yaml
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m
>> dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> Documentation/devicetree/bindings/crypto/starfive-
>> crypto.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No
>> such file or directory
>> 21 | #include <dt-bindings/clock/starfive-jh7110.h>
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> compilation terminated.
>> make[1]: *** [scripts/Makefile.lib:406:
>> Documentation/devicetree/bindings/crypto/starfive-crypto.example.dtb]
>> Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> make: *** [Makefile:1492: dt_binding_check] Error 2
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://patchwork.ozlabs.org/project/devicetree-
>> bindings/patch/[email protected]
>>
>> The base for the series is generally the latest rc1. A different dependency
>> should be noted in *this* patch.
>>
>
> Hi Rob Herring,
>
> The #include in example have dependencies on the following patches:
> https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
> https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
> I've noted them in the cover letter.
> How do I add them in this patch?
You cannot. Testing bot does not take dependencies, so it did not have
above clock IDs. This also should point your attention that if crypto
maintainers pick up this patch, they also won't have the clock IDs so
their tree will have such failure as well.
You can wait with your patch till dependency hits mainline or you can
just replace clock IDs with numbers and drop the header (and later you
can correct the example if needed... or leave it as is).
Best regards,
Krzysztof
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Tuesday, December 6, 2022 4:26 PM
> To: JiaJie Ho <[email protected]>; Rob Herring <[email protected]>
> Cc: [email protected]; [email protected]; Rob
> Herring <[email protected]>; Herbert Xu
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; David S . Miller <[email protected]>
> Subject: Re: [PATCH 5/6] dt-bindings: crypto: Add bindings for Starfive crypto
> driver
>
> On 06/12/2022 04:48, JiaJie Ho wrote:
> >
> >
> > The #include in example have dependencies on the following patches:
> > https://patchwork.kernel.org/project/linux-riscv/cover/20221118010627.
> > [email protected]/
> > https://patchwork.kernel.org/project/linux-riscv/cover/20221118011714.
> > [email protected]/
> > I've noted them in the cover letter.
> > How do I add them in this patch?
>
> You cannot. Testing bot does not take dependencies, so it did not have
> above clock IDs. This also should point your attention that if crypto
> maintainers pick up this patch, they also won't have the clock IDs so their tree
> will have such failure as well.
>
> You can wait with your patch till dependency hits mainline or you can just
> replace clock IDs with numbers and drop the header (and later you can
> correct the example if needed... or leave it as is).
>
I'll replace the IDs and drop the header then.
Thanks.
Regards,
Jia Jie
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Thursday, December 1, 2022 5:27 PM
> To: JiaJie Ho <[email protected]>; Herbert Xu
> <[email protected]>; David S . Miller <[email protected]>;
> Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] dt-bindings: crypto: Add bindings for Starfive crypto
> driver
>
> On 01/12/2022 10:01, JiaJie Ho wrote:
>
> >>> +
> >>> + enable-side-channel-mitigation:
> >>> + description: Enable side-channel-mitigation feature for AES module.
> >>> + Enabling this feature will affect the speed performance of
> >>> + crypto engine.
> >>> + type: boolean
> >>
> >> Why exactly this is a hardware (DT) property, not runtime?
> >>
> >
> > This is a hardware setting provided in StarFive crypto engine only.
> > The crypto API doesn't control this setting during runtime and leaving this
> always on will impact speed performance.
> > So, I added this property to allow user to control this in dtb.
>
> Devicetree should not describe policies, so without justification it does not
> look like hardware property. Drop.
>
I'll remove this in v2 and set the value using module param instead.
Thanks.
Best regards,
Jia Jie