| Message ID | 20221119221219.1232541-1-linus.walleij@linaro.org |
|---|---|
| Headers | show |
| Series | crypto: stm32 - reuse for Ux500 | expand |
On Mon, Nov 21, 2022 at 2:35 PM Lionel DEBIEVE <lionel.debieve@foss.st.com> wrote: > + writel_relaxed(readl_relaxed(cryp->regs + CRYP_CR) | CR_KEYRDEN, cryp->regs + CRYP_CR); > > Why are you still using CRYP_CR here, would it be better to use cryp->caps->cr to keep compatibility? I noticed that the first two registers were not moved in the memory map so I just kept CR and SR as hard defines. It could be argued that the code will be containing one less memory reference and addition so it is more efficient, but it's so little so I am fine to change it to an offset in the caps if you prefer it to be uniform. Yours, Linus Walleij
On 19/11/2022 23:12, Linus Walleij wrote: > This adds device tree bindings for the Ux500 CRYP block > as a compatible in the STM32 CRYP bindings. > > The Ux500 CRYP binding has been used for ages in the kernel > device tree for Ux500 but was never documented, so fill in > the gap by making it a sibling of the STM32 CRYP block, > which is what it is. > > The relationship to the existing STM32 CRYP block is pretty > obvious when looking at the register map, and I have written > patches to reuse the STM32 CRYP driver on the Ux500. > > The two properties added are DMA channels and power domain. > Power domains are a generic SoC feature and the STM32 variant > also has DMA channels. > > Cc: devicetree@vger.kernel.org > Cc: Lionel Debieve <lionel.debieve@foss.st.com> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > This was previously sent out as an open question but > nothing happened, now I send it as part of the STM32 > bindings, in a series making the Linux STM32 driver > use the STM32 driver. > --- > .../bindings/crypto/st,stm32-cryp.yaml | 32 +++++++++++++++++++ > 1 file changed, 32 insertions(+) Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. > > diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml b/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > index ed23bf94a8e0..69614ab51f81 100644 > --- a/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > +++ b/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > @@ -6,12 +6,18 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > > title: STMicroelectronics STM32 CRYP bindings > > +description: The STM32 CRYP block is built on the CRYP block found in > + the STn8820 SoC introduced in 2007, and subsequently used in the U8500 > + SoC in 2010. > + > maintainers: > - Lionel Debieve <lionel.debieve@foss.st.com> > > properties: > compatible: > enum: > + - st,stn8820-cryp > + - stericsson,ux500-cryp > - st,stm32f756-cryp > - st,stm32mp1-cryp > > @@ -27,6 +33,19 @@ properties: > resets: > maxItems: 1 > > + dmas: > + items: > + - description: mem2cryp DMA channel > + - description: cryp2mem DMA channel > + > + dma-names: > + items: > + - const: mem2cryp > + - const: cryp2mem > + > + power-domains: > + maxItems: 1 Are these all valid for other variants? > + > required: > - compatible > - reg > @@ -48,4 +67,17 @@ examples: > resets = <&rcc CRYP1_R>; > }; > > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/reset/stericsson,db8500-prcc-reset.h> > + #include <dt-bindings/arm/ux500_pm_domains.h> > + cryp@a03cb000 { Drop the example, it's almost the same and difference in one new property does not warrant a new example. Best regards, Krzysztof
On Wed, Nov 23, 2022 at 5:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > Cc: devicetree@vger.kernel.org > > Cc: Lionel Debieve <lionel.debieve@foss.st.com> > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> (...) > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. The people reported by get_maintainers are maybe not on the CC line of the patch, but if you look at the mail header they are on the Cc: line... because I pass the not immediately relevant people to git-send-email rather than add them in the Cc tags. > > diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml b/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > > index ed23bf94a8e0..69614ab51f81 100644 > > --- a/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > > +++ b/Documentation/devicetree/bindings/crypto/st,stm32-cryp.yaml > > @@ -6,12 +6,18 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > title: STMicroelectronics STM32 CRYP bindings > > > > +description: The STM32 CRYP block is built on the CRYP block found in > > + the STn8820 SoC introduced in 2007, and subsequently used in the U8500 > > + SoC in 2010. > > + > > maintainers: > > - Lionel Debieve <lionel.debieve@foss.st.com> > > > > properties: > > compatible: > > enum: > > + - st,stn8820-cryp > > + - stericsson,ux500-cryp > > - st,stm32f756-cryp > > - st,stm32mp1-cryp > > > > @@ -27,6 +33,19 @@ properties: > > resets: > > maxItems: 1 > > > > + dmas: > > + items: > > + - description: mem2cryp DMA channel > > + - description: cryp2mem DMA channel > > + > > + dma-names: > > + items: > > + - const: mem2cryp > > + - const: cryp2mem > > + > > + power-domains: > > + maxItems: 1 > > Are these all valid for other variants? The commit message of the patch reads: > The two properties added are DMA channels and power domain. > Power domains are a generic SoC feature and the STM32 variant > also has DMA channels. I think of power domains kind of like resets, clocks or supplies, something that is optional. > > + - | > > + #include <dt-bindings/interrupt-controller/irq.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/reset/stericsson,db8500-prcc-reset.h> > > + #include <dt-bindings/arm/ux500_pm_domains.h> > > + cryp@a03cb000 { > > Drop the example, it's almost the same and difference in one new > property does not warrant a new example. OK I drop it. Thanks for reviewing! Yours, Linus Walleij
On Thu, Nov 24, 2022 at 10:22 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > I am referring to the mail header, not to "CC" lines above. You missed > to Cc Devicetree maintainers (maybe more folks, I did not check all > addresses). Aha yeah that by default I just add Cc devicetree@vger.kernel.org for bindings, I guess because of the old ambition of separating device tree work from kernel work, which I think we have now given up on so yeah I should know better :/ Thanks! Linus Walleij