| Message ID | 20250505125538.2991314-1-pavitrakumarm@vayavyalabs.com |
|---|---|
| Headers | show |
| Series | Add SPAcc Crypto Driver | expand |
Hi Krzysztof, My comments are embedded below. Warm regards, PK On Mon, May 5, 2025 at 9:22 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 05/05/2025 17:48, Krzysztof Kozlowski wrote: > > On 05/05/2025 14:55, Pavitrakumar M wrote: > >> From: Pavitrakumar Managutte <pavitrakumarm@vayavyalabs.com> > >> > >> Add DT bindings related to the SPAcc driver for Documentation. > >> DWC Synopsys Security Protocol Accelerator(SPAcc) Hardware Crypto > >> Engine is a crypto IP designed by Synopsys. > >> > >> Co-developed-by: Bhoomika Kadabi <bhoomikak@vayavyalabs.com> > >> Signed-off-by: Bhoomika Kadabi <bhoomikak@vayavyalabs.com> > >> Signed-off-by: Pavitrakumar Managutte <pavitrakumarm@vayavyalabs.com> > >> Acked-by: Ruud Derwig <Ruud.Derwig@synopsys.com> > > > > > > I do not see any improvements. It seems you ignored all comments, not > > single one was responded to or addressed. PK: Addressed all the below 1. SoC Bindings: We dont have any SoC bindings since its tested on the Zynq platform (on FPGA). So I have retained just the Synopsys SPAcc device here. Also added a detailed description for the same, which describes how we have tested the SPAcc peripheral on Zynq. This was based on your inputs to describe the existing hardware. 2. snps,vspacc-priority: I have removed this from the device tree and now it will come as a KCONFIG option. Let the user configure the vspacc-priority based on his needs. Its a static configuration for a serup. This is needed as virtual-SPAcc is used in heterogeneous processor environments. So we bind each processor with a virtual SPAcc. 3. snps,vpsacc-id - Descriptions updated as per your inputs. I had mentioned driver usage, its cleaned up. 4. snps,spacc-wdtimer: SPAcc Watchdog is not a traditional watchdog, but just an internal counter which we have renamed to "spacc-internal-counter". Its not a watchdog in the traditional sense, so I have not used the existing watchdog schema and its property. 5. interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>; - Updated with macros as per your inputs. 6. Herbert's comments have been addressed for skcipher, Aead and Hashes as below for the completion callbacks into the Crypto subsystem. Since we use workqueue for our bottom half, which is a process context, we disable/enable the local bh as shown below HASH: local_bh_disable(); ahash_request_complete(cb->req, err); local_bh_enable(); AEAD: local_bh_disable(); aead_request_complete(cb->req, err); local_bh_enable(); SKCIPHER: local_bh_disable(); skcipher_request_complete(cb->req, err); local_bh_enable(); 7. Herbert's comment on "mutex_lock" usage in my bottom half is addressed as well. I am using a workqueue for my bottom half, which is a process context. I have mentioned the same in the email response to Herbert. 8. I have updated the changelog from V1 -> V2 as well, detailing all the above changes. > > > > NAK > > > > <form letter> > > This is a friendly reminder during the review process. > > > > It seems my or other reviewer's previous comments were not fully > > addressed. Maybe the feedback got lost between the quotes, maybe you > > just forgot to apply it. Please go back to the previous discussion and > > either implement all requested changes or keep discussing them. > > > > Hm, actually I see now email you responded to some but ignored several > others, so still a no. PK: I am and I will be addressing every single comment. Looking out for your inputs and feedback. > > Best regards, > Krzysztof
Add the driver for SPAcc(Security Protocol Accelerator), which is a crypto acceleration IP from Synopsys. The SPAcc supports multiple ciphers, hashes and AEAD algorithms with various modes. The driver currently supports below AEAD: - ccm(sm4) - ccm(aes) - gcm(sm4) - gcm(aes) - rfc7539(chacha20,poly1305) cipher: - cbc(sm4) - ecb(sm4) - ctr(sm4) - xts(sm4) - cts(cbc(sm4)) - cbc(aes) - ecb(aes) - xts(aes) - cts(cbc(aes)) - ctr(aes) - chacha20 - ecb(des) - cbc(des) - ecb(des3_ede) - cbc(des3_ede) hash: - cmac(aes) - xcbc(aes) - cmac(sm4) - xcbc(sm4) - hmac(md5) - md5 - hmac(sha1) - sha1 - sha224 - sha256 - sha384 - sha512 - hmac(sha224) - hmac(sha256) - hmac(sha384) - hmac(sha512) - sha3-224 - sha3-256 - sha3-384 - sha3-512 - hmac(sm3) - sm3 - michael_mic Pavitrakumar Managutte (6): dt-bindings: crypto: Document support for SPAcc Add SPAcc Skcipher support Add SPAcc AUTODETECT Support Add SPAcc ahash support Add SPAcc AEAD support Add SPAcc Kconfig and Makefile changelog: v1->v2 changes: - Added local_bh_disable() and local_bh_enable() for the below calls. a. for ciphers skcipher_request_complete() b. for aead aead_request_complete() c. for hash ahash_request_complete() - dt-bindings updates a. removed snps,vspacc-priority and made it into config option b. renamed snps,spacc-wdtimer to snps,spacc-internal-counter c. Added description to all properties - Updated corresponding dt-binding changes to code .../bindings/crypto/snps,dwc-spacc.yaml | 81 + drivers/crypto/Kconfig | 1 + drivers/crypto/Makefile | 1 + drivers/crypto/dwc-spacc/Kconfig | 103 + drivers/crypto/dwc-spacc/Makefile | 16 + drivers/crypto/dwc-spacc/spacc_aead.c | 1297 +++++++++ drivers/crypto/dwc-spacc/spacc_ahash.c | 972 +++++++ drivers/crypto/dwc-spacc/spacc_core.c | 2464 +++++++++++++++++ drivers/crypto/dwc-spacc/spacc_core.h | 829 ++++++ drivers/crypto/dwc-spacc/spacc_device.c | 309 +++ drivers/crypto/dwc-spacc/spacc_device.h | 231 ++ drivers/crypto/dwc-spacc/spacc_hal.c | 374 +++ drivers/crypto/dwc-spacc/spacc_hal.h | 114 + drivers/crypto/dwc-spacc/spacc_interrupt.c | 324 +++ drivers/crypto/dwc-spacc/spacc_manager.c | 610 ++++ drivers/crypto/dwc-spacc/spacc_skcipher.c | 779 ++++++ 16 files changed, 8505 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/snps,dwc-spacc.yaml create mode 100644 drivers/crypto/dwc-spacc/Kconfig create mode 100644 drivers/crypto/dwc-spacc/Makefile create mode 100755 drivers/crypto/dwc-spacc/spacc_aead.c create mode 100644 drivers/crypto/dwc-spacc/spacc_ahash.c create mode 100644 drivers/crypto/dwc-spacc/spacc_core.c create mode 100644 drivers/crypto/dwc-spacc/spacc_core.h create mode 100644 drivers/crypto/dwc-spacc/spacc_device.c create mode 100644 drivers/crypto/dwc-spacc/spacc_device.h create mode 100644 drivers/crypto/dwc-spacc/spacc_hal.c create mode 100644 drivers/crypto/dwc-spacc/spacc_hal.h create mode 100644 drivers/crypto/dwc-spacc/spacc_interrupt.c create mode 100644 drivers/crypto/dwc-spacc/spacc_manager.c create mode 100644 drivers/crypto/dwc-spacc/spacc_skcipher.c base-commit: 2dfc7cd74a5e062a5405560447517e7aab1c7341