| Message ID | 20240607141242.2616580-18-avromanov@salutedevices.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support more Amlogic SoC families in crypto driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 27 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Fri, Jun 07, 2024 at 05:12:36PM +0300, Alexey Romanov wrote: > GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW. > Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto > engine is hard weired to a clk81 (CLKID_CLK81). typo. > > Also, GXL crypto IP isn't to seconnd interrput line. So we must 2 more typos. Spell checkers exist. Use them instead of me please. I think you forgot the word 'connected' too. > remove it from dt-bindings. So did this binding not work at all? Are there any users? You need a bit more justification for an ABI breaking change. > > Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings > documentation for amlogic-crypto") This line should not be wrapped. > Drop the blank line. > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > .../bindings/crypto/amlogic,gxl-crypto.yaml | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > index 948e11ebe4ee..aff6f3234dc9 100644 > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > @@ -20,13 +20,15 @@ properties: > interrupts: > items: > - description: Interrupt for flow 0 > - - description: Interrupt for flow 1 > > clocks: > maxItems: 1 > > clock-names: > - const: blkmv > + const: clk81 Clocks are supposed be named local to the block like what function or part of the block they clock. This sounds like something global. With only 1 clock, I'd just drop the name altogether. > + > + power-domains: > + maxItems: 1 > > required: > - compatible > @@ -46,7 +48,7 @@ examples: > crypto: crypto-engine@c883e000 { > compatible = "amlogic,gxl-crypto"; > reg = <0xc883e000 0x36>; > - interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>; > - clocks = <&clkc CLKID_BLKMV>; > - clock-names = "blkmv"; > + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>; > + clocks = <&clkc CLKID_CLK81>; > + clock-names = "clk81"; > }; > -- > 2.34.1 >
Hi Rob, thank you for the review. On Mon, Jun 10, 2024 at 04:28:27PM -0600, Rob Herring wrote: > On Fri, Jun 07, 2024 at 05:12:36PM +0300, Alexey Romanov wrote: > > GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW. > > Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto > > engine is hard weired to a clk81 (CLKID_CLK81). > > typo. > > > > > Also, GXL crypto IP isn't to seconnd interrput line. So we must > > 2 more typos. Spell checkers exist. Use them instead of me please. > > I think you forgot the word 'connected' too. Yep, sorry, I will fix all typos. > > > remove it from dt-bindings. > > > So did this binding not work at all? Yes. > Are there any users? No. > You need a bit > more justification for an ABI breaking change. I will add more clarification information in commit message in V9. > > > > > Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings > > documentation for amlogic-crypto") > > This line should not be wrapped. > > > > Drop the blank line. > > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > > --- > > .../bindings/crypto/amlogic,gxl-crypto.yaml | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > index 948e11ebe4ee..aff6f3234dc9 100644 > > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > @@ -20,13 +20,15 @@ properties: > > interrupts: > > items: > > - description: Interrupt for flow 0 > > - - description: Interrupt for flow 1 > > > > clocks: > > maxItems: 1 > > > > clock-names: > > - const: blkmv > > + const: clk81 > > Clocks are supposed be named local to the block like what function or > part of the block they clock. This sounds like something global. > > With only 1 clock, I'd just drop the name altogether. I think I have to remove clock-names field in V9. > > > + > > + power-domains: > > + maxItems: 1 > > > > required: > > - compatible > > @@ -46,7 +48,7 @@ examples: > > crypto: crypto-engine@c883e000 { > > compatible = "amlogic,gxl-crypto"; > > reg = <0xc883e000 0x36>; > > - interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>; > > - clocks = <&clkc CLKID_BLKMV>; > > - clock-names = "blkmv"; > > + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>; > > + clocks = <&clkc CLKID_CLK81>; > > + clock-names = "clk81"; > > }; > > -- > > 2.34.1 > >
diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml index 948e11ebe4ee..aff6f3234dc9 100644 --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml @@ -20,13 +20,15 @@ properties: interrupts: items: - description: Interrupt for flow 0 - - description: Interrupt for flow 1 clocks: maxItems: 1 clock-names: - const: blkmv + const: clk81 + + power-domains: + maxItems: 1 required: - compatible @@ -46,7 +48,7 @@ examples: crypto: crypto-engine@c883e000 { compatible = "amlogic,gxl-crypto"; reg = <0xc883e000 0x36>; - interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>; - clocks = <&clkc CLKID_BLKMV>; - clock-names = "blkmv"; + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>; + clocks = <&clkc CLKID_CLK81>; + clock-names = "clk81"; };
GXL and newer SoC's uses the DMA engine (not blkmv) for crypto HW. Crypto HW doesn't actually use the blkmv clk. At RTL level, crypto engine is hard weired to a clk81 (CLKID_CLK81). Also, GXL crypto IP isn't to seconnd interrput line. So we must remove it from dt-bindings. Fixes: 7f7d115dfb51 ("dt-bindings: crypto: Add DT bindings documentation for amlogic-crypto") Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> --- .../bindings/crypto/amlogic,gxl-crypto.yaml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)