| Message ID | 20240326153219.2915080-18-avromanov@salutedevices.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Support more Amlogic SoC families in crypto driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 28 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Tue, Mar 26, 2024 at 06:32:13PM +0300, Alexey Romanov wrote: > GXL crypto IP uses DMA engine, which doesn't require clk input > and second interrupt line. > > Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node") How does this fix a commit that only modifies a dts? The commit cited here should be a dt-bindings commit. However, your commit message says "require" but you do more than remove these as required, you remove them entirely. What am I missing? Thanks, Conor. > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > .../bindings/crypto/amlogic,gxl-crypto.yaml | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > index 948e11ebe4ee..d3af7b4d5f39 100644 > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > @@ -20,20 +20,11 @@ properties: > interrupts: > items: > - description: Interrupt for flow 0 > - - description: Interrupt for flow 1 > - > - clocks: > - maxItems: 1 > - > - clock-names: > - const: blkmv > > required: > - compatible > - reg > - interrupts > - - clocks > - - clock-names > > additionalProperties: false > > @@ -46,7 +37,5 @@ 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>; > }; > -- > 2.34.1 >
Hi Conor, On Tue, Mar 26, 2024 at 06:09:37PM +0000, Conor Dooley wrote: > On Tue, Mar 26, 2024 at 06:32:13PM +0300, Alexey Romanov wrote: > > GXL crypto IP uses DMA engine, which doesn't require clk input > > and second interrupt line. > > > > Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node") > > How does this fix a commit that only modifies a dts? The commit cited > here should be a dt-bindings commit. Yep, my bad. Will fix it in next series. > > However, your commit message says "require" but you do more than remove > these as required, you remove them entirely. What am I missing? Crypto HW isn't connected to clk / 2nd interrput line, so we must remove them from dt. I will reformulate commit message. > > Thanks, > Conor. > > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > > --- > > .../bindings/crypto/amlogic,gxl-crypto.yaml | 13 +------------ > > 1 file changed, 1 insertion(+), 12 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > index 948e11ebe4ee..d3af7b4d5f39 100644 > > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > > @@ -20,20 +20,11 @@ properties: > > interrupts: > > items: > > - description: Interrupt for flow 0 > > - - description: Interrupt for flow 1 > > - > > - clocks: > > - maxItems: 1 > > - > > - clock-names: > > - const: blkmv > > > > required: > > - compatible > > - reg > > - interrupts > > - - clocks > > - - clock-names > > > > additionalProperties: false > > > > @@ -46,7 +37,5 @@ 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>; > > }; > > -- > > 2.34.1 > >
On Fri, Mar 29, 2024 at 11:14:29AM +0000, Alexey Romanov wrote: > Hi Conor, > > On Tue, Mar 26, 2024 at 06:09:37PM +0000, Conor Dooley wrote: > > On Tue, Mar 26, 2024 at 06:32:13PM +0300, Alexey Romanov wrote: > > > GXL crypto IP uses DMA engine, which doesn't require clk input > > > and second interrupt line. > > > > > > Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node") > > > > How does this fix a commit that only modifies a dts? The commit cited > > here should be a dt-bindings commit. > > Yep, my bad. Will fix it in next series. > > > > > However, your commit message says "require" but you do more than remove > > these as required, you remove them entirely. What am I missing? > > Crypto HW isn't connected to clk / 2nd interrput line, so we must remove > them from dt. I will reformulate commit message. Yeah, please do. Be clear about the hardware not actually having these things. And hopefully Corentin can chime in as to why he thought it did.
diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml index 948e11ebe4ee..d3af7b4d5f39 100644 --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml @@ -20,20 +20,11 @@ properties: interrupts: items: - description: Interrupt for flow 0 - - description: Interrupt for flow 1 - - clocks: - maxItems: 1 - - clock-names: - const: blkmv required: - compatible - reg - interrupts - - clocks - - clock-names additionalProperties: false @@ -46,7 +37,5 @@ 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>; };
GXL crypto IP uses DMA engine, which doesn't require clk input and second interrupt line. Fixes: c4a0457eb858 ("ARM64: dts: amlogic: adds crypto hardware node") Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> --- .../bindings/crypto/amlogic,gxl-crypto.yaml | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)