[PATCH] crypto: doc - clarify return values for async hash methods
>From af1b10e1e6aaf67f8dc45ed78de89b0469794a98 Mon Sep 17 00:00:00 2001
From: Kamil Konieczny
Date: Thu, 17 Aug 2017 12:11:36 +0200
Subject: [PATCH] crypto: doc - clarify return values for async hash methods
* fix documentation of return values for crypto_ahash_init(),
crypto_ahash_finup(), crypto_ahash_final(),
crypto_ahash_digest() and crypto_ahash_update()
Also while at it:
* add notes for device driver developers in struct ahash_alg
description
* fix description of @final method in struct ahash_alg
* fix typo in crypto_ahash_finup() description
Signed-off-by: Kamil Konieczny
---
include/crypto/hash.h | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index b5727bcd2336..0ed31fd80242 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -75,6 +75,7 @@ struct ahash_request {
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
* transformation. No data processing happens at this point.
+ * Note: mandatory.
* @update: Push a chunk of data into the driver for transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -84,16 +85,20 @@ struct ahash_request {
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
*[SHASH] or asynchronously [AHASH] at this point.
+ *Note: mandatory.
* @final: Retrieve result from the driver. This function finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
- *point.
+ *point unless hardware requires it to finish the transformation
+ *(then the data buffered by the device driver is processed).
+ *Note: mandatory.
* @finup: Combination of @update and @final. This function is effectively a
*combination of @update and @final calls issued in sequence. As some
*hardware cannot do @update and @final separately, this callback was
*added to allow such hardware to be used at least by IPsec. Data
*processing can happen synchronously [SHASH] or asynchronously [AHASH]
*at this point.
+ *Note: optional.
* @digest: Combination of @init and @update and @final. This function
* effectively behaves as the entire chain of operations, @init,
* @update and @final issued in sequence. Just like @finup, this was
@@ -416,11 +421,10 @@ static inline bool crypto_ahash_has_setkey(struct
crypto_ahash *tfm)
* needed to perform the cipher operation
*
* This function is a "short-hand" for the function calls of
- * crypto_ahash_update and crypto_shash_final. The parameters have the same
+ * crypto_ahash_update and crypto_ahash_final. The parameters have the same
* meaning as discussed for those separate functions.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return: see crypto_ahash_final()
*/
int crypto_ahash_finup(struct ahash_request *req);
@@ -433,8 +437,11 @@ int crypto_ahash_finup(struct ahash_request *req);
* based on all data added to the cipher handle. The message digest is placed
* into the output buffer registered with the ahash_request handle.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return:
+ * 0 if the message digest was successfully calculated;
+ * -EINPROGRESSif data is feeded into hardware (DMA) or queued for
later;
+ * -EBUSY if queue is full and request should be resubmitted later;
+ * other < 0 if an error occurred
*/
int crypto_ahash_final(struct ahash_request *req);
@@ -447,8 +454,7 @@ int crypto_ahash_final(struct ahash_request *req);
* crypto_ahash_update and crypto_ahash_final. The parameters have the same
* meaning as discussed for those separate three functions.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return: see crypto_ahash_final()
*/
int crypto_ahash_digest(struct ahash_request *req);
@@ -493,8 +499,7 @@ static inline int crypto_ahash_import(struct ahash_request
*req, const void *in)
* handle. Any potentially existing state created by previous operations is
* discarded.
*
- * Return: 0 if the message digest initialization was successful; < 0 if an
- *error occurred
+ * Return: see crypto_ahash_final()
*/
static inline int crypto_ahash_init(struct ahash_request *req)
{
@@ -510,8 +515,7 @@ static inline int crypto_ahash_init(struct ahash_request
*req)
* is pointed to by the scatter/g
Re: [PATCH v5] crypto: s5p-sss: Add HASH support for Exynos
Hello Vladimir,
thank you for review. I will apply most of your suggestions,
in a few places it is not possible, see comments below.
Your review helped me to improve two functions.
On 12.10.2017 13:45, Vladimir Zapolskiy wrote:
> Hello Kamil,
>
> thank you for the change, please find below a number of minor
> review comments.
>
> On 10/09/2017 02:12 PM, Kamil Konieczny wrote:
>> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW.
>> It uses the crypto framework asynchronous hash api.
>> It is based on omap-sham.c driver.
>> S5P has some HW differencies and is not implemented.
>>
>
> [snip]
>
>>
>> /* Feed control registers */
>> #define SSS_REG_FCINTSTAT 0x
>> +#define SSS_FCINTSTAT_HPARTINT BIT(7)
>> +#define SSS_FCINTSTAT_HDONEINT BIT(5)
>
> ^
> Please use the same style of whitespaces as it is found around.
>
> Generally I do agree that the tabs are better here, please feel free
> to send a preceding change, which changes the spacing to tabs, otherwise
> use space symbols in this change.
ok, I will break this patch into two, first one change spaces into tabs.
>> #define SSS_FCINTSTAT_BRDMAINT BIT(3)
>> #define SSS_FCINTSTAT_BTDMAINT BIT(2)
>> #define SSS_FCINTSTAT_HRDMAINT BIT(1)
>> #define SSS_FCINTSTAT_PKDMAINT BIT(0)
>>
>> #define SSS_REG_FCINTENSET 0x0004
>> +#define SSS_FCINTENSET_HPARTINTENSETBIT(7)
>> +#define SSS_FCINTENSET_HDONEINTENSETBIT(5)
>
> Same as above.
ok with this one and following
>> [skip]
> [snip]
>
>> struct s5p_aes_reqctx {
>> @@ -195,6 +284,19 @@ struct s5p_aes_ctx {
>> * protects against concurrent access to these fields.
>> * @lock: Lock for protecting both access to device hardware registers
>> * and fields related to current request (including the busy
>> field).
>> + * @res:Resources for hash.
>> + * @io_hash_base: Per-variant offset for HASH block IO memory.
>> + * @hash_lock: Lock for protecting hash_req, hash_queue and hash_flags
>> + * variable.
>> + * @hash_tasklet: New HASH request scheduling job.
>> + * @xmit_buf: Buffer for current HASH request transfer into SSS block.
>> + * @hash_flags: Flags for current HASH op.
>> + * @hash_queue: Async hash queue.
>> + * @hash_req: Current request sending to SSS HASH block.
>> + * @hash_sg_iter: Scatterlist transferred through DMA into SSS HASH block.
>> + * @hash_sg_cnt: Counter for hash_sg_iter.
>> + *
>> + * @use_hash: true if HASH algs enabled
>
> You may want to reorder the lines to get the same order as in the struct.
ok
>> */
>> struct s5p_aes_dev {
>> struct device *dev;
>> @@ -215,16 +317,88 @@ struct s5p_aes_dev {
>> struct crypto_queue queue;
>> boolbusy;
>> spinlock_t lock;
>> +
>> +struct resource *res;
>> +void __iomem*io_hash_base;
>> +
>> +spinlock_t hash_lock; /* protect hash_ vars */
>> +unsigned long hash_flags;
>> +struct crypto_queue hash_queue;
>> +struct tasklet_struct hash_tasklet;
>> +
>> +u8 xmit_buf[BUFLEN];
>> +struct ahash_request*hash_req;
>> +struct scatterlist *hash_sg_iter;
>> +int hash_sg_cnt;
>
> Please change the type to 'unsigned int'.
>
>> +
>> +booluse_hash;
>> };
>>
>> -static struct s5p_aes_dev *s5p_dev;
>> +enum hash_op {
>> +HASH_OP_UPDATE = 1,
>> +HASH_OP_FINAL = 2
>> +};
>
> If this type is not going to be extended, then I'd rather suggest to
> use a boolean correspondent field in the struct s5p_hash_reqctx instead
> of a new introduced type.
>
I will change this into 'bool op_final'.
>> +
>> +/**
>> + * struct s5p_hash_reqctx - HASH request context
>> + * @dev:Associated device
>> + * @op: Current request operation (OP_UPDATE or OP_FINAL)
>
> See a comment above.
>
>> + * @digcnt: Number of bytes processed by HW (without buffer[] ones)
>> + * @digest: Digest message or IV for partial result
>> + * @bufcnt: Number of bytes holded in buffer[]
>> + * @nregs: Num
Re: [PATCH v5] crypto: s5p-sss: Add HASH support for Exynos
Hello Vladimir,
>>[snip]
>> -static struct s5p_aes_dev *s5p_dev;
>> +enum hash_op {
>> +HASH_OP_UPDATE = 1,
>> +HASH_OP_FINAL = 2
>> +};
>
> If this type is not going to be extended, then I'd rather suggest to
> use a boolean correspondent field in the struct s5p_hash_reqctx instead
> of a new introduced type. [...]
In prevoious mail I wrote 'op_final', but this will actually be 'op_update'.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
[PATCH 1/2] change spaces into tabs in defines
Change spaces into tabs in defines. Signed-off-by: Kamil Konieczny --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) +#define SSS_HASHIN_CIPHER_OUTPUT
[PATCH 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch cleans up spaces in defines, second adds HASH support for Exynos. Changes: version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): change spaces into tabs in defines Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1596 +++--- 2 files changed, 1505 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
[PATCH 2/2] Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1406 +- 2 files changed, 1410 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..b5a7e49f9285 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +88,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +163,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_ENBIT(5) +#define SSS_HASH_INI
Re: [PATCH 0/2] crypto: s5p-sss: Add HASH support for Exynos
Hi all, sorry for error in subject line, it should be [PATCH v6 0/2] and so on, I can resend this if needed ? On 16.10.2017 19:43, Kamil Konieczny wrote: > First patch cleans up spaces in defines, second adds HASH support for Exynos. > Changes: -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH 2/2] Add HASH support for Exynos
On 17.10.2017 12:07, Krzysztof Kozlowski wrote: > On Mon, Oct 16, 2017 at 7:43 PM, Kamil Konieczny > wrote: >> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >>[...] >> +#if IS_ENABLED(CONFIG_CRYPTO_DEV_EXYNOS_HASH) > > I missed that in v5 because you wrote it is fixed... but it was not. > You still have #ifdef here. Please fix it: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n993 Thank you for link, I didn't get it then, now it is clear, I will rewrite and send v7 of patch. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH v7 1/2] crypto: s5p-sss: change spaces into tabs in defines
change spaces into tabs in defines Signed-off-by: Kamil Konieczny --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) +#define SSS_HASHIN_CIPHER_OUTPUT
[PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1407 +- 2 files changed, 1411 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..c9feccf21efe 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +88,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +163,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_ENBIT(5) +#define SSS_HASH_INI
[PATCH v7 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch cleans up spaces in defines, second adds HASH support for Exynos. Changes: version 7: - fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): change spaces into tabs in defines Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1597 +++--- 2 files changed, 1506 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
Re: [PATCH v7 1/2] crypto: s5p-sss: change spaces into tabs in defines
Hi Vladimir, Thank you for review. On 22.10.2017 12:18, Vladimir Zapolskiy wrote: > Hi Kamil, > > On 10/17/2017 02:28 PM, Kamil Konieczny wrote: >> change spaces into tabs in defines > > Here a grammatically correct sentence in English is welcome. What about: "Change spaces to tabs" ? >> Signed-off-by: Kamil Konieczny >> --- > > Please feel free to add a tag > > Acked-by: Vladimir Zapolskiy -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos
Hi Vladimir,
Thank you for review, I will apply almost all of your remarks,
see answers below.
On 22.10.2017 12:18, Vladimir Zapolskiy wrote:
> Hi Kamil,
>
> thank you for updates, I have just a few more comments.
>
> On 10/17/2017 02:28 PM, Kamil Konieczny wrote:
>> [...]
>> - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH
>> as they are nedded for fallback.
>
> Typo, s/nedded/needed/
ok, Thank you for spotting this.
> [snip]
>
>>
>> #include
>> #include
>> +#include
>
> I can not find which interfaces from linux/delay.h are needed.
> I suppose it should not be added.
>
> [snip]
Yes, you are right, I will delete this 'include delay.h'
>> -static struct s5p_aes_dev *s5p_dev;
>> +/**
>> + * struct s5p_hash_reqctx - HASH request context
>> + * @dev:Associated device
>
> dev or dd?
dd
>> + * @op_update: Current request operation (OP_UPDATE or OP_FINAL)
>> + * @digcnt: Number of bytes processed by HW (without buffer[] ones)
>> + * @digest: Digest message or IV for partial result
>> + * @nregs: Number of HW registers for digest or IV read/write
>> + * @engine: Bits for selecting type of HASH in SSS block
>> + * @sg: sg for DMA transfer
>> + * @sg_len: Length of sg for DMA transfer
>> + * @sgl[]: sg for joining buffer and req->src scatterlist
>> + * @skip: Skip offset in req->src for current op
>> + * @total: Total number of bytes for current request
>> + * @finup: Keep state for finup or final.
>> + * @error: Keep track of error.
>> + * @bufcnt: Number of bytes holded in buffer[]
>> + * @buffer[]: For byte(s) from end of req->src in UPDATE op
>> + */
>> +struct s5p_hash_reqctx {
>> +struct s5p_aes_dev *dd;
>> +boolop_update;
>> +
>
> [snip]
>
>> +
>> +/**
>> + * s5p_hash_shash_digest() - calculate shash digest
>> + * @tfm:crypto transformation
>> + * @flags: tfm flags
>> + * @data: input data
>> + * @len:length of data
>> + * @out:output buffer
>> + */
>> +static int s5p_hash_shash_digest(struct crypto_shash *tfm, u32 flags,
>> + const u8 *data, unsigned int len, u8 *out)
>> +{
>> +SHASH_DESC_ON_STACK(shash, tfm);
>
> Just for information, this line produces a spatch warning:
>
> drivers/crypto/s5p-sss.c:1534:9: warning: Variable length array is used.
>
> I think it can be ignored.
I also think it can be ignored, it uses 104 bytes on stack in case of SHA256 ,
sizeof struct sha256_state for SW generic implementation, found in
include/crypto/sha.h
>> +
>> +shash->tfm = tfm;
>> +shash->flags = flags & ~CRYPTO_TFM_REQ_MAY_SLEEP;
>> +
>> +return crypto_shash_digest(shash, data, len, out);
>> +}
>> +
>
> [snip]
>
>> +/**
>> + * s5p_hash_final() - close up hash and calculate digest
>> + * @req:AHASH request
>> + *
>> + * Note: in final req->src do not have any data, and req->nbytes can be
>> + * non-zero.
>> + *
>> + * If there were no input data processed yet and the buffered hash data is
>> + * less than BUFLEN (64) then calculate the final hash immediately by using
>> + * SW algorithm fallback.
>> + *
>> + * Otherwise enqueues the current AHASH request with OP_FINAL operation op
>> + * and finalize hash message in HW. Note that if digcnt!=0 then there were
>> + * previous update op, so there are always some buffered bytes in
>> ctx->buffer,
>> + * which means that ctx->bufcnt!=0
>> + *
>> + * Returns:
>> + * 0 if the request has been processed immediately,
>> + * -EINPROGRESS if the operation has been queued for later execution or is
>> set
>> + * to processing by HW,
>> + * -EBUSY if queue is full and request should be resubmitted later, other
>> + * negative values on error.
>
> Do you want to add -EINVAL into the list also?
Here I made bad formatting, it should read:
* -EBUSY if queue is full and request should be resubmitted later,
* other negative values on error.
I do not want to specify explicitly all error negative codes here, as it also
uses
s5p_hash_enqueue and these return codes are referenced from other places. I
intended
to listing success values, 0, -EINPROGRESS, then explaining -EBUSY, then adding
all
other negative as error. The other values can be -EINVAL, -ENOMEM or maybe
other, when
this module gets extended to HMAC implementation.
>> + */
>> +static int s5p_hash_final(struct ahash_request *req)
>> +{
>>
[PATCH v8 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch change spaces to tabs, second adds HASH support for Exynos. Changes: version 8: - fixes suggested by Vladimir Zapolskiy: drop first condition check in s5p_hash_import, delete unused include delay.h, fix typo in commit message, fix descriptions of struct s5p_hash_reqctx and function s5p_hash_final() version 7: - fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): crypto: s5p-sss: Change spaces to tabs crypto: s5p-sss: Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1596 +++--- 2 files changed, 1505 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
[PATCH 2/2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are needed for fallback. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Kamil Konieczny --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1406 +- 2 files changed, 1410 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..142c6020cec7 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,14 +1,16 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include @@ -30,28 +32,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +87,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +162,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_ENBIT(5) +#define SSS_HASH_INIT_BIT BIT(4) +#define S
[PATCH 1/2] crypto: s5p-sss: Change spaces to tabs
Change #define lines to use tabs consistently. Acked-by: Vladimir Zapolskiy Reviewed-by: Krzysztof Kozlowski Signed-off-by: Kamil Konieczny --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_
Re: [PATCH 2/2] crypto: s5p-sss: Add HASH support for Exynos
Hi Vladimir, On 25.10.2017 17:09, Vladimir Zapolskiy wrote: > > thank you for updates, everything looks good from my point of view. > Thank you for your work, I am still newbie, I forgot 'v8' and to: linux-crypto :( So I will resend this patch series with improved title and your Ack added. > On 10/25/2017 05:57 PM, Kamil Konieczny wrote: >> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >> It uses the crypto framework asynchronous hash api. >> It is based on omap-sham.c driver. >> S5P has some HW differencies and is not implemented. >>[...] >> Reviewed-by: Krzysztof Kozlowski >> Signed-off-by: Kamil Konieczny >> --- > > I won't linger this time :) > > Please feel free to add > > Acked-by: Vladimir Zapolskiy > -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH v8 2/2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are needed for fallback. Acked-by: Vladimir Zapolskiy Reviewed-by: Krzysztof Kozlowski Signed-off-by: Kamil Konieczny --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1406 +- 2 files changed, 1410 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..142c6020cec7 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,14 +1,16 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include @@ -30,28 +32,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +87,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +162,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_ENBIT(5) +#define SSS_HASH_INI
[PATCH v8 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch change spaces to tabs, second adds HASH support for Exynos. Changes: version 8: - fixes suggested by Vladimir Zapolskiy: drop first condition check in s5p_hash_import, delete unused include delay.h, fix typo in commit message, fix descriptions of struct s5p_hash_reqctx and function s5p_hash_final() version 7: - fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): crypto: s5p-sss: Change spaces to tabs crypto: s5p-sss: Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1596 +++--- 2 files changed, 1505 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
[PATCH v8 1/2] crypto: s5p-sss: Change spaces to tabs
Change #define lines to use tabs consistently. Acked-by: Vladimir Zapolskiy Reviewed-by: Krzysztof Kozlowski Signed-off-by: Kamil Konieczny --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_
[PATCH] Documentation: crypto: Fixed bugs, added example usage of calc_hash().
- Fixed bugs in example for shash and rng (added missing "*" and " *").
- Corrected pr_info() in calc_hash().
- Added example usage of calc_hash().
- No need for negate PTR_ERR to get error code, as crypto_alloc_rng
already returns negative values like ERR_PTR(-ENOMEM). Fixed.
Signed-off-by: Kamil Konieczny
---
Documentation/crypto/api-samples.rst | 38 ++--
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst
b/Documentation/crypto/api-samples.rst
index d021fd96a76d..2531948db89f 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -155,9 +155,9 @@ Code Example For Use of Operational State Memory With SHASH
char ctx[];
};
-static struct sdesc init_sdesc(struct crypto_shash *alg)
+static struct sdesc *init_sdesc(struct crypto_shash *alg)
{
-struct sdesc sdesc;
+struct sdesc *sdesc;
int size;
size = sizeof(struct shash_desc) + crypto_shash_descsize(alg);
@@ -169,15 +169,16 @@ Code Example For Use of Operational State Memory With
SHASH
return sdesc;
}
-static int calc_hash(struct crypto_shashalg,
- const unsigned chardata, unsigned int datalen,
- unsigned chardigest) {
-struct sdesc sdesc;
+static int calc_hash(struct crypto_shash *alg,
+ const unsigned char *data, unsigned int datalen,
+ unsigned char *digest)
+{
+struct sdesc *sdesc;
int ret;
sdesc = init_sdesc(alg);
if (IS_ERR(sdesc)) {
-pr_info("trusted_key: can't alloc %s\n", hash_alg);
+pr_info("can't alloc sdesc\n");
return PTR_ERR(sdesc);
}
@@ -186,6 +187,23 @@ Code Example For Use of Operational State Memory With SHASH
return ret;
}
+static int test_hash(const unsigned char *data, unsigned int datalen,
+ unsigned char *digest)
+{
+struct crypto_shash *alg;
+char *hash_alg_name = "sha1-padlock-nano";
+int ret;
+
+alg = crypto_alloc_shash(hash_alg_name, CRYPTO_ALG_TYPE_SHASH, 0);
+if (IS_ERR(alg)) {
+pr_info("can't alloc alg %s\n", hash_alg_name);
+return PTR_ERR(alg);
+}
+ret = calc_hash(alg, data, datalen, digest);
+crypto_free_shash(alg);
+return ret;
+}
+
Code Example For Random Number Generator Usage
--
@@ -195,8 +213,8 @@ Code Example For Random Number Generator Usage
static int get_random_numbers(u8 *buf, unsigned int len)
{
-struct crypto_rngrng = NULL;
-chardrbg = "drbg_nopr_sha256"; /* Hash DRBG with SHA-256, no PR */
+struct crypto_rng *rng = NULL;
+char *drbg = "drbg_nopr_sha256"; /* Hash DRBG with SHA-256, no PR */
int ret;
if (!buf || !len) {
@@ -207,7 +225,7 @@ Code Example For Random Number Generator Usage
rng = crypto_alloc_rng(drbg, 0, 0);
if (IS_ERR(rng)) {
pr_debug("could not allocate RNG handle for %s\n", drbg);
-return -PTR_ERR(rng);
+return PTR_ERR(rng);
}
ret = crypto_rng_get_bytes(rng, buf, len);
--
2.7.4
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:16, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny
> ---
> drivers/crypto/mxs-dcp.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index 764be3e6933c..a10c418d4e5c 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
> return dcp_sha_finup(req);
> }
>
> +static int dcp_sha_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int dcp_sha_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int dcp_sha_cra_init(struct crypto_tfm *tfm)
> {
> crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
> @@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
> .final = dcp_sha_final,
> .finup = dcp_sha_finup,
> .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
> .halg = {
> .digestsize = SHA1_DIGEST_SIZE,
> .base = {
> @@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
> .final = dcp_sha_final,
> .finup = dcp_sha_finup,
> .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
> .halg = {
> .digestsize = SHA256_DIGEST_SIZE,
> .base = {
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH] crypto: n2_core: Add empty hash export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:18, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny
> ---
> drivers/crypto/n2_core.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
> index 662e709812cc..80e9c842aad4 100644
> --- a/drivers/crypto/n2_core.c
> +++ b/drivers/crypto/n2_core.c
> @@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
> return crypto_ahash_finup(&rctx->fallback_req);
> }
>
> +static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int n2_hash_async_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int n2_hash_cra_init(struct crypto_tfm *tfm)
> {
> const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
> @@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
> n2_hash_tmpl *tmpl)
> ahash->final = n2_hash_async_final;
> ahash->finup = n2_hash_async_finup;
> ahash->digest = n2_hash_async_digest;
> + ahash->export = n2_hash_async_noexport;
> + ahash->import = n2_hash_async_noimport;
>
> halg = &ahash->halg;
> halg->digestsize = tmpl->digest_size;
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH] crypto: ux500/hash: Add empty export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:32, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny
> ---
> drivers/crypto/ux500/hash/hash_core.c | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/crypto/ux500/hash/hash_core.c
> b/drivers/crypto/ux500/hash/hash_core.c
> index 9acccad26928..2d0a677bcc76 100644
> --- a/drivers/crypto/ux500/hash/hash_core.c
> +++ b/drivers/crypto/ux500/hash/hash_core.c
> @@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request
> *req)
> return ret1 ? ret1 : ret2;
> }
>
> +static int ahash_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int ahash_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int hmac_sha1_init(struct ahash_request *req)
> {
> struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> @@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
> .update = ahash_update,
> .final = ahash_final,
> .digest = ahash_sha1_digest,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA1_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
> .update = ahash_update,
> .final = ahash_final,
> .digest = ahash_sha256_digest,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA256_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
> .final = ahash_final,
> .digest = hmac_sha1_digest,
> .setkey = hmac_sha1_setkey,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA1_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
> .final = ahash_final,
> .digest = hmac_sha256_digest,
> .setkey = hmac_sha256_setkey,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA256_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
[PATCH v3 0/5] crypto: ahash.c: Require export/import in ahash
First four patches add empty hash export and import functions to each driver, with the same behaviour as in crypto framework. The last one drops them from crypto framework. Last one for ahash.c depends on all previous. Changes in v3: added change for bfin_crc.c make this a patchset, instead of unreleated patches make commit message more descriptive Kamil Konieczny (5): crypto: mxs-dcp: Add empty hash export and import crypto: n2_core: Add empty hash export and import crypto: ux500/hash: Add empty export and import crypto: bfin_crc: Add empty hash export and import crypto: ahash.c: Require export/import in ahash crypto/ahash.c| 18 ++ drivers/crypto/bfin_crc.c | 12 drivers/crypto/mxs-dcp.c | 14 ++ drivers/crypto/n2_core.c | 12 drivers/crypto/ux500/hash/hash_core.c | 18 ++ 5 files changed, 58 insertions(+), 16 deletions(-) -- 2.15.0
[PATCH 2/5] crypto: n2_core: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/n2_core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index 662e709812cc..80e9c842aad4 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
return crypto_ahash_finup(&rctx->fallback_req);
}
+static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int n2_hash_async_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int n2_hash_cra_init(struct crypto_tfm *tfm)
{
const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
@@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
n2_hash_tmpl *tmpl)
ahash->final = n2_hash_async_final;
ahash->finup = n2_hash_async_finup;
ahash->digest = n2_hash_async_digest;
+ ahash->export = n2_hash_async_noexport;
+ ahash->import = n2_hash_async_noimport;
halg = &ahash->halg;
halg->digestsize = tmpl->digest_size;
--
2.15.0
[PATCH v3 1/5] crypto: mxs-dcp: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/mxs-dcp.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 764be3e6933c..a10c418d4e5c 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
return dcp_sha_finup(req);
}
+static int dcp_sha_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int dcp_sha_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int dcp_sha_cra_init(struct crypto_tfm *tfm)
{
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
@@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA1_DIGEST_SIZE,
.base = {
@@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA256_DIGEST_SIZE,
.base = {
--
2.15.0
[PATCH 5/5] crypto: ahash.c: Require export/import in ahash
Export and import are mandatory in async hash. As drivers were
rewritten, drop empty wrappers and correct init of ahash transformation.
Signed-off-by: Kamil Konieczny
---
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..c3cce508c1d4 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->final = alg->final;
hash->finup = alg->finup ?: ahash_def_finup;
hash->digest = alg->digest;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (alg->setkey) {
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
[PATCH 3/5] crypto: ux500/hash: Add empty export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny
Acked-by: Linus Walleij
---
drivers/crypto/ux500/hash/hash_core.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/crypto/ux500/hash/hash_core.c
b/drivers/crypto/ux500/hash/hash_core.c
index 9acccad26928..2d0a677bcc76 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request *req)
return ret1 ? ret1 : ret2;
}
+static int ahash_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int ahash_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int hmac_sha1_init(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha1_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha256_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha1_digest,
.setkey = hmac_sha1_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha256_digest,
.setkey = hmac_sha256_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
--
2.15.0
[PATCH 4/5] crypto: bfin_crc: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them.
Add empty hash export and import, with the same behaviour as in framework
and expose this directly in driver. This can also prevent OOPS when config
option in Cryptographic API 'Disable run-time self tests' will be enabled.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/bfin_crc.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/bfin_crc.c b/drivers/crypto/bfin_crc.c
index a118b9bed669..65a8e07835e8 100644
--- a/drivers/crypto/bfin_crc.c
+++ b/drivers/crypto/bfin_crc.c
@@ -450,6 +450,16 @@ static int bfin_crypto_crc_digest(struct ahash_request
*req)
return bfin_crypto_crc_finup(req);
}
+static int bfin_crypto_crc_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int bfin_crypto_crc_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int bfin_crypto_crc_setkey(struct crypto_ahash *tfm, const u8 *key,
unsigned int keylen)
{
@@ -487,6 +497,8 @@ static struct ahash_alg algs = {
.final = bfin_crypto_crc_final,
.finup = bfin_crypto_crc_finup,
.digest = bfin_crypto_crc_digest,
+ .export = bfin_crypto_crc_noexport,
+ .import = bfin_crypto_crc_noimport,
.setkey = bfin_crypto_crc_setkey,
.halg.digestsize= CHKSUM_DIGEST_SIZE,
.halg.base = {
--
2.15.0
Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash
On 18.01.2018 22:31, Marek Vasut wrote:
> On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
>> Export and import are mandatory in async hash. As drivers were
>> rewritten, drop empty wrappers and correct init of ahash transformation.
>
> Are you moving checks from the core subsystem to drivers ? This looks
> really nonsensical and the commit message doesn't explain the rationale
> for that at all.
I am removing checks from core. Export and import were optional in beginnig
of crypto framework, but as time goes on they become mandatory.
>
>> Signed-off-by: Kamil Konieczny
>> ---
>> crypto/ahash.c | 18 ++
>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/crypto/ahash.c b/crypto/ahash.c
>> index 3a35d67de7d9..c3cce508c1d4 100644
>> --- a/crypto/ahash.c
>> +++ b/crypto/ahash.c
>> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>> return ahash_def_finup_finish1(req, err);
>> }
>>
>> -static int ahash_no_export(struct ahash_request *req, void *out)
>> -{
>> -return -ENOSYS;
>> -}
>> -
>> -static int ahash_no_import(struct ahash_request *req, const void *in)
>> -{
>> -return -ENOSYS;
>> -}
>> -
>> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>> {
>> struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
>> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>
>> hash->setkey = ahash_nosetkey;
>> hash->has_setkey = false;
>> -hash->export = ahash_no_export;
>> -hash->import = ahash_no_import;
>>
>> if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
>> return crypto_init_shash_ops_async(tfm);
>> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>> *tfm)
>> hash->final = alg->final;
>> hash->finup = alg->finup ?: ahash_def_finup;
>> hash->digest = alg->digest;
>> +hash->export = alg->export;
>> +hash->import = alg->import;
>>
>> if (alg->setkey) {
>> hash->setkey = alg->setkey;
>> hash->has_setkey = true;
>> }
>> -if (alg->export)
>> -hash->export = alg->export;
>> -if (alg->import)
>> -hash->import = alg->import;
>>
>> return 0;
>> }
>>
>
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash
On 19.01.2018 11:08, Marek Vasut wrote:
> On 01/19/2018 10:53 AM, Kamil Konieczny wrote:
>> On 18.01.2018 22:31, Marek Vasut wrote:
>>> On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
>>>> Export and import are mandatory in async hash. As drivers were
>>>> rewritten, drop empty wrappers and correct init of ahash transformation.
>>>
>>> Are you moving checks from the core subsystem to drivers ? This looks
>>> really nonsensical and the commit message doesn't explain the rationale
>>> for that at all.
>>
>> I am removing checks from core. Export and import were optional in beginnig
>> of crypto framework, but as time goes on they become mandatory.
>
> Seems like if the driver doesn't implement those, the core can easily
> detect that and perform the necessary action. Moving the checks out of
> core seems like the wrong thing to do, rather you should enhance the
> checks in core if they're insufficient in my opinion.
I removed all checks. No checks in driver and no checks in crypto framework.
If you would like any check, I think the place to add them is in ahash alg
registraction, in function ahash_prepare_alg add something like:
if ((alg->init == NULL) ||
(alg->digest == NULL) ||
(alg->final == NULL) ||
(alg->update == NULL) ||
(alg->export == NULL) ||
(alg->import == NULL))
return -EINVAL;
The only downsize is this will be usefull in backport (to prevent NULL
dereference),
as any new driver will have all those pointers sets.
>>>> Signed-off-by: Kamil Konieczny
>>>> ---
>>>> crypto/ahash.c | 18 ++
>>>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/crypto/ahash.c b/crypto/ahash.c
>>>> index 3a35d67de7d9..c3cce508c1d4 100644
>>>> --- a/crypto/ahash.c
>>>> +++ b/crypto/ahash.c
>>>> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>>>>return ahash_def_finup_finish1(req, err);
>>>> }
>>>>
>>>> -static int ahash_no_export(struct ahash_request *req, void *out)
>>>> -{
>>>> - return -ENOSYS;
>>>> -}
>>>> -
>>>> -static int ahash_no_import(struct ahash_request *req, const void *in)
>>>> -{
>>>> - return -ENOSYS;
>>>> -}
>>>> -
>>>> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>>> {
>>>>struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
>>>> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>>>> *tfm)
>>>>
>>>>hash->setkey = ahash_nosetkey;
>>>>hash->has_setkey = false;
>>>> - hash->export = ahash_no_export;
>>>> - hash->import = ahash_no_import;
>>>>
>>>>if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
>>>>return crypto_init_shash_ops_async(tfm);
>>>> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>>>> *tfm)
>>>>hash->final = alg->final;
>>>>hash->finup = alg->finup ?: ahash_def_finup;
>>>>hash->digest = alg->digest;
>>>> + hash->export = alg->export;
>>>> + hash->import = alg->import;
>>>>
>>>>if (alg->setkey) {
>>>>hash->setkey = alg->setkey;
>>>>hash->has_setkey = true;
>>>>}
>>>> - if (alg->export)
>>>> - hash->export = alg->export;
>>>> - if (alg->import)
>>>> - hash->import = alg->import;
>>>>
>>>>return 0;
>>>> }
>>>>
>>>
>>>
>>
>
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH] crypto: clear htmldocs build warnings for crypto/hash
On 07.01.2018 00:01, Tobin C. Harding wrote:
> SPHINX build emits multiple warnings of kind:
>
> warning: duplicate section name 'Note'
>
> (when building kernel via make target 'htmldocs')
>
> This is caused by repeated use of comments of form:
>
> * Note: soau soaeusoa uoe
>
> We can change the format without loss of clarity and clear the build
> warnings.
>
> Add '**[mandatory]**' or '**[optional]**' as kernel-doc field element
> description prefix
>
> This renders in HTML as (prefixes in bold)
>
> final
> [mandatory] Retrieve result from the driver. This function finalizes the
> transformation and retrieves the resulting hash from the driver and
> pushes it back to upper layers. No data processing happens at this
> point unless hardware requires it to finish the transformation (then
> the data buffered by the device driver is processed).
>
> Signed-off-by: Tobin C. Harding
> ---
>
> This patch begs the question why the other members of struct ahash_alg
> are not marked? Some are marked 'optional' some 'mandatory'. It would
> seem that if the marking were necessary for some members it is necessary
> for all to eliminate ambiguity?
>
> thanks
import, export are optional
digest is mandatory
see declaration for function
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
in crypto/hash.c
setkey is not allowed in hash, but mandatory for HMAC
(as you can find out from description)
> include/crypto/hash.h | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
> index 0ed31fd80242..d1cd75faff40 100644
> --- a/include/crypto/hash.h
> +++ b/include/crypto/hash.h
> @@ -71,12 +71,11 @@ struct ahash_request {
>
> /**
> * struct ahash_alg - asynchronous message digest definition
> - * @init: Initialize the transformation context. Intended only to initialize
> the
> + * @init: **[mandatory]** Initialize the transformation context. Intended
> only to initialize the
> * state of the HASH transformation at the beginning. This shall fill in
> * the internal structures used during the entire duration of the whole
> * transformation. No data processing happens at this point.
> - * Note: mandatory.
> - * @update: Push a chunk of data into the driver for transformation. This
> + * @update: **[mandatory]** Push a chunk of data into the driver for
> transformation. This
> * function actually pushes blocks of data from upper layers into the
> * driver, which then passes those to the hardware as seen fit. This
> * function must not finalize the HASH transformation by calculating the
> @@ -85,20 +84,17 @@ struct ahash_request {
> * context, as this function may be called in parallel with the same
> * transformation object. Data processing can happen synchronously
> * [SHASH] or asynchronously [AHASH] at this point.
> - * Note: mandatory.
> - * @final: Retrieve result from the driver. This function finalizes the
> + * @final: **[mandatory]** Retrieve result from the driver. This function
> finalizes the
> * transformation and retrieves the resulting hash from the driver and
> * pushes it back to upper layers. No data processing happens at this
> * point unless hardware requires it to finish the transformation
> * (then the data buffered by the device driver is processed).
> - * Note: mandatory.
> - * @finup: Combination of @update and @final. This function is effectively a
> + * @finup: **[optional]** Combination of @update and @final. This function
> is effectively a
> * combination of @update and @final calls issued in sequence. As some
> * hardware cannot do @update and @final separately, this callback was
> * added to allow such hardware to be used at least by IPsec. Data
> * processing can happen synchronously [SHASH] or asynchronously [AHASH]
> * at this point.
> - * Note: optional.
> * @digest: Combination of @init and @update and @final. This function
> * effectively behaves as the entire chain of operations, @init,
> * @update and @final issued in sequence. Just like @finup, this was
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH] crypto: clear htmldocs build warnings for crypto/hash
On 08.01.2018 16:56, Herbert Xu wrote: > On Mon, Jan 08, 2018 at 02:11:21PM +0100, Kamil Konieczny wrote: >> >> >> On 07.01.2018 00:01, Tobin C. Harding wrote: >>> SPHINX build emits multiple warnings of kind: >>> >>> warning: duplicate section name 'Note' >>> >>> (when building kernel via make target 'htmldocs') >>> >>> This is caused by repeated use of comments of form: >>> >>> * Note: soau soaeusoa uoe >>> >>> We can change the format without loss of clarity and clear the build >>> warnings. >>> >>> Add '**[mandatory]**' or '**[optional]**' as kernel-doc field element >>> description prefix >>> >>> This renders in HTML as (prefixes in bold) >>> >>> final >>> [mandatory] Retrieve result from the driver. This function finalizes the >>> transformation and retrieves the resulting hash from the driver and >>> pushes it back to upper layers. No data processing happens at this >>> point unless hardware requires it to finish the transformation (then >>> the data buffered by the device driver is processed). >>> >>> Signed-off-by: Tobin C. Harding >>> --- >>> >>> This patch begs the question why the other members of struct ahash_alg >>> are not marked? Some are marked 'optional' some 'mandatory'. It would >>> seem that if the marking were necessary for some members it is necessary >>> for all to eliminate ambiguity? >>> >>> thanks >> >> import, export are optional > > No import/export must be implemented for all hashes. Is it mandatory for both async hash and shash ? in crypto/ahash.c in function static int crypto_ahash_init_tfm(struct crypto_tfm *tfm) there is: hash->export = ahash_no_export; hash->import = ahash_no_import; and later in the same function: if (alg->export) hash->export = alg->export; if (alg->import) hash->import = alg->import; -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH] crypto: n2_core: Add empty hash export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/n2_core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index 662e709812cc..80e9c842aad4 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
return crypto_ahash_finup(&rctx->fallback_req);
}
+static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int n2_hash_async_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int n2_hash_cra_init(struct crypto_tfm *tfm)
{
const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
@@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
n2_hash_tmpl *tmpl)
ahash->final = n2_hash_async_final;
ahash->finup = n2_hash_async_finup;
ahash->digest = n2_hash_async_digest;
+ ahash->export = n2_hash_async_noexport;
+ ahash->import = n2_hash_async_noimport;
halg = &ahash->halg;
halg->digestsize = tmpl->digest_size;
--
2.15.0
[PATCH] crypto: ux500/hash: Add empty export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/ux500/hash/hash_core.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/crypto/ux500/hash/hash_core.c
b/drivers/crypto/ux500/hash/hash_core.c
index 9acccad26928..2d0a677bcc76 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request *req)
return ret1 ? ret1 : ret2;
}
+static int ahash_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int ahash_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int hmac_sha1_init(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha1_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha256_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha1_digest,
.setkey = hmac_sha1_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha256_digest,
.setkey = hmac_sha256_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
--
2.15.0
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
On 16.01.2018 17:56, Marek Vasut wrote: > On 01/16/2018 05:16 PM, Kamil Konieczny wrote: >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. > > Shouldn't this be handled on the subsystem level with some > > if (foo->bar) > foo->bar(); > > instead? I am sorry, I should write more elaborate description for patch. It is handled by subsystem. Most drivers have them, and testmgr is testing for export/import and drivers without them fail internal crypto tests, so I prepared patch which removed these two wrappers from crypto framework. In summary: export/import are now required, so crypto framework can work properly. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
On 16.01.2018 18:28, Fabio Estevam wrote: > Hi Kamil, > > On Tue, Jan 16, 2018 at 2:16 PM, Kamil Konieczny > wrote: >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. > > Which Oops exactly are you getting? None now, it is for preparation for patch removing export/import wrappers. > > Just booted 4.14.13 and the mxs-dcp driver does not even probe successfully: > > [2.455404] mxs-dcp 80028000.dcp: Failed to register sha1 hash! > [2.464042] mxs-dcp: probe of 80028000.dcp failed with error -22 > With this option turned on in config: crypto: Disable run-time self tests driver should load. Can you verify ? Btw, there is no maintainer for this file. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH v2] crypto/ahash: Require export/import in ahash
Export and import were optional in async hash. As most drivers were
rewritten, they become mandatory now, so correct init of ahash
transformation.
Signed-off-by: Kamil Konieczny
---
This is resend of previous patch. As Bartlomiej Zolnierkiewicz pointed out,
there are still three crypto drivers that didn't have export/import implemented:
drivers/crypto/mxs-dcp.c
drivers/crypto/n2_core.c
drivers/crypto/ux500/hash/hash_core.c
I have no documentation for them, so I sended patches with the behaviour taken
from crypto framework, but maybe that hardware is capable of import/export,
so proper implementation is possible. Unfortunatly, there is no maintainer
for any of these files.
Please take this patch after these remainig drivers will be patched.
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..7a8906d5af53 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
Re: [PATCH v2] crypto/ahash: Require export/import in ahash
On 16.01.2018 19:38, Kamil Konieczny wrote:
> Export and import were optional in async hash. As most drivers were
> rewritten, they become mandatory now, so correct init of ahash
> transformation.
>
> Signed-off-by: Kamil Konieczny
Please drop this patch, as there is one more driver needed for convert,
namely bfin_crc.c
I will also prepare this as patch series, to be sure that this patch is last
> ---
> This is resend of previous patch. As Bartlomiej Zolnierkiewicz pointed out,
> there are still three crypto drivers that didn't have export/import
> implemented:
>
> drivers/crypto/mxs-dcp.c
> drivers/crypto/n2_core.c
> drivers/crypto/ux500/hash/hash_core.c
>
> I have no documentation for them, so I sended patches with the behaviour taken
> from crypto framework, but maybe that hardware is capable of import/export,
> so proper implementation is possible. Unfortunatly, there is no maintainer
> for any of these files.
>
> Please take this patch after these remainig drivers will be patched.
>
> crypto/ahash.c | 18 ++
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/crypto/ahash.c b/crypto/ahash.c
> index 3a35d67de7d9..7a8906d5af53 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
> return ahash_def_finup_finish1(req, err);
> }
>
> -static int ahash_no_export(struct ahash_request *req, void *out)
> -{
> - return -ENOSYS;
> -}
> -
> -static int ahash_no_import(struct ahash_request *req, const void *in)
> -{
> - return -ENOSYS;
> -}
> -
> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
> {
> struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
> @@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>
> hash->setkey = ahash_nosetkey;
> hash->has_setkey = false;
> - hash->export = ahash_no_export;
> - hash->import = ahash_no_import;
> + hash->export = alg->export;
> + hash->import = alg->import;
>
> if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
> return crypto_init_shash_ops_async(tfm);
> @@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
> hash->setkey = alg->setkey;
> hash->has_setkey = true;
> }
> - if (alg->export)
> - hash->export = alg->export;
> - if (alg->import)
> - hash->import = alg->import;
>
> return 0;
> }
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH] crypto: ux500/hash: Add empty export and import
On 18.01.2018 11:06, Linus Walleij wrote: > On Tue, Jan 16, 2018 at 5:32 PM, Kamil Konieczny > wrote: > >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. >> >> Signed-off-by: Kamil Konieczny > > Acked-by: Linus Walleij > > But why isn't the framework code just checking the vtable for NULL? > > if (foo->fp) > foo->fp(bar); This will be inefficient, it should be checked once at ahash alg register, or with the old method by using wrapper -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[RFT PATCH] crypto: ahash.c: Require export/import in ahash
Export and import were optional in async hash. As drivers were rewritten,
they become mandatory now, so correct init of ahash transformation.
Signed-off-by: Kamil Konieczny
---
Tested with crypto run-time self test on Odroid-U3 with Exynos 4412 CPU,
with insmod s5p-sss.ko
Please test with other crypto hash drivers.
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..7a8906d5af53 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
[PATCH] crypto: testmgr.c: test misuse of result in ahash
Async hash operations can use result pointer in final/finup/digest,
but not in init/update/export/import, so test it for misuse.
Signed-off-by: Kamil Konieczny
---
Tested with crypto run-time self test on Odroid-U3 with Exynos 4412 CPU
with insmod s5p-sss.ko
crypto/testmgr.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 44a85d4b3561..d5e23a142a04 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -177,6 +177,18 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
free_page((unsigned long)buf[i]);
}
+static int ahash_guard_result(char *result, char c, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ if (result[i] != c)
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ahash_partial_update(struct ahash_request **preq,
struct crypto_ahash *tfm, const struct hash_testvec *template,
void *hash_buff, int k, int temp, struct scatterlist *sg,
@@ -186,6 +198,7 @@ static int ahash_partial_update(struct ahash_request **preq,
struct ahash_request *req;
int statesize, ret = -EINVAL;
static const unsigned char guard[] = { 0x00, 0xba, 0xad, 0x00 };
+ int digestsize = crypto_ahash_digestsize(tfm);
req = *preq;
statesize = crypto_ahash_statesize(
@@ -196,12 +209,19 @@ static int ahash_partial_update(struct ahash_request
**preq,
goto out_nostate;
}
memcpy(state + statesize, guard, sizeof(guard));
+ memset(result, 1, digestsize);
ret = crypto_ahash_export(req, state);
WARN_ON(memcmp(state + statesize, guard, sizeof(guard)));
if (ret) {
pr_err("alg: hash: Failed to export() for %s\n", algo);
goto out;
}
+ ret = ahash_guard_result(result, 1, digestsize);
+ if (ret) {
+ pr_err("alg: hash: Failed, export used req->result for %s\n",
+ algo);
+ goto out;
+ }
ahash_request_free(req);
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -221,6 +241,12 @@ static int ahash_partial_update(struct ahash_request
**preq,
pr_err("alg: hash: Failed to import() for %s\n", algo);
goto out;
}
+ ret = ahash_guard_result(result, 1, digestsize);
+ if (ret) {
+ pr_err("alg: hash: Failed, import used req->result for %s\n",
+ algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_update(req), wait);
if (ret)
goto out;
@@ -316,18 +342,31 @@ static int __test_hash(struct crypto_ahash *tfm,
goto out;
}
} else {
+ memset(result, 1, digest_size);
ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alg: hash: init failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
+ ret = ahash_guard_result(result, 1, digest_size);
+ if (ret) {
+ pr_err("alg: hash: init failed on test %d "
+ "for %s: used req->result\n", j, algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alg: hash: update failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
+ ret = ahash_guard_result(result, 1, digest_size);
+ if (ret) {
+ pr_err("alg: hash: update failed on test %d "
+ "for %s: used req->result\n", j, algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_final(req), &wait);
if (ret) {
pr_err("alg: hash: final failed on test %d "
--
2.15.0
[PATCH] crypto: mxs-dcp: Add empty hash export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/mxs-dcp.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 764be3e6933c..a10c418d4e5c 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
return dcp_sha_finup(req);
}
+static int dcp_sha_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int dcp_sha_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int dcp_sha_cra_init(struct crypto_tfm *tfm)
{
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
@@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA1_DIGEST_SIZE,
.base = {
@@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA256_DIGEST_SIZE,
.base = {
--
2.15.0
[PATCH] crypto: hash.h: Prevent use of req->digest in ahash update
Prevent improper use of req->digest field in ahash update, init, export and
import functions in drivers code. A driver should use ahash request context
if it needs to save internal state.
Signed-off-by: Kamil Konieczny
---
include/crypto/hash.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 2d1849dffb80..e97c2e662d6a 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -74,7 +74,8 @@ struct ahash_request {
* @init: **[mandatory]** Initialize the transformation context. Intended only
to initialize the
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
- * transformation. No data processing happens at this point.
+ * transformation. No data processing happens at this point. Driver code
+ * implementation must not use req->digest.
* @update: **[mandatory]** Push a chunk of data into the driver for
transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -83,7 +84,8 @@ struct ahash_request {
*transformation. This function shall not modify the transformation
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
- *[SHASH] or asynchronously [AHASH] at this point.
+ *[SHASH] or asynchronously [AHASH] at this point. Driver must not use
+ *req->digest.
* @final: **[mandatory]** Retrieve result from the driver. This function
finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
@@ -120,11 +122,12 @@ struct ahash_request {
* you want to save partial result of the transformation after
* processing certain amount of data and reload this partial result
* multiple times later on for multiple re-use. No data processing
- * happens at this point.
+ * happens at this point. Driver must not use req->digest.
* @import: Import partial state of the transformation. This function loads the
* entire state of the ongoing transformation from a provided block of
* data so the transformation can continue from this point onward. No
- * data processing happens at this point.
+ * data processing happens at this point. Driver must not use
+ * req->digest.
* @halg: see struct hash_alg_common
*/
struct ahash_alg {
--
2.16.2
Re: [PATCH] crypto: hash.h: Prevent use of req->digest in ahash update
On 06.03.2018 19:04, Tom Lendacky wrote:
> On 3/6/2018 5:45 AM, Kamil Konieczny wrote:
>> Prevent improper use of req->digest field in ahash update, init, export and
>
> Shouldn't that be req->result (here and below)?
Yes, it should, I will send version 2 soon,
thank you.
Best regards,
Kamil Konieczny
>> import functions in drivers code. A driver should use ahash request context
>> if it needs to save internal state.
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> include/crypto/hash.h | 11 +++
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
>> index 2d1849dffb80..e97c2e662d6a 100644
>> --- a/include/crypto/hash.h
>> +++ b/include/crypto/hash.h
>> @@ -74,7 +74,8 @@ struct ahash_request {
>> * @init: **[mandatory]** Initialize the transformation context. Intended
>> only to initialize the
>> *state of the HASH transformation at the beginning. This shall fill in
>> *the internal structures used during the entire duration of the whole
>> - *transformation. No data processing happens at this point.
>> + *transformation. No data processing happens at this point. Driver code
>> + *implementation must not use req->digest.
>> * @update: **[mandatory]** Push a chunk of data into the driver for
>> transformation. This
>> * function actually pushes blocks of data from upper layers into the
>> * driver, which then passes those to the hardware as seen fit. This
>> @@ -83,7 +84,8 @@ struct ahash_request {
>> * transformation. This function shall not modify the transformation
>> * context, as this function may be called in parallel with the same
>> * transformation object. Data processing can happen synchronously
>> - * [SHASH] or asynchronously [AHASH] at this point.
>> + * [SHASH] or asynchronously [AHASH] at this point. Driver must not use
>> + * req->digest.
>> * @final: **[mandatory]** Retrieve result from the driver. This function
>> finalizes the
>> * transformation and retrieves the resulting hash from the driver and
>> * pushes it back to upper layers. No data processing happens at this
>> @@ -120,11 +122,12 @@ struct ahash_request {
>> * you want to save partial result of the transformation after
>> * processing certain amount of data and reload this partial result
>> * multiple times later on for multiple re-use. No data processing
>> - * happens at this point.
>> + * happens at this point. Driver must not use req->digest.
>> * @import: Import partial state of the transformation. This function loads
>> the
>> * entire state of the ongoing transformation from a provided block of
>> * data so the transformation can continue from this point onward. No
>> - * data processing happens at this point.
>> + * data processing happens at this point. Driver must not use
>> + * req->digest.
>> * @halg: see struct hash_alg_common
>> */
>> struct ahash_alg {
>>
>
>
>
[PATCH v2] crypto: hash.h: Prevent use of req->result in ahash update
Prevent improper use of req->result field in ahash update, init, export and
import functions in drivers code. A driver should use ahash request context
if it needs to save internal state.
Signed-off-by: Kamil Konieczny
---
version 2:
Change req->digest to req->result, as pointed out by Tom Lendacky
include/crypto/hash.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 2d1849dffb80..76e432cab75d 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -74,7 +74,8 @@ struct ahash_request {
* @init: **[mandatory]** Initialize the transformation context. Intended only
to initialize the
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
- * transformation. No data processing happens at this point.
+ * transformation. No data processing happens at this point. Driver code
+ * implementation must not use req->result.
* @update: **[mandatory]** Push a chunk of data into the driver for
transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -83,7 +84,8 @@ struct ahash_request {
*transformation. This function shall not modify the transformation
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
- *[SHASH] or asynchronously [AHASH] at this point.
+ *[SHASH] or asynchronously [AHASH] at this point. Driver must not use
+ *req->result.
* @final: **[mandatory]** Retrieve result from the driver. This function
finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
@@ -120,11 +122,12 @@ struct ahash_request {
* you want to save partial result of the transformation after
* processing certain amount of data and reload this partial result
* multiple times later on for multiple re-use. No data processing
- * happens at this point.
+ * happens at this point. Driver must not use req->result.
* @import: Import partial state of the transformation. This function loads the
* entire state of the ongoing transformation from a provided block of
* data so the transformation can continue from this point onward. No
- * data processing happens at this point.
+ * data processing happens at this point. Driver must not use
+ * req->result.
* @halg: see struct hash_alg_common
*/
struct ahash_alg {
--
2.16.2
Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine
On 20.03.2018 08:56, Horia Geantă wrote: > Add a note that it is perfectly legal to "abandon" a request object: > - call .init() and then (as many times) .update() > - _not_ call any of .final(), .finup() or .export() at any point in > future > > Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au > Signed-off-by: Horia Geantă > --- > Documentation/crypto/devel-algos.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/crypto/devel-algos.rst > b/Documentation/crypto/devel-algos.rst > index 66f50d32dcec..c45c6f400dbd 100644 > --- a/Documentation/crypto/devel-algos.rst > +++ b/Documentation/crypto/devel-algos.rst > @@ -236,6 +236,14 @@ when used from another part of the kernel. > | > '---> HASH2 > > +Note that it is perfectly legal to "abandon" a request object: > +- call .init() and then (as many times) .update() > +- _not_ call any of .final(), .finup() or .export() at any point in future > + > +In other words implementations should mind the resource allocation and > clean-up. > +No resources related to request objects should remain allocated after a call -- > +to .init() or .update(), since there might be no chance to free them. is it for crypto api users or for drivers ? the creator of request context is responsible for alloc and destroy, so why there are no chance of free ? -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH 3/4] crypto: bcm: Constify *hash_alg_name[]
On 27.02.2018 23:01, Hernán Gonzalez wrote:
> Note: This is compile only tested.
> No gain from this except some self-documenting.
>
> Signed-off-by: Hernán Gonzalez
> ---
> drivers/crypto/bcm/spu.c | 5 +++--
> drivers/crypto/bcm/spu.h | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/bcm/spu.c b/drivers/crypto/bcm/spu.c
> index efaf3cf..c7bb79e 100644
> --- a/drivers/crypto/bcm/spu.c
> +++ b/drivers/crypto/bcm/spu.c
> @@ -23,8 +23,9 @@
> #include "cipher.h"
>
> /* This array is based on the hash algo type supported in spu.h */
> -char *hash_alg_name[] = { "None", "md5", "sha1", "sha224", "sha256", "aes",
^^^
'aes' is not hash, so either remove 'aes' or change array name to
crypto_alg_name
Or maybe I am missing something, or is it hardcoded in silicon ?
> - "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384", "sha3_512" };
> +char const * const hash_alg_name[] = { "None", "md5", "sha1", "sha224",
> + "sha256", "aes", "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384",
> + "sha3_512" };
>
> /* Assumes SPU-M messages are in big endian */
> void spum_dump_msg_hdr(u8 *buf, unsigned int buf_len)
> diff --git a/drivers/crypto/bcm/spu.h b/drivers/crypto/bcm/spu.h
> index f252367..71cf6b5 100644
> --- a/drivers/crypto/bcm/spu.h
> +++ b/drivers/crypto/bcm/spu.h
> @@ -111,7 +111,7 @@ enum aead_type {
> AEAD_TYPE_LAST
> };
>
> -extern char *hash_alg_name[HASH_ALG_LAST];
> +extern const char * const hash_alg_name[HASH_ALG_LAST];
>
> struct spu_request_opts {
> bool is_inbound;
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH 4/4] clk: samsung: exynos5433: add imem clock
Hi, > On 2018년 11월 21일 21:05, Kamil Konieczny wrote: >> Add imem clock for exynos5433. > > It is diffcult to understand the meaning of 'imem' without the description. > Please add more detailed description as the patch2 description. > [...] Thank you for review, will this be enough for description: Add imem clock for exynos5433. This will enable to use crypto Security SubSystem (in short SSS) and SlimSSS IP blocks. In documentation there is no description of "imem" name origin. I can guess "mem" stand for "memory", and 'i' ? internal ? -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH 4/4] clk: samsung: exynos5433: add imem clock
Add imem clock for exynos5433.
Signed-off-by: Kamil Konieczny
---
drivers/clk/samsung/clk-exynos5433.c | 123 +
include/dt-bindings/clock/exynos5433.h | 55 +++
2 files changed, 178 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5433.c
b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..db29cbd1fbdc 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -2345,6 +2345,129 @@ static const struct samsung_cmu_info fsys_cmu_info
__initconst = {
.clk_name = "aclk_fsys_200",
};
+/*
+ * Register offset definitions for CMU_IMEM
+ *
+ */
+
+#define ENABLE_ACLK_IMEM 0x0800
+#define ENABLE_ACLK_IMEM_SSS 0x0808
+#define ENABLE_ACLK_IMEM_SLIMSSS 0x080c
+#define ENABLE_PCLK_IMEM 0x0900
+#define ENABLE_PCLK_IMEM_SSS 0x0904
+#define ENABLE_PCLK_IMEM_SLIMSSS 0x0908
+
+static const unsigned long imem_clk_regs[] __initconst = {
+ENABLE_ACLK_IMEM,
+ENABLE_ACLK_IMEM_SSS,
+ENABLE_ACLK_IMEM_SLIMSSS,
+ENABLE_PCLK_IMEM,
+ENABLE_PCLK_IMEM_SSS,
+ENABLE_PCLK_IMEM_SLIMSSS,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+ /* ENABLE_ACLK_IMEM */
+ GATE(CLK_ACLK_AXI2AHB_IMEMH, "aclk_axi2ahb_imemh", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 24, 0, 0),
+ GATE(CLK_ACLK_AXIDS_SROMC, "aclk_axids_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 23, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_SROMC, "aclk_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 22, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_ARBG_TX, "aclk_bts_arbg_tx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 21, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_IMEMX, "aclk_asyncaxi_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 20, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIUS_IMEMX, "aclk_axius_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 19, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_XIU_IMEMX, "aclk_xiu_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 18, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_SSSX, "aclk_asyncaxi_sssx", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 17, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SLIMSSS, "aclk_bts_slimsss", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 15, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_DRAM, "aclk_bts_sss_dram", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 14, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_CCI, "aclk_bts_sss_cci", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 13, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ALB_IMEM, "aclk_alb_imem", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 12, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM1P, "aclk_axids_pimemx_imem1p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 11, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM0P, "aclk_axids_pimemx_imem0p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 10, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_GIC, "aclk_axids_pimemx_gic",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 9, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAHBM_SSS_ATLAS, "aclk_asyncahbm_sss_atlas",
"aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 7, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXIS_MIF_PIMEMX, "aclk_asyncaxis_mif_pimemx",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 6, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXI2APB_IMEM1P, "aclk_axi2apb_imem1p", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 5, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXI2APB_IMEM0P, "aclk_axi2apb_imem0p", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 4, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_XIU_SSSX, "aclk_xiu_sssx", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 3, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_XIU_PIMEMX, "aclk_xiu_pimemx", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 2, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_IMEMND_266, "aclk_imemnd_266", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 1, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_GIC, "aclk_gic", "ac
Re: [PATCH 4/4] clk: samsung: exynos5433: add imem clock
Hi,
Thank you for your review, see below for answers and questions.
On 21.11.2018 13:39, Chanwoo Choi wrote:
> Hi,
>
> On 2018년 11월 21일 21:05, Kamil Konieczny wrote:
>> Add imem clock for exynos5433.
>
> It is diffcult to understand the meaning of 'imem' without the description.
> Please add more detailed description as the patch2 description.
>
Will this be enough for description:
Add imem clock for exynos5433. This will enable to use crypto Security
SubSystem (in short SSS) and SlimSSS IP blocks.
What do you think ?
>> Signed-off-by: Kamil Konieczny
>> ---
>> drivers/clk/samsung/clk-exynos5433.c | 123 +
>> include/dt-bindings/clock/exynos5433.h | 55 +++
>> 2 files changed, 178 insertions(+)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5433.c
>> b/drivers/clk/samsung/clk-exynos5433.c
>> index 24c3360db65b..db29cbd1fbdc 100644
>> --- a/drivers/clk/samsung/clk-exynos5433.c
>> +++ b/drivers/clk/samsung/clk-exynos5433.c
>> @@ -2345,6 +2345,129 @@ static const struct samsung_cmu_info fsys_cmu_info
>> __initconst = {
>> .clk_name = "aclk_fsys_200",
>> };
>>
>> +/*
>> + * Register offset definitions for CMU_IMEM
>> + *
>> + */
>> +
>
> Remove unneeded blank line.
ok
>
>> +#define ENABLE_ACLK_IMEM0x0800
>> +#define ENABLE_ACLK_IMEM_SSS0x0808
>> +#define ENABLE_ACLK_IMEM_SLIMSSS0x080c
>> +#define ENABLE_PCLK_IMEM0x0900
>> +#define ENABLE_PCLK_IMEM_SSS0x0904
>> +#define ENABLE_PCLK_IMEM_SLIMSSS0x0908
>
> When I checked the registers of IMEM block, there are more registers as
> following:
> Why do you implement the clocks for only six registers?
I added only those that will be used by crypto block (aes and hash block).
I can add all of them in version 2.
> CLK_ENABLE_ACLK_IMEM 0x0800
>
> CLK_ENABLE_ACLK_IMEM_SECURE_INT_MEM 0x0804
>
> CLK_ENABLE_ACLK_IMEM_SECURE_SSS 0x0808
>
> CLK_ENABLE_ACLK_IMEM_SECURE_SLIMSSS 0x080C
>
> CLK_ENABLE_ACLK_IMEM_SECURE_RTIC 0x0810
>
> CLK_ENABLE_ACLK_IMEM_SECURE_SMMU_SSS 0x0814
> [...]
>
>> +
>> +static const unsigned long imem_clk_regs[] __initconst = {
>> +ENABLE_ACLK_IMEM,
>> +ENABLE_ACLK_IMEM_SSS,
>> +ENABLE_ACLK_IMEM_SLIMSSS,
>> +ENABLE_PCLK_IMEM,
>> +ENABLE_PCLK_IMEM_SSS,
>> +ENABLE_PCLK_IMEM_SLIMSSS,
>
> Add a tab in front of registers.
>
ok
>> [...]
>> +static void __init exynos5433_cmu_imem_init(struct device_node *np)
>> +{
>> +samsung_cmu_register_one(np, &imem_cmu_info);
>> +}
>> +
>> +CLK_OF_DECLARE(exynos5433_cmu_imem, "samsung,exynos5433-cmu-imem",
>> +exynos5433_cmu_imem_init);
>
> Except for "samsmung,exynos5433-cmu-atlas/apollo", the remained clock blocks
> were added to exynos5433_cmu_of_match[] table for power management.
>
> If there is no any h/w issue, add the new entry to exynos5433_cmu_of_match
> for imem block.
ok, I will add this.
>> [...]
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
[PATCH v2 5/5] clk: samsung: exynos5433: add imem clock
Add imem clock for exynos5433. This will enable to use crypto Security
SubSystem (in short SSS) and SlimSSS IP blocks.
Signed-off-by: Kamil Konieczny
---
drivers/clk/samsung/clk-exynos5433.c | 189 +
include/dt-bindings/clock/exynos5433.h | 55 +++
2 files changed, 244 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5433.c
b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..ab03126f350b 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -2345,6 +2345,192 @@ static const struct samsung_cmu_info fsys_cmu_info
__initconst = {
.clk_name = "aclk_fsys_200",
};
+/*
+ * Register offset definitions for CMU_IMEM
+ *
+ */
+#define ENABLE_ACLK_IMEM 0x0800
+#define ENABLE_ACLK_IMEM_INT_MEM 0x0804
+#define ENABLE_ACLK_IMEM_SSS 0x0808
+#define ENABLE_ACLK_IMEM_SLIMSSS 0x080c
+#define ENABLE_ACLK_IMEM_RTIC 0x0810
+#define ENABLE_ACLK_IMEM_SMMU_SSS 0x0814
+#define ENABLE_ACLK_IMEM_SMMU_SLIMSSS 0x0818
+#define ENABLE_ACLK_IMEM_SMMU_RTIC 0x081c
+#define ENABLE_ACLK_IMEM_ARBG_TX 0x0820
+#define ENABLE_ACLK_IMEM_SMMU_ARBG_TX 0x0824
+#define ENABLE_PCLK_IMEM 0x0900
+#define ENABLE_PCLK_IMEM_SSS 0x0904
+#define ENABLE_PCLK_IMEM_SLIMSSS 0x0908
+#define ENABLE_PCLK_IMEM_RTIC 0x090c
+#define ENABLE_PCLK_IMEM_SMMU_SSS 0x0910
+#define ENABLE_PCLK_IMEM_SMMU_SLIMSSS 0x0914
+#define ENABLE_PCLK_IMEM_SMMU_RTIC 0x0918
+#define ENABLE_PCLK_IMEM_SMMU_ARGB_TX 0x091c
+
+static const unsigned long imem_clk_regs[] __initconst = {
+ ENABLE_ACLK_IMEM,
+ ENABLE_ACLK_IMEM_INT_MEM,
+ ENABLE_ACLK_IMEM_SSS,
+ ENABLE_ACLK_IMEM_SLIMSSS,
+ ENABLE_ACLK_IMEM_RTIC,
+ ENABLE_ACLK_IMEM_SMMU_SSS,
+ ENABLE_ACLK_IMEM_SMMU_SLIMSSS,
+ ENABLE_ACLK_IMEM_SMMU_RTIC,
+ ENABLE_ACLK_IMEM_ARBG_TX,
+ ENABLE_ACLK_IMEM_SMMU_ARBG_TX,
+ ENABLE_PCLK_IMEM,
+ ENABLE_PCLK_IMEM_SSS,
+ ENABLE_PCLK_IMEM_SLIMSSS,
+ ENABLE_PCLK_IMEM_RTIC,
+ ENABLE_PCLK_IMEM_SMMU_SSS,
+ ENABLE_PCLK_IMEM_SMMU_SLIMSSS,
+ ENABLE_PCLK_IMEM_SMMU_RTIC,
+ ENABLE_PCLK_IMEM_SMMU_ARGB_TX,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+ /* ENABLE_ACLK_IMEM */
+ GATE(CLK_ACLK_AXI2AHB_IMEMH, "aclk_axi2ahb_imemh", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 24, 0, 0),
+ GATE(CLK_ACLK_AXIDS_SROMC, "aclk_axids_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 23, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_SROMC, "aclk_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 22, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_ARBG_TX, "aclk_bts_arbg_tx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 21, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_IMEMX, "aclk_asyncaxi_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 20, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIUS_IMEMX, "aclk_axius_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 19, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_XIU_IMEMX, "aclk_xiu_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 18, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_SSSX, "aclk_asyncaxi_sssx", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 17, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SLIMSSS, "aclk_bts_slimsss", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 15, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_DRAM, "aclk_bts_sss_dram", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 14, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_CCI, "aclk_bts_sss_cci", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 13, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ALB_IMEM, "aclk_alb_imem", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 12, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM1P, "aclk_axids_pimemx_imem1p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 11, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM0P, "aclk_axids_pimemx_imem0p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 10, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_GIC, "aclk_axids_pimemx_gic",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 9, CLK_IGNORE_UNUSED, 0),
+
[PATCH v3 5/5] clk: samsung: exynos5433: add imem clocks
Add imem clocks for exynos5433. This will enable to use crypto Security
SubSystem (in short SSS) and SlimSSS IP blocks.
Signed-off-by: Kamil Konieczny
---
drivers/clk/samsung/clk-exynos5433.c | 193 +
include/dt-bindings/clock/exynos5433.h | 55 +++
2 files changed, 248 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5433.c
b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..7f557d6f41f0 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5467,6 +5467,196 @@ static const struct samsung_cmu_info cam1_cmu_info
__initconst = {
.clk_name = "aclk_cam1_400",
};
+/*
+ * Register offset definitions for CMU_IMEM
+ */
+#define ENABLE_ACLK_IMEM 0x0800
+#define ENABLE_ACLK_IMEM_INT_MEM 0x0804
+#define ENABLE_ACLK_IMEM_SSS 0x0808
+#define ENABLE_ACLK_IMEM_SLIMSSS 0x080c
+#define ENABLE_ACLK_IMEM_RTIC 0x0810
+#define ENABLE_ACLK_IMEM_SMMU_SSS 0x0814
+#define ENABLE_ACLK_IMEM_SMMU_SLIMSSS 0x0818
+#define ENABLE_ACLK_IMEM_SMMU_RTIC 0x081c
+#define ENABLE_ACLK_IMEM_ARBG_TX 0x0820
+#define ENABLE_ACLK_IMEM_SMMU_ARBG_TX 0x0824
+#define ENABLE_PCLK_IMEM 0x0900
+#define ENABLE_PCLK_IMEM_SSS 0x0904
+#define ENABLE_PCLK_IMEM_SLIMSSS 0x0908
+#define ENABLE_PCLK_IMEM_RTIC 0x090c
+#define ENABLE_PCLK_IMEM_SMMU_SSS 0x0910
+#define ENABLE_PCLK_IMEM_SMMU_SLIMSSS 0x0914
+#define ENABLE_PCLK_IMEM_SMMU_RTIC 0x0918
+#define ENABLE_PCLK_IMEM_SMMU_ARGB_TX 0x091c
+
+static const unsigned long imem_clk_regs[] __initconst = {
+ ENABLE_ACLK_IMEM,
+ ENABLE_ACLK_IMEM_INT_MEM,
+ ENABLE_ACLK_IMEM_SSS,
+ ENABLE_ACLK_IMEM_SLIMSSS,
+ ENABLE_ACLK_IMEM_RTIC,
+ ENABLE_ACLK_IMEM_SMMU_SSS,
+ ENABLE_ACLK_IMEM_SMMU_SLIMSSS,
+ ENABLE_ACLK_IMEM_SMMU_RTIC,
+ ENABLE_ACLK_IMEM_ARBG_TX,
+ ENABLE_ACLK_IMEM_SMMU_ARBG_TX,
+ ENABLE_PCLK_IMEM,
+ ENABLE_PCLK_IMEM_SSS,
+ ENABLE_PCLK_IMEM_SLIMSSS,
+ ENABLE_PCLK_IMEM_RTIC,
+ ENABLE_PCLK_IMEM_SMMU_SSS,
+ ENABLE_PCLK_IMEM_SMMU_SLIMSSS,
+ ENABLE_PCLK_IMEM_SMMU_RTIC,
+ ENABLE_PCLK_IMEM_SMMU_ARGB_TX,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+ /* ENABLE_ACLK_IMEM */
+ GATE(CLK_ACLK_AXI2AHB_IMEMH, "aclk_axi2ahb_imemh", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 24, 0, 0),
+ GATE(CLK_ACLK_AXIDS_SROMC, "aclk_axids_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 23, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_SROMC, "aclk_sromc", "aclk_imem_200",
+ ENABLE_ACLK_IMEM, 22, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_ARBG_TX, "aclk_bts_arbg_tx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 21, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_IMEMX, "aclk_asyncaxi_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 20, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIUS_IMEMX, "aclk_axius_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 19, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_XIU_IMEMX, "aclk_xiu_imemx", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 18, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ASYNCAXI_SSSX, "aclk_asyncaxi_sssx", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 17, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SLIMSSS, "aclk_bts_slimsss", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 15, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_DRAM, "aclk_bts_sss_dram", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 14, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_BTS_SSS_CCI, "aclk_bts_sss_cci", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM, 13, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_ALB_IMEM, "aclk_alb_imem", "aclk_imem_266",
+ ENABLE_ACLK_IMEM, 12, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM1P, "aclk_axids_pimemx_imem1p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 11, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_IMEM0P, "aclk_axids_pimemx_imem0p",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 10, CLK_IGNORE_UNUSED, 0),
+ GATE(CLK_ACLK_AXIDS_PIMEMX_GIC, "aclk_axids_pimemx_gic",
"aclk_imem_200",
+ ENABLE_ACLK_IMEM, 9, CLK_IGNORE_UNUSED, 0),
+
[PATCH] crypto: s5p-sss - fix AES support for Exynos5433
Commit 0918f18c7179 ("crypto: s5p - add AES support for Exynos5433")
introduced bug in dereferencing clk_names[1] on platforms different from
Exynos5433. On Exynos board XU3 call trace is:
"Unable to handle kernel paging request at virtual address 4000"
(strcmp) from [] (of_property_match_string+0x58/0xd0)
...
(devm_clk_get) from [] (s5p_aes_probe+0xd4/0x4a0)
(s5p_aes_probe) from [] (platform_drv_probe+0x6c/0xa4)
Fix this by setting array clk_names size to 2.
Fixes: 0918f18c7179 ("crypto: s5p - add AES support for Exynos5433")
Reported-by: Krzysztof Kozlowski
Signed-off-by: Kamil Konieczny
---
drivers/crypto/s5p-sss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 3f45cc5cb94a..2fb4ae459c45 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -241,7 +241,7 @@
struct samsung_aes_variant {
unsigned intaes_offset;
unsigned inthash_offset;
- const char *clk_names[];
+ const char *clk_names[2];
};
struct s5p_aes_reqctx {
--
2.21.0
Re: [PATCH v2 1/2] hwrng: iproc-rng200 - Set the quality value
Hi, On 19.05.2020 23:25, Łukasz Stelmach wrote: > The value was estimaded with ea_iid[1] using on 10485760 bytes read from > the RNG via /dev/hwrng. The min-entropy value calculated using the most > common value estimate (NIST SP 800-90P[2], section 6.3.1) was 7.964464. > > [1] > https://protect2.fireeye.com/url?k=316f3d79-6cf9840e-316eb636-0cc47a312ab0-5f119f729b3ddc11&q=1&u=https%3A%2F%2Fgithub.com%2Fusnistgov%2FSP800-90B_EntropyAssessment This link looks ugly and do not protect anything. Can you just cut out that "protect2" thing and put proper direct link to github ? > [2] https://csrc.nist.gov/publications/detail/sp/800-90b/final > > Signed-off-by: Łukasz Stelmach > --- > drivers/char/hw_random/iproc-rng200.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/hw_random/iproc-rng200.c > b/drivers/char/hw_random/iproc-rng200.c > index 32d9fe61a225..95669ece050f 100644 > --- a/drivers/char/hw_random/iproc-rng200.c > +++ b/drivers/char/hw_random/iproc-rng200.c > @@ -199,6 +199,7 @@ static int iproc_rng200_probe(struct platform_device > *pdev) > priv->rng.read = iproc_rng200_read, > priv->rng.init = iproc_rng200_init, > priv->rng.cleanup = iproc_rng200_cleanup, > + priv->rng.quality = 1000, > > /* Register driver */ > ret = devm_hwrng_register(dev, &priv->rng); > -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [RESEND PATCH v5 0/4] add coupled regulators for Exynos5422/5800
Dear MyungJoo, On 08.08.2019 11:02, Kamil Konieczny wrote: > Hi, > > The main purpose of this patch series is to add coupled regulators for > Exynos5422/5800 to keep constrain on voltage difference between vdd_arm > and vdd_int to be at most 300mV. In exynos-bus instead of using > regulator_set_voltage_tol() with default voltage tolerance it should be > used regulator_set_voltage_triplet() with volatege range, and this is > already present in opp/core.c code, so it can be reused. While at this, > move setting regulators into opp/core. > > This patchset was tested on Odroid XU3. > > The DTS coupled regulators patch depends on previous patches. > > Changes: > v5: > - squashed last patch "remove exynos_bus_passive_target()" into second > - added Acked-by to patch "correct clock enable sequence" Could you please merge patches 1/4 and 2/4 for v5.4 (they were already Ack-ed by Chanwoo Choi)? -- Best regards, Kamil Konieczny Samsung R&D Institute Poland > v4: > - removed "opp: core: add regulators enable and disable" from patchset > as it was applied by Viresh Kumar and changed cover letter > - fix patch "devfreq: exynos-bus: correct clock enable sequence" to > correct order of enable/disable > - removed unrelated changes in "devfreq: exynos-bus: convert to use > dev_pm_opp_set_rate()" > - added new patch "devfreq: exynos-bus: remove exynos_bus_passive_target()" > as suggested by Chanwoo Choi > v3: > - added new exynos-bus patch to correct clock and regulator enabling > and disabling sequence as suggested by Chanwoo Choi > - corrected error path in enable and improved commit message in opp/core > - improve comment in devfreq/exynos-bus.c before devfreq_recommended_opp() > - change cover letter as there is new patch > - added note before Signed-off-by in 4th patch > v2: > - improve regulators enable/disable code in opp/core as suggested by > Viresh Kumar > - add new patch for remove unused dt-bindings as suggested by Krzysztof > Kozlowski > > Kamil Konieczny (3): > devfreq: exynos-bus: correct clock enable sequence > devfreq: exynos-bus: convert to use dev_pm_opp_set_rate() > dt-bindings: devfreq: exynos-bus: remove unused property > > Marek Szyprowski (1): > ARM: dts: exynos: add initial data for coupled regulators for > Exynos5422/5800 > > .../bindings/devfreq/exynos-bus.txt | 2 - > arch/arm/boot/dts/exynos5420.dtsi | 34 ++-- > arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 + > arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 + > arch/arm/boot/dts/exynos5800.dtsi | 32 ++-- > drivers/devfreq/exynos-bus.c | 153 +- > 6 files changed, 78 insertions(+), 151 deletions(-) >
Re: Devfreq looks for unsupported frequencies on Exynos
On 04.10.2019 12:31, Krzysztof Kozlowski wrote: > > Hi All, > > I noticed recently error logs when booting Odroid HC1 (Exynos5422) on > exynos_defconfig, 5.4.0-rc1-next-20191004: > [...] > [5.225222] exynos-bus soc:bus_wcore: dev_pm_opp_set_rate: failed to find > current OPP for freq 53200 (-34) > [5.225975] exynos-bus: new bus device registered: soc:bus_peri ( 67000 > KHz ~ 67000 KHz) > [5.238824] exynos-bus soc:bus_noc: dev_pm_opp_set_rate: failed to find > current OPP for freq 11100 (-34) > [5.245719] exynos-bus: new bus device registered: soc:bus_g2d ( 84000 KHz > ~ 333000 KHz) > [5.252653] exynos-bus soc:bus_fsys_apb: dev_pm_opp_set_rate: failed to > find current OPP for freq 22200 (-34) > [...] > You can see that Exynos devfreq was trying to set 53200 Hz for > wcore, 11100 for noc and 22200 for fsys_apb. These are higher > frequencies than we have in DTSI. > > Any ideas why? It does not look normal... Thank you for report. This is known issue, clocks left by u-boot are too high and they are not present in opp tables. Lukasz Luba sended patchset "[PATCH v1 00/50] Exynos5x clocks and buses changes" but it should first be made in u-boot. I have idea for a fix and will send it soon after weekend. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH v2 1/4] opp: core: add regulators enable and disable
On 16.07.2019 06:03, Chanwoo Choi wrote:
> Hi Kamil,
>
> On 19. 7. 15. 오후 9:04, Kamil Konieczny wrote:
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>
> IMHO, it is not proper to mention the specific driver name.
> If you explain the reason why enable the regulator before using it,
> it is enough description.
>
>>
>> Signed-off-by: Kamil Konieczny
>> --
>> Changes in v2:
>>
>> - move regulator enable and disable into loop
>>
>> ---
>> drivers/opp/core.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..069c5cf8827e 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>> goto free_regulators;
>> }
>>
>> +ret = regulator_enable(reg);
>> +if (ret < 0)
>> +goto disable;
>> +
>> opp_table->regulators[i] = reg;
>> }
>>
>> @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>>
>> return opp_table;
>>
>> +disable:
>> +regulator_put(reg);
>> +--i;
>> +
>> free_regulators:
>> -while (i != 0)
>> -regulator_put(opp_table->regulators[--i]);
>> +for (; i >= 0; --i) {
>> +regulator_disable(opp_table->regulators[i]);
>> +regulator_put(opp_table->regulators[i]);
>> +}
>>
>> kfree(opp_table->regulators);
>> opp_table->regulators = NULL;
>> @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table
>> *opp_table)
>> /* Make sure there are no concurrent readers while updating opp_table */
>> WARN_ON(!list_empty(&opp_table->opp_list));
>>
>> -for (i = opp_table->regulator_count - 1; i >= 0; i--)
>> +for (i = opp_table->regulator_count - 1; i >= 0; i--) {
>> +regulator_disable(opp_table->regulators[i]);
>> regulator_put(opp_table->regulators[i]);
>> +}
>>
>> _free_set_opp_data(opp_table);
>>
>>
>
> I agree to enable the regulator before using it.
> The bootloader might not enable the regulators
> and the kernel need to enable regulator in order to increase
> the reference count explicitly event if bootloader enables it.
>
> Reviewed-by: Chanwoo Choi
Thank you, I will change commit description and send v3.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH v2 1/4] opp: core: add regulators enable and disable
On 16.07.2019 12:05, Viresh Kumar wrote:
> On 15-07-19, 14:04, Kamil Konieczny wrote:
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny
>> --
>> Changes in v2:
>>
>> - move regulator enable and disable into loop
>>
>> ---
>> drivers/opp/core.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..069c5cf8827e 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>> goto free_regulators;
>> }
>>
>> +ret = regulator_enable(reg);
>> +if (ret < 0)
>> +goto disable;
>
> The name of this label is logically incorrect because we won't disable
> the regulator from there but put it. Over that, I would rather prefer
> to remove the label and add regulator_put() here itself.
I will change this and following according to your suggestions and will send v3.
>> +
>> opp_table->regulators[i] = reg;
>> }
>>
>> @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>>
>> return opp_table;
>>
>> +disable:
>> +regulator_put(reg);
>> +--i;
>> +
>> free_regulators:
>> -while (i != 0)
>> -regulator_put(opp_table->regulators[--i]);
>> +for (; i >= 0; --i) {
>> +regulator_disable(opp_table->regulators[i]);
>> +regulator_put(opp_table->regulators[i]);
>
> This is incorrect as this will now try to put/disable the regulator
> which we failed to acquire. As --i happens only after the loop has run
> once. You can rather do:
>
> while (i--) {
> regulator_disable(opp_table->regulators[i]);
> regulator_put(opp_table->regulators[i]);
> }
>
>
>> +}
>>
>> kfree(opp_table->regulators);
>> opp_table->regulators = NULL;
>> @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table
>> *opp_table)
>> /* Make sure there are no concurrent readers while updating opp_table */
>> WARN_ON(!list_empty(&opp_table->opp_list));
>>
>> -for (i = opp_table->regulator_count - 1; i >= 0; i--)
>> +for (i = opp_table->regulator_count - 1; i >= 0; i--) {
>> +regulator_disable(opp_table->regulators[i]);
>> regulator_put(opp_table->regulators[i]);
>> +}
>>
>> _free_set_opp_data(opp_table);
>>
>> --
>> 2.22.0
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
[PATCH v2] crypto: s5p: update iv after AES-CBC op end
Fix bug "s5p-sss crypto driver doesn't set next AES-CBC IV". While at this,
fix also AES-CTR mode. Tested on Odroid U3 with Eric Biggers branch
"iv-out-testing".
Signed-off-by: Kamil Konieczny
Reported-by: Eric Biggers
---
Changes since v1:
- reworded Subject and commit message
- changed code according to comments by Krzysztof Kozlowski
- added Reported-by line
---
drivers/crypto/s5p-sss.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..30b4f1015d8f 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -463,6 +463,9 @@ static void s5p_sg_copy_buf(void *buf, struct scatterlist
*sg,
static void s5p_sg_done(struct s5p_aes_dev *dev)
{
+ struct ablkcipher_request *req = dev->req;
+ struct s5p_aes_reqctx *reqctx = ablkcipher_request_ctx(req);
+
if (dev->sg_dst_cpy) {
dev_dbg(dev->dev,
"Copying %d bytes of output data back to original
place\n",
@@ -472,6 +475,11 @@ static void s5p_sg_done(struct s5p_aes_dev *dev)
}
s5p_free_sg_cpy(dev, &dev->sg_src_cpy);
s5p_free_sg_cpy(dev, &dev->sg_dst_cpy);
+ if (reqctx->mode & FLAGS_AES_CBC)
+ memcpy_fromio(req->info, dev->aes_ioaddr +
SSS_REG_AES_IV_DATA(0), AES_BLOCK_SIZE);
+
+ else if (reqctx->mode & FLAGS_AES_CTR)
+ memcpy_fromio(req->info, dev->aes_ioaddr +
SSS_REG_AES_CNT_DATA(0), AES_BLOCK_SIZE);
}
/* Calls the completion. Cannot be called with dev->lock hold. */
--
2.20.1
[PATCH v5 5/5] clk: samsung: exynos5433: add imem clocks
Add imem clocks for exynos5433. This will enable to use crypto Slim
Security SubSystem (in short SlimSSS) IP block.
Acked-by: Chanwoo Choi
Signed-off-by: Kamil Konieczny
---
drivers/clk/samsung/clk-exynos5433.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5433.c
b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..dae1c96de933 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5467,6 +5467,35 @@ static const struct samsung_cmu_info cam1_cmu_info
__initconst = {
.clk_name = "aclk_cam1_400",
};
+/*
+ * Register offset definitions for CMU_IMEM
+ */
+#define ENABLE_ACLK_IMEM_SLIMSSS 0x080c
+#define ENABLE_PCLK_IMEM_SLIMSSS 0x0908
+
+static const unsigned long imem_clk_regs[] __initconst = {
+ ENABLE_ACLK_IMEM_SLIMSSS,
+ ENABLE_PCLK_IMEM_SLIMSSS,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+ /* ENABLE_ACLK_IMEM_SLIMSSS */
+ GATE(CLK_ACLK_SLIMSSS, "aclk_slimsss", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+
+ /* ENABLE_PCLK_IMEM_SLIMSSS */
+ GATE(CLK_PCLK_SLIMSSS, "pclk_slimsss", "aclk_imem_200",
+ ENABLE_PCLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+};
+
+static const struct samsung_cmu_info imem_cmu_info __initconst = {
+ .gate_clks = imem_gate_clks,
+ .nr_gate_clks = ARRAY_SIZE(imem_gate_clks),
+ .nr_clk_ids = IMEM_NR_CLK,
+ .clk_regs = imem_clk_regs,
+ .nr_clk_regs= ARRAY_SIZE(imem_clk_regs),
+ .clk_name = "aclk_imem_200",
+};
struct exynos5433_cmu_data {
struct samsung_clk_reg_dump *clk_save;
@@ -5654,6 +5683,9 @@ static const struct of_device_id
exynos5433_cmu_of_match[] = {
}, {
.compatible = "samsung,exynos5433-cmu-mscl",
.data = &mscl_cmu_info,
+ }, {
+ .compatible = "samsung,exynos5433-cmu-imem",
+ .data = &imem_cmu_info,
}, {
},
};
--
2.20.0
[PATCH v1 0/3] add AES support for Exynos5433
Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on Exynos5433 board with crypto run-time self tests and with tcrypt with command insmod tcrypt.ko mode=500 sec=1 Kamil Konieczny (3): arm64: dts: exynos: add SlimSSS for Exynos5433 dt-bindings: crypto: document Exynos5433 SlimSSS crypto: s5p: add AES support for Exynos5433 .../bindings/crypto/samsung-sss.txt | 13 +++-- arch/arm64/boot/dts/exynos/exynos5433.dtsi| 9 drivers/crypto/s5p-sss.c | 50 +-- 3 files changed, 65 insertions(+), 7 deletions(-) -- 2.20.0
[PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS
Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security SubSystem) IP. Signed-off-by: Kamil Konieczny --- .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt index 7a5ca56683cc..75718f86fe50 100644 --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt @@ -1,4 +1,4 @@ -Samsung SoC SSS (Security SubSystem) module +Samsung SoC SSS (Security SubSystem) and SlimSSS module The SSS module in S5PV210 SoC supports the following: -- Feeder (FeedCtrl) @@ -15,6 +15,11 @@ supports the following also: -- True Random Number Generator (TRNG) -- Secure Key Manager +The SlimSSS module in Exynos5 (Exynos5433) supports: +-- Feeder (FeedCtrl) +-- Advanced Encryption Standard (AES) +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256) + Required properties: - compatible : Should contain entries for this and backward compatible @@ -22,11 +27,13 @@ Required properties: - "samsung,s5pv210-secss" for S5PV210 SoC. - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250, Exynos5260 and Exynos5420 SoCs. + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs. - reg : Offset and length of the register set for the module - interrupts : interrupt specifiers of SSS module interrupts (one feed control interrupt). - clocks : list of clock phandle and specifier pairs for all clocks listed in clock-names property. -- clock-names : list of device clock input names; should contain one entry - "secss". +- clock-names : list of device clock input names; should contain "pclk" and + "aclk" for slim-sss in Exynos5433, and one entry "secss" for + other compatibles. -- 2.20.0
[PATCH v1 1/3] arm64: dts: exynos: add SlimSSS for Exynos5433
Add DT node for SlimSSS (aka Slim SecuritySubSystem) in Exynos5433 SoC.
The users can use compatibility "samsung,exynos5433-slim-sss".
Signed-off-by: Kamil Konieczny
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index e7cd3b67d818..9e5fcb6c7776 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -559,6 +559,15 @@
<&cmu_top CLK_DIV_ACLK_IMEM_200>;
};
+ slim_sss: slim-sss@1114 {
+ compatible = "samsung,exynos5433-slim-sss";
+ reg = <0x1114 0x1000>;
+ interrupts = ;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_imem CLK_ACLK_SLIMSSS>,
+<&cmu_imem CLK_PCLK_SLIMSSS>;
+ };
+
pd_gscl: power-domain@105c4000 {
compatible = "samsung,exynos5433-pd";
reg = <0x105c4000 0x20>;
--
2.20.0
[PATCH v1 3/3] crypto: s5p: add AES support for Exynos5433
Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/s5p-sss.c | 50
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..1ae570ad4391 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -232,6 +232,7 @@
* struct samsung_aes_variant - platform specific SSS driver data
* @aes_offset: AES register offset from SSS module's base.
* @hash_offset: HASH register offset from SSS module's base.
+ * @clk_names: names of clocks needed to run SSS IP
*
* Specifies platform specific configuration of SSS module.
* Note: A structure for driver specific platform data is used for future
@@ -240,6 +241,7 @@
struct samsung_aes_variant {
unsigned intaes_offset;
unsigned inthash_offset;
+ char*clk_names[];
};
struct s5p_aes_reqctx {
@@ -296,6 +298,7 @@ struct s5p_aes_ctx {
struct s5p_aes_dev {
struct device *dev;
struct clk *clk;
+ struct clk *pclk;
void __iomem*ioaddr;
void __iomem*aes_ioaddr;
int irq_fc;
@@ -384,11 +387,19 @@ struct s5p_hash_ctx {
static const struct samsung_aes_variant s5p_aes_data = {
.aes_offset = 0x4000,
.hash_offset= 0x6000,
+ .clk_names = { "secss", }
};
static const struct samsung_aes_variant exynos_aes_data = {
.aes_offset = 0x200,
.hash_offset= 0x400,
+ .clk_names = { "secss", }
+};
+
+static const struct samsung_aes_variant exynos5433_slim_aes_data = {
+ .aes_offset = 0x400,
+ .hash_offset= 0x800,
+ .clk_names = { "pclk", "aclk", }
};
static const struct of_device_id s5p_sss_dt_match[] = {
@@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
.compatible = "samsung,exynos4210-secss",
.data = &exynos_aes_data,
},
+ {
+ .compatible = "samsung,exynos5433-slim-sss",
+ .data = &exynos5433_slim_aes_data,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
@@ -2208,18 +2223,40 @@ static int s5p_aes_probe(struct platform_device *pdev)
return PTR_ERR(pdata->ioaddr);
}
- pdata->clk = devm_clk_get(dev, "secss");
+ pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
if (IS_ERR(pdata->clk)) {
- dev_err(dev, "failed to find secss clock source\n");
+ dev_err(dev, "failed to find secss clock %s\n",
+ variant->clk_names[0]);
return -ENOENT;
}
err = clk_prepare_enable(pdata->clk);
if (err < 0) {
- dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
return err;
}
+ if (variant->clk_names[1]) {
+ pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
+ if (IS_ERR(pdata->pclk)) {
+ dev_err(dev, "failed to find clock %s\n",
+ variant->clk_names[1]);
+ clk_disable_unprepare(pdata->clk);
+ return -ENOENT;
+ }
+
+ err = clk_prepare_enable(pdata->pclk);
+ if (err < 0) {
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
+ clk_disable_unprepare(pdata->clk);
+ return err;
+ }
+ } else {
+ pdata->pclk = NULL;
+ }
+
spin_lock_init(&pdata->lock);
spin_lock_init(&pdata->hash_lock);
@@ -2295,8 +2332,10 @@ static int s5p_aes_probe(struct platform_device *pdev)
tasklet_kill(&pdata->tasklet);
err_irq:
- clk_disable_unprepare(pdata->clk);
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+ clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
return err;
@@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
pdata->use_hash = false;
}
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+
clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
--
2.20.0
[PATCH v4 5/5] clk: samsung: exynos5433: add imem clocks
Add imem clocks for exynos5433. This will enable to use crypto Slim
Security SubSystem (in short SlimSSS) IP block.
Signed-off-by: Kamil Konieczny
---
drivers/clk/samsung/clk-exynos5433.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/clk/samsung/clk-exynos5433.c
b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..dae1c96de933 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5467,6 +5467,35 @@ static const struct samsung_cmu_info cam1_cmu_info
__initconst = {
.clk_name = "aclk_cam1_400",
};
+/*
+ * Register offset definitions for CMU_IMEM
+ */
+#define ENABLE_ACLK_IMEM_SLIMSSS 0x080c
+#define ENABLE_PCLK_IMEM_SLIMSSS 0x0908
+
+static const unsigned long imem_clk_regs[] __initconst = {
+ ENABLE_ACLK_IMEM_SLIMSSS,
+ ENABLE_PCLK_IMEM_SLIMSSS,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+ /* ENABLE_ACLK_IMEM_SLIMSSS */
+ GATE(CLK_ACLK_SLIMSSS, "aclk_slimsss", "aclk_imem_sssx_266",
+ ENABLE_ACLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+
+ /* ENABLE_PCLK_IMEM_SLIMSSS */
+ GATE(CLK_PCLK_SLIMSSS, "pclk_slimsss", "aclk_imem_200",
+ ENABLE_PCLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+};
+
+static const struct samsung_cmu_info imem_cmu_info __initconst = {
+ .gate_clks = imem_gate_clks,
+ .nr_gate_clks = ARRAY_SIZE(imem_gate_clks),
+ .nr_clk_ids = IMEM_NR_CLK,
+ .clk_regs = imem_clk_regs,
+ .nr_clk_regs= ARRAY_SIZE(imem_clk_regs),
+ .clk_name = "aclk_imem_200",
+};
struct exynos5433_cmu_data {
struct samsung_clk_reg_dump *clk_save;
@@ -5654,6 +5683,9 @@ static const struct of_device_id
exynos5433_cmu_of_match[] = {
}, {
.compatible = "samsung,exynos5433-cmu-mscl",
.data = &mscl_cmu_info,
+ }, {
+ .compatible = "samsung,exynos5433-cmu-imem",
+ .data = &imem_cmu_info,
}, {
},
};
--
2.20.0
Re: [PATCH v1 3/3] crypto: s5p: add AES support for Exynos5433
Hi Krzysztof,
On 23.01.2019 09:13, Krzysztof Kozlowski wrote:
> On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
> wrote:
>>
>> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>>
>> Signed-off-by: Kamil Konieczny
>> [...]
>> @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
>> static const struct samsung_aes_variant s5p_aes_data = {
>> .aes_offset = 0x4000,
>> .hash_offset= 0x6000,
>> + .clk_names = { "secss", }
>
> Put trailing comma so it can be extended without touching previous
> line (just like old code).
>
ok, I will do this and two following commas.
>> [...]
>> + if (variant->clk_names[1]) {
>> + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
>> + if (IS_ERR(pdata->pclk)) {
>> + dev_err(dev, "failed to find clock %s\n",
>> + variant->clk_names[1]);
>> + clk_disable_unprepare(pdata->clk);
>> + return -ENOENT;
>
> goto err_irq (eventually rename the label)?
I will add 'goto err_clk' with proper cleanup.
>> + }
>
> Indentation looks wrong here.
ok
>> +
>> + err = clk_prepare_enable(pdata->pclk);
>> + if (err < 0) {
>> + dev_err(dev, "Enabling clock %s failed, err %d\n",
>> + variant->clk_names[0], err);
>> + clk_disable_unprepare(pdata->clk);
>> + return err;
>
> goto...
Yes, I will put the same goto here.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS
Hi Krzysztof, On 23.01.2019 08:59, Krzysztof Kozlowski wrote: > On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny > wrote: >> >> Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security >> SubSystem) IP. >> >> Signed-off-by: Kamil Konieczny >> --- >> .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> b/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> index 7a5ca56683cc..75718f86fe50 100644 >> --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> @@ -1,4 +1,4 @@ >> -Samsung SoC SSS (Security SubSystem) module >> +Samsung SoC SSS (Security SubSystem) and SlimSSS module >> >> The SSS module in S5PV210 SoC supports the following: >> -- Feeder (FeedCtrl) >> @@ -15,6 +15,11 @@ supports the following also: >> -- True Random Number Generator (TRNG) >> -- Secure Key Manager >> >> +The SlimSSS module in Exynos5 (Exynos5433) supports: >> +-- Feeder (FeedCtrl) >> +-- Advanced Encryption Standard (AES) >> +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256) >> + >> Required properties: >> >> - compatible : Should contain entries for this and backward compatible >> @@ -22,11 +27,13 @@ Required properties: >>- "samsung,s5pv210-secss" for S5PV210 SoC. >>- "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, >> Exynos5250, >> Exynos5260 and Exynos5420 SoCs. >> + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs. > > For Exynos5420 there is a driver for regular/full SSS module. It would > be nice to explain what is the difference between them and/or mention > potential conflicts of usage. If Exynos has SlimSSS, it has also SSS. SlimSSS is separated from SSS, it is like SSS limited to only AES and HASH, and HASH got limited in capabilities. In Exynos5420 SSS HASH has SHA-1,SHA-256,md5 with/without HMAC, but slimSSS has only SHA-1,SHA-256/HMAC. SlimSSS AES has all modes present in SSS, these are ECB, CBC, CTR, XTS and CMAC. The same goes for Exynos5433. There are no conflicts between SlimSSS and SSS. > The point is that previously we added drivers/bindings for SSS and now > you use the other interface. It might be pretty confusing for users... SlimSSS has same AES registers offsets from base as in SSS, so the s5p-sss driver can reuse code. It is preferred to use SSS but on Exynos5433 I was unable to read its register (FEEDCTRL) with all clocks enabled, maybe because firmware loaded secureOS (trustzone). >[...] >>[...] -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS
On 23.01.2019 08:59, Krzysztof Kozlowski wrote: > On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny > wrote: >> >> Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security >> SubSystem) IP. >> >> Signed-off-by: Kamil Konieczny >> --- >> .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> b/Documentation/devicetree/bindings/crypto/samsung-sss.txt >> [...] >> - compatible : Should contain entries for this and backward compatible >> @@ -22,11 +27,13 @@ Required properties: >>- "samsung,s5pv210-secss" for S5PV210 SoC. >>- "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, >> Exynos5250, >> Exynos5260 and Exynos5420 SoCs. >> + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs. > > For Exynos5420 there is a driver for regular/full SSS module. It would > be nice to explain what is the difference between them and/or mention > potential conflicts of usage. > > The point is that previously we added drivers/bindings for SSS and now > you use the other interface. It might be pretty confusing for users... By the way, Exynos4 (Exynos4210, 4412) SSS do not have ARCFOUR (ARC4), nor TRNG nor Secure Key Manager. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[PATCH v2 1/3] arm64: dts: exynos: add SlimSSS for Exynos5433
Add DT node for SlimSSS (aka Slim SecuritySubSystem) in Exynos5433 SoC.
The users can use compatibility "samsung,exynos5433-slim-sss".
Signed-off-by: Kamil Konieczny
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index e7cd3b67d818..9e5fcb6c7776 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -559,6 +559,15 @@
<&cmu_top CLK_DIV_ACLK_IMEM_200>;
};
+ slim_sss: slim-sss@1114 {
+ compatible = "samsung,exynos5433-slim-sss";
+ reg = <0x1114 0x1000>;
+ interrupts = ;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_imem CLK_ACLK_SLIMSSS>,
+<&cmu_imem CLK_PCLK_SLIMSSS>;
+ };
+
pd_gscl: power-domain@105c4000 {
compatible = "samsung,exynos5433-pd";
reg = <0x105c4000 0x20>;
--
2.20.0
[PATCH v2 3/3] crypto: s5p: add AES support for Exynos5433
Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
Signed-off-by: Kamil Konieczny
---
drivers/crypto/s5p-sss.c | 50
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..e2d247f59254 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -232,6 +232,7 @@
* struct samsung_aes_variant - platform specific SSS driver data
* @aes_offset: AES register offset from SSS module's base.
* @hash_offset: HASH register offset from SSS module's base.
+ * @clk_names: names of clocks needed to run SSS IP
*
* Specifies platform specific configuration of SSS module.
* Note: A structure for driver specific platform data is used for future
@@ -240,6 +241,7 @@
struct samsung_aes_variant {
unsigned intaes_offset;
unsigned inthash_offset;
+ char*clk_names[];
};
struct s5p_aes_reqctx {
@@ -296,6 +298,7 @@ struct s5p_aes_ctx {
struct s5p_aes_dev {
struct device *dev;
struct clk *clk;
+ struct clk *pclk;
void __iomem*ioaddr;
void __iomem*aes_ioaddr;
int irq_fc;
@@ -384,11 +387,19 @@ struct s5p_hash_ctx {
static const struct samsung_aes_variant s5p_aes_data = {
.aes_offset = 0x4000,
.hash_offset= 0x6000,
+ .clk_names = { "secss", },
};
static const struct samsung_aes_variant exynos_aes_data = {
.aes_offset = 0x200,
.hash_offset= 0x400,
+ .clk_names = { "secss", },
+};
+
+static const struct samsung_aes_variant exynos5433_slim_aes_data = {
+ .aes_offset = 0x400,
+ .hash_offset= 0x800,
+ .clk_names = { "pclk", "aclk", },
};
static const struct of_device_id s5p_sss_dt_match[] = {
@@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
.compatible = "samsung,exynos4210-secss",
.data = &exynos_aes_data,
},
+ {
+ .compatible = "samsung,exynos5433-slim-sss",
+ .data = &exynos5433_slim_aes_data,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
@@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
return PTR_ERR(pdata->ioaddr);
}
- pdata->clk = devm_clk_get(dev, "secss");
+ pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
if (IS_ERR(pdata->clk)) {
- dev_err(dev, "failed to find secss clock source\n");
+ dev_err(dev, "failed to find secss clock %s\n",
+ variant->clk_names[0]);
return -ENOENT;
}
err = clk_prepare_enable(pdata->clk);
if (err < 0) {
- dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
return err;
}
+ if (variant->clk_names[1]) {
+ pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
+ if (IS_ERR(pdata->pclk)) {
+ dev_err(dev, "failed to find clock %s\n",
+ variant->clk_names[1]);
+ err = -ENOENT;
+ goto err_clk;
+ }
+
+ err = clk_prepare_enable(pdata->pclk);
+ if (err < 0) {
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
+ goto err_clk;
+ }
+ } else {
+ pdata->pclk = NULL;
+ }
+
spin_lock_init(&pdata->lock);
spin_lock_init(&pdata->hash_lock);
@@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
tasklet_kill(&pdata->tasklet);
err_irq:
- clk_disable_unprepare(pdata->clk);
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+err_clk:
+ clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
return err;
@@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
pdata->use_hash = false;
}
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+
clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
--
2.20.0
[PATCH v2 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS
Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security SubSystem) IP. Signed-off-by: Kamil Konieczny --- .../devicetree/bindings/crypto/samsung-sss.txt | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt index 7a5ca56683cc..d9af679d38ab 100644 --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt @@ -1,4 +1,4 @@ -Samsung SoC SSS (Security SubSystem) module +Samsung SoC SSS (Security SubSystem) and SlimSSS module The SSS module in S5PV210 SoC supports the following: -- Feeder (FeedCtrl) @@ -15,6 +15,12 @@ supports the following also: -- True Random Number Generator (TRNG) -- Secure Key Manager +Exynos5433 has both SSS and SlimSSS module. +SlimSSS in Exynos5433 supports: +-- Feeder (FeedCtrl) +-- Advanced Encryption Standard (AES) +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256) + Required properties: - compatible : Should contain entries for this and backward compatible @@ -22,11 +28,13 @@ Required properties: - "samsung,s5pv210-secss" for S5PV210 SoC. - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250, Exynos5260 and Exynos5420 SoCs. + - "samsung,exynos5433-slim-sss" for Exynos5433 SoCs. - reg : Offset and length of the register set for the module - interrupts : interrupt specifiers of SSS module interrupts (one feed control interrupt). - clocks : list of clock phandle and specifier pairs for all clocks listed in clock-names property. -- clock-names : list of device clock input names; should contain one entry - "secss". +- clock-names : list of device clock input names; should contain "pclk" and + "aclk" for slim-sss in Exynos5433, and one entry "secss" for + other compatibles. -- 2.20.0
[PATCH v2 0/3] add AES support for Exynos5433
Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on Exynos5433 board with crypto run-time self tests and with tcrypt with command insmod tcrypt.ko mode=500 sec=1 Changes since v1: - address Krzysztof Kozlowski review: add missing comma in struct definition, add goto for error code path, correct indentation, in documentation add that Exynos5433 has both slimSSS and SSS IPs. Kamil Konieczny (3): arm64: dts: exynos: add SlimSSS for Exynos5433 dt-bindings: crypto: document Exynos5433 SlimSSS crypto: s5p: add AES support for Exynos5433 .../bindings/crypto/samsung-sss.txt | 14 -- arch/arm64/boot/dts/exynos/exynos5433.dtsi| 9 drivers/crypto/s5p-sss.c | 50 +-- 3 files changed, 66 insertions(+), 7 deletions(-) -- 2.20.0
Re: [PATCH v2 3/3] crypto: s5p: add AES support for Exynos5433
On 24.01.2019 14:37, Corentin Labbe wrote:
> On Wed, Jan 23, 2019 at 05:55:33PM +0100, Kamil Konieczny wrote:
>> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> drivers/crypto/s5p-sss.c | 50
>> 1 file changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
>> index 0064be0e3941..e2d247f59254 100644
>> [...]
>> @@ -240,6 +241,7 @@
>> struct samsung_aes_variant {
>> unsigned intaes_offset;
>> unsigned inthash_offset;
>> +char*clk_names[];
>
> Hello
>
> this could be set as const
Definitions sets const:
grep "static const" drivers/crypto/s5p-sss.c
static const struct samsung_aes_variant s5p_aes_data = {
static const struct samsung_aes_variant exynos_aes_data = {
static const struct samsung_aes_variant exynos5433_slim_aes_data = {
so it is not needed at struct declaration, or am I missing something ?
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH v2 3/3] crypto: s5p: add AES support for Exynos5433
On 24.01.2019 15:39, Krzysztof Kozlowski wrote:
> On Thu, 24 Jan 2019 at 15:34, Kamil Konieczny
> wrote:
>>
>>
>>
>> On 24.01.2019 14:37, Corentin Labbe wrote:
>>> On Wed, Jan 23, 2019 at 05:55:33PM +0100, Kamil Konieczny wrote:
>>>> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>>>>
>>>> Signed-off-by: Kamil Konieczny
>>>> ---
>>>> drivers/crypto/s5p-sss.c | 50
>>>> 1 file changed, 46 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
>>>> index 0064be0e3941..e2d247f59254 100644
>>>> [...]
>>>> @@ -240,6 +241,7 @@
>>>> struct samsung_aes_variant {
>>>> unsigned intaes_offset;
>>>> unsigned inthash_offset;
>>>> +char*clk_names[];
>>>
>>> Hello
>>>
>>> this could be set as const
>>
>> Definitions sets const:
>>
>> grep "static const" drivers/crypto/s5p-sss.c
>>
>> static const struct samsung_aes_variant s5p_aes_data = {
>> static const struct samsung_aes_variant exynos_aes_data = {
>> static const struct samsung_aes_variant exynos5433_slim_aes_data = {
>>
>> so it is not needed at struct declaration, or am I missing something ?
>
> The struct contains the pointer, so the pointer will be const. However
> the pointer can point to either const string or non-const string.
> That's the Corentin's comment about. So in fact as he says - this
> should be a pointer to a const string.
Thank you for clarifing this, I will send v3 soon.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
[PATCH v3 3/3] crypto: s5p: add AES support for Exynos5433
Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
Reviewed-by: Krzysztof Kozlowski
Signed-off-by: Kamil Konieczny
---
drivers/crypto/s5p-sss.c | 50
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..3f45cc5cb94a 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -232,6 +232,7 @@
* struct samsung_aes_variant - platform specific SSS driver data
* @aes_offset: AES register offset from SSS module's base.
* @hash_offset: HASH register offset from SSS module's base.
+ * @clk_names: names of clocks needed to run SSS IP
*
* Specifies platform specific configuration of SSS module.
* Note: A structure for driver specific platform data is used for future
@@ -240,6 +241,7 @@
struct samsung_aes_variant {
unsigned intaes_offset;
unsigned inthash_offset;
+ const char *clk_names[];
};
struct s5p_aes_reqctx {
@@ -296,6 +298,7 @@ struct s5p_aes_ctx {
struct s5p_aes_dev {
struct device *dev;
struct clk *clk;
+ struct clk *pclk;
void __iomem*ioaddr;
void __iomem*aes_ioaddr;
int irq_fc;
@@ -384,11 +387,19 @@ struct s5p_hash_ctx {
static const struct samsung_aes_variant s5p_aes_data = {
.aes_offset = 0x4000,
.hash_offset= 0x6000,
+ .clk_names = { "secss", },
};
static const struct samsung_aes_variant exynos_aes_data = {
.aes_offset = 0x200,
.hash_offset= 0x400,
+ .clk_names = { "secss", },
+};
+
+static const struct samsung_aes_variant exynos5433_slim_aes_data = {
+ .aes_offset = 0x400,
+ .hash_offset= 0x800,
+ .clk_names = { "pclk", "aclk", },
};
static const struct of_device_id s5p_sss_dt_match[] = {
@@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
.compatible = "samsung,exynos4210-secss",
.data = &exynos_aes_data,
},
+ {
+ .compatible = "samsung,exynos5433-slim-sss",
+ .data = &exynos5433_slim_aes_data,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
@@ -2208,18 +2223,39 @@ static int s5p_aes_probe(struct platform_device *pdev)
return PTR_ERR(pdata->ioaddr);
}
- pdata->clk = devm_clk_get(dev, "secss");
+ pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
if (IS_ERR(pdata->clk)) {
- dev_err(dev, "failed to find secss clock source\n");
+ dev_err(dev, "failed to find secss clock %s\n",
+ variant->clk_names[0]);
return -ENOENT;
}
err = clk_prepare_enable(pdata->clk);
if (err < 0) {
- dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
return err;
}
+ if (variant->clk_names[1]) {
+ pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
+ if (IS_ERR(pdata->pclk)) {
+ dev_err(dev, "failed to find clock %s\n",
+ variant->clk_names[1]);
+ err = -ENOENT;
+ goto err_clk;
+ }
+
+ err = clk_prepare_enable(pdata->pclk);
+ if (err < 0) {
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
+ goto err_clk;
+ }
+ } else {
+ pdata->pclk = NULL;
+ }
+
spin_lock_init(&pdata->lock);
spin_lock_init(&pdata->hash_lock);
@@ -2295,8 +2331,11 @@ static int s5p_aes_probe(struct platform_device *pdev)
tasklet_kill(&pdata->tasklet);
err_irq:
- clk_disable_unprepare(pdata->clk);
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+err_clk:
+ clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
return err;
@@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
pdata->use_hash = false;
}
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+
clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;
--
2.20.0
[PATCH v3 1/3] arm64: dts: exynos: add SlimSSS for Exynos5433
Add DT node for SlimSSS (aka Slim SecuritySubSystem) in Exynos5433 SoC.
The users can use compatibility "samsung,exynos5433-slim-sss".
Signed-off-by: Kamil Konieczny
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index e7cd3b67d818..9e5fcb6c7776 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -559,6 +559,15 @@
<&cmu_top CLK_DIV_ACLK_IMEM_200>;
};
+ slim_sss: slim-sss@1114 {
+ compatible = "samsung,exynos5433-slim-sss";
+ reg = <0x1114 0x1000>;
+ interrupts = ;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_imem CLK_ACLK_SLIMSSS>,
+<&cmu_imem CLK_PCLK_SLIMSSS>;
+ };
+
pd_gscl: power-domain@105c4000 {
compatible = "samsung,exynos5433-pd";
reg = <0x105c4000 0x20>;
--
2.20.0
[PATCH v3 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS
Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security SubSystem) IP. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Kamil Konieczny --- .../devicetree/bindings/crypto/samsung-sss.txt | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt index 7a5ca56683cc..d9af679d38ab 100644 --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt @@ -1,4 +1,4 @@ -Samsung SoC SSS (Security SubSystem) module +Samsung SoC SSS (Security SubSystem) and SlimSSS module The SSS module in S5PV210 SoC supports the following: -- Feeder (FeedCtrl) @@ -15,6 +15,12 @@ supports the following also: -- True Random Number Generator (TRNG) -- Secure Key Manager +Exynos5433 has both SSS and SlimSSS module. +SlimSSS in Exynos5433 supports: +-- Feeder (FeedCtrl) +-- Advanced Encryption Standard (AES) +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256) + Required properties: - compatible : Should contain entries for this and backward compatible @@ -22,11 +28,13 @@ Required properties: - "samsung,s5pv210-secss" for S5PV210 SoC. - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250, Exynos5260 and Exynos5420 SoCs. + - "samsung,exynos5433-slim-sss" for Exynos5433 SoCs. - reg : Offset and length of the register set for the module - interrupts : interrupt specifiers of SSS module interrupts (one feed control interrupt). - clocks : list of clock phandle and specifier pairs for all clocks listed in clock-names property. -- clock-names : list of device clock input names; should contain one entry - "secss". +- clock-names : list of device clock input names; should contain "pclk" and + "aclk" for slim-sss in Exynos5433, and one entry "secss" for + other compatibles. -- 2.20.0
[PATCH v3 0/3] add AES support for Exynos5433
Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on Exynos5433 board with crypto run-time self tests and with tcrypt with command insmod tcrypt.ko mode=500 sec=1 Changes since v2: - address Corentine Labbe note: add "const" to char* in declaration of struct samsung_aes_variant Changes since v1: - address Krzysztof Kozlowski review: add missing comma in struct definition, add goto for error code path, correct indentation, in documentation add that Exynos5433 has both slimSSS and SSS IPs. Kamil Konieczny (3): arm64: dts: exynos: add SlimSSS for Exynos5433 dt-bindings: crypto: document Exynos5433 SlimSSS crypto: s5p: add AES support for Exynos5433 .../bindings/crypto/samsung-sss.txt | 14 -- arch/arm64/boot/dts/exynos/exynos5433.dtsi| 9 drivers/crypto/s5p-sss.c | 50 +-- 3 files changed, 66 insertions(+), 7 deletions(-) -- 2.20.0
[PATCH v2 0/4] add coupled regulators for Exynos5422/5800
Hi, The main purpose of this patch series is to add coupled regulators for Exynos5422/5800 to keep constrain on voltage difference between vdd_arm and vdd_int to be at most 300mV. In exynos-bus instead of using regulator_set_voltage_tol() with default voltage tolerance it should be used regulator_set_voltage_triplet() with volatege range, and this is already present in opp/core.c code, so it can be reused. While at this, move setting regulators into opp/core. This patchset was tested on Odroid XU3. The last patch depends on two previous. Changes in v2: - improve regulators enable/disable code in opp/core as suggested by Viresh Kumar - add new patch for remove unused dt-bindings as suggested by Krzysztof Kozlowski Regards, Kamil Kamil Konieczny (3): opp: core: add regulators enable and disable devfreq: exynos-bus: convert to use dev_pm_opp_set_rate() dt-bindings: devfreq: exynos-bus: remove unused property Marek Szyprowski (1): ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800 .../bindings/devfreq/exynos-bus.txt | 2 - arch/arm/boot/dts/exynos5420.dtsi | 34 ++-- arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 + arch/arm/boot/dts/exynos5800.dtsi | 32 ++-- drivers/devfreq/exynos-bus.c | 172 +++--- drivers/opp/core.c| 18 +- 7 files changed, 122 insertions(+), 144 deletions(-) -- 2.22.0
[PATCH v2 1/4] opp: core: add regulators enable and disable
Add enable regulators to dev_pm_opp_set_regulators() and disable
regulators to dev_pm_opp_put_regulators(). This prepares for
converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
Signed-off-by: Kamil Konieczny
--
Changes in v2:
- move regulator enable and disable into loop
---
drivers/opp/core.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0e7703fe733f..069c5cf8827e 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct
device *dev,
goto free_regulators;
}
+ ret = regulator_enable(reg);
+ if (ret < 0)
+ goto disable;
+
opp_table->regulators[i] = reg;
}
@@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct
device *dev,
return opp_table;
+disable:
+ regulator_put(reg);
+ --i;
+
free_regulators:
- while (i != 0)
- regulator_put(opp_table->regulators[--i]);
+ for (; i >= 0; --i) {
+ regulator_disable(opp_table->regulators[i]);
+ regulator_put(opp_table->regulators[i]);
+ }
kfree(opp_table->regulators);
opp_table->regulators = NULL;
@@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table
*opp_table)
/* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list));
- for (i = opp_table->regulator_count - 1; i >= 0; i--)
+ for (i = opp_table->regulator_count - 1; i >= 0; i--) {
+ regulator_disable(opp_table->regulators[i]);
regulator_put(opp_table->regulators[i]);
+ }
_free_set_opp_data(opp_table);
--
2.22.0
[PATCH v2 3/4] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800
Declare Exynos5422/5800 voltage ranges for opp points for big cpu core and
bus wcore and couple their voltage supllies as vdd_arm and vdd_int should
be in 300mV range.
Signed-off-by: Marek Szyprowski
Signed-off-by: Kamil Konieczny
---
arch/arm/boot/dts/exynos5420.dtsi | 34 +--
arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +++
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +++
arch/arm/boot/dts/exynos5800.dtsi | 32 -
4 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi
b/arch/arm/boot/dts/exynos5420.dtsi
index 5fb2326875dc..0cbf74750553 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -48,62 +48,62 @@
opp-shared;
opp-18 {
opp-hz = /bits/ 64 <18>;
- opp-microvolt = <125>;
+ opp-microvolt = <125 125 150>;
clock-latency-ns = <14>;
};
opp-17 {
opp-hz = /bits/ 64 <17>;
- opp-microvolt = <1212500>;
+ opp-microvolt = <1212500 1212500 150>;
clock-latency-ns = <14>;
};
opp-16 {
opp-hz = /bits/ 64 <16>;
- opp-microvolt = <1175000>;
+ opp-microvolt = <1175000 1175000 150>;
clock-latency-ns = <14>;
};
opp-15 {
opp-hz = /bits/ 64 <15>;
- opp-microvolt = <1137500>;
+ opp-microvolt = <1137500 1137500 150>;
clock-latency-ns = <14>;
};
opp-14 {
opp-hz = /bits/ 64 <14>;
- opp-microvolt = <1112500>;
+ opp-microvolt = <1112500 1112500 150>;
clock-latency-ns = <14>;
};
opp-13 {
opp-hz = /bits/ 64 <13>;
- opp-microvolt = <1062500>;
+ opp-microvolt = <1062500 1062500 150>;
clock-latency-ns = <14>;
};
opp-12 {
opp-hz = /bits/ 64 <12>;
- opp-microvolt = <1037500>;
+ opp-microvolt = <1037500 1037500 150>;
clock-latency-ns = <14>;
};
opp-11 {
opp-hz = /bits/ 64 <11>;
- opp-microvolt = <1012500>;
+ opp-microvolt = <1012500 1012500 150>;
clock-latency-ns = <14>;
};
opp-10 {
opp-hz = /bits/ 64 <10>;
- opp-microvolt = < 987500>;
+ opp-microvolt = < 987500 987500 150>;
clock-latency-ns = <14>;
};
opp-9 {
opp-hz = /bits/ 64 <9>;
- opp-microvolt = < 962500>;
+ opp-microvolt = < 962500 962500 150>;
clock-latency-ns = <14>;
};
opp-8 {
opp-hz = /bits/ 64 <8>;
- opp-microvolt = < 937500>;
+ opp-microvolt = < 937500 937500 150>;
clock-latency-ns = <14>;
};
opp-7 {
opp-hz = /bits/ 64 <7>;
- opp-microvolt = < 912500>;
+ opp-microvolt = < 912500 912500 150>;
clock-latency-ns = <14>;
[PATCH v2 4/4] dt-bindings: devfreq: exynos-bus: remove unused property
Remove unused DT property "exynos,voltage-tolerance". Signed-off-by: Kamil Konieczny --- Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt index f8e946471a58..e71f752cc18f 100644 --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt @@ -50,8 +50,6 @@ Required properties only for passive bus device: Optional properties only for parent bus device: - exynos,saturation-ratio: the percentage value which is used to calibrate the performance count against total cycle count. -- exynos,voltage-tolerance: the percentage value for bus voltage tolerance - which is used to calculate the max voltage. Detailed correlation between sub-blocks and power line according to Exynos SoC: - In case of Exynos3250, there are two power line as following: -- 2.22.0
[PATCH v2 2/4] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
Reuse opp core code for setting bus clock and voltage. As a side
effect this allow useage of coupled regulators feature (required
for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
uses regulator_set_voltage_triplet() for setting regulator voltage
while the old code used regulator_set_voltage_tol() with fixed
tolerance. This patch also removes no longer needed parsing of DT
property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
it).
Signed-off-by: Kamil Konieczny
---
drivers/devfreq/exynos-bus.c | 172 ++-
1 file changed, 66 insertions(+), 106 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 486cc5b422f1..7fc4f76bd848 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -25,7 +25,6 @@
#include
#define DEFAULT_SATURATION_RATIO 40
-#define DEFAULT_VOLTAGE_TOLERANCE 2
struct exynos_bus {
struct device *dev;
@@ -37,9 +36,9 @@ struct exynos_bus {
unsigned long curr_freq;
- struct regulator *regulator;
+ struct opp_table *opp_table;
+
struct clk *clk;
- unsigned int voltage_tolerance;
unsigned int ratio;
};
@@ -99,56 +98,25 @@ static int exynos_bus_target(struct device *dev, unsigned
long *freq, u32 flags)
{
struct exynos_bus *bus = dev_get_drvdata(dev);
struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq, new_volt, tol;
int ret = 0;
-
- /* Get new opp-bus instance according to new bus clock */
+ /*
+* New frequency for bus may not be exactly matched to opp, adjust
+* *freq to correct value.
+*/
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
dev_err(dev, "failed to get recommended opp instance\n");
return PTR_ERR(new_opp);
}
- new_freq = dev_pm_opp_get_freq(new_opp);
- new_volt = dev_pm_opp_get_voltage(new_opp);
dev_pm_opp_put(new_opp);
- old_freq = bus->curr_freq;
-
- if (old_freq == new_freq)
- return 0;
- tol = new_volt * bus->voltage_tolerance / 100;
-
/* Change voltage and frequency according to new OPP level */
mutex_lock(&bus->lock);
+ ret = dev_pm_opp_set_rate(dev, *freq);
+ if (!ret)
+ bus->curr_freq = *freq;
- if (old_freq < new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
-
- ret = clk_set_rate(bus->clk, new_freq);
- if (ret < 0) {
- dev_err(dev, "failed to change clock of bus\n");
- clk_set_rate(bus->clk, old_freq);
- goto out;
- }
-
- if (old_freq > new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
- bus->curr_freq = new_freq;
-
- dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
- old_freq, new_freq, clk_get_rate(bus->clk));
-out:
mutex_unlock(&bus->lock);
return ret;
@@ -194,10 +162,11 @@ static void exynos_bus_exit(struct device *dev)
if (ret < 0)
dev_warn(dev, "failed to disable the devfreq-event devices\n");
- if (bus->regulator)
- regulator_disable(bus->regulator);
+ if (bus->opp_table)
+ dev_pm_opp_put_regulators(bus->opp_table);
dev_pm_opp_of_remove_table(dev);
+
clk_disable_unprepare(bus->clk);
}
@@ -209,39 +178,26 @@ static int exynos_bus_passive_target(struct device *dev,
unsigned long *freq,
{
struct exynos_bus *bus = dev_get_drvdata(dev);
struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq;
- int ret = 0;
+ int ret;
- /* Get new opp-bus instance according to new bus clock */
+ /*
+* New frequency for bus may not be exactly matched to opp, adjust
+* *freq to correct value.
+*/
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
dev_err(dev, "failed to get recommended opp instance\n");
return PTR_ERR(new_opp);
}
- new_freq = dev_pm_opp_get_freq(new_opp);
dev_pm_opp_put(new_opp);
- old_freq = bus->curr_freq;
-
- if (old_freq == new_freq)
- return 0;
-
/* Change the frequency according to new OPP level */
Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
Hi Chanwoo,
On 25.07.2019 12:17, Chanwoo Choi wrote:
> Hi Kamil,
>
> Looks good to me. But, I have some comment. Please check them.
Thank you for review, please see answers below.
> After this patch, exynos_bus_target is perfectly same with
> exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
Ok, I will make it in separate patch to simplify review process.
> On 19. 7. 20. 오전 12:05, k.koniec...@partner.samsung.com wrote:
>> Reuse opp core code for setting bus clock and voltage. As a side
>> effect this allow useage of coupled regulators feature (required
>
> s/useage/usage ?
Good catch, I will change it.
>> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
>> uses regulator_set_voltage_triplet() for setting regulator voltage
>> while the old code used regulator_set_voltage_tol() with fixed
>> tolerance. This patch also removes no longer needed parsing of DT
>> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
>> it).
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> drivers/devfreq/exynos-bus.c | 143 +--
>> 1 file changed, 37 insertions(+), 106 deletions(-)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index f391044aa39d..c2147b0912a0 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -25,7 +25,6 @@
>> #include
>>
>> #define DEFAULT_SATURATION_RATIO40
>> -#define DEFAULT_VOLTAGE_TOLERANCE 2
>>
>> struct exynos_bus {
>> struct device *dev;
>> @@ -37,9 +36,9 @@ struct exynos_bus {
>>
>> unsigned long curr_freq;
>>
>> -struct regulator *regulator;
>> +struct opp_table *opp_table;
>> +
>> struct clk *clk;
>> -unsigned int voltage_tolerance;
>> unsigned int ratio;
>> };
>>
>> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev,
>> unsigned long *freq, u32 flags)
>> {
>> struct exynos_bus *bus = dev_get_drvdata(dev);
>> struct dev_pm_opp *new_opp;
>> -unsigned long old_freq, new_freq, new_volt, tol;
>> int ret = 0;
>>
>> -/* Get new opp-bus instance according to new bus clock */
>> +/* Get correct frequency for bus. */
>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>> if (IS_ERR(new_opp)) {
>> dev_err(dev, "failed to get recommended opp instance\n");
>> return PTR_ERR(new_opp);
>> }
>>
>> -new_freq = dev_pm_opp_get_freq(new_opp);
>> -new_volt = dev_pm_opp_get_voltage(new_opp);
>> dev_pm_opp_put(new_opp);
>>
>> -old_freq = bus->curr_freq;
>> -
>> -if (old_freq == new_freq)
>> -return 0;
>> -tol = new_volt * bus->voltage_tolerance / 100;
>> -
>> /* Change voltage and frequency according to new OPP level */
>> mutex_lock(&bus->lock);
>> +ret = dev_pm_opp_set_rate(dev, *freq);
>> +if (!ret)
>> +bus->curr_freq = *freq;
>>
>> -if (old_freq < new_freq) {
>> -ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>> -if (ret < 0) {
>> -dev_err(bus->dev, "failed to set voltage\n");
>> -goto out;
>> -}
>> -}
>> -
>> -ret = clk_set_rate(bus->clk, new_freq);
>> -if (ret < 0) {
>> -dev_err(dev, "failed to change clock of bus\n");
>> -clk_set_rate(bus->clk, old_freq);
>> -goto out;
>> -}
>> -
>> -if (old_freq > new_freq) {
>> -ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>> -if (ret < 0) {
>> -dev_err(bus->dev, "failed to set voltage\n");
>> -goto out;
>> -}
>> -}
>> -bus->curr_freq = new_freq;
>> -
>> -dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>> -old_freq, new_freq, clk_get_rate(bus->clk));
>> -out:
>> mutex_unlock(&bus->lock);
>>
>> return ret;
>> @@ -195,8 +161,8 @@ static void exynos_bus_exit(struct device *dev)
>> dev_warn(dev, "failed to disable the devfreq-event devices\n");
>>
>> clk_disable_unprepare(bus->
Re: [PATCH v4 1/5] devfreq: exynos-bus: correct clock enable sequence
Hi,
On 25.07.2019 16:59, Chanwoo Choi wrote:
> Hi,
>
> You are missing my Acked tag.
I changed code, so I can not add your Ack in v4.
Please send new Ack for this patch.
> 2019년 7월 25일 (목) 오후 11:44, 님이 작성:
>>
>> Regulators should be enabled before clocks to avoid h/w hang. This
>> require change in exynos_bus_probe() to move exynos_bus_parse_of()
>> after exynos_bus_parent_parse_of() and change in error handling.
>> Similar change is needed in exynos_bus_exit() where clock should be
>> disabled before regulators.
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> Changes:
>> v4:
>> - move regulator disable after clock disable
>> - remove unrelated changes
>> - add disabling regulators in error path in exynos_bus_probe()
>>
>> ---
>> drivers/devfreq/exynos-bus.c | 31 +--
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index 486cc5b422f1..f34fa26f00d0 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
>> if (ret < 0)
>> dev_warn(dev, "failed to disable the devfreq-event
>> devices\n");
>>
>> - if (bus->regulator)
>> - regulator_disable(bus->regulator);
>> -
>> dev_pm_opp_of_remove_table(dev);
>> clk_disable_unprepare(bus->clk);
>> + if (bus->regulator)
>> + regulator_disable(bus->regulator);
>> }
>>
>> /*
>> @@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
>> struct exynos_bus *bus;
>> int ret, max_state;
>> unsigned long min_freq, max_freq;
>> + bool passive = false;
>>
>> if (!np) {
>> dev_err(dev, "failed to find devicetree node\n");
>> @@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device
>> *pdev)
>> bus->dev = &pdev->dev;
>> platform_set_drvdata(pdev, bus);
>>
>> - /* Parse the device-tree to get the resource information */
>> - ret = exynos_bus_parse_of(np, bus);
>> - if (ret < 0)
>> - return ret;
>> -
>> profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
>> - if (!profile) {
>> - ret = -ENOMEM;
>> - goto err;
>> - }
>> + if (!profile)
>> + return -ENOMEM;
>>
>> node = of_parse_phandle(dev->of_node, "devfreq", 0);
>> if (node) {
>> of_node_put(node);
>> - goto passive;
>> + passive = true;
>> } else {
>> ret = exynos_bus_parent_parse_of(np, bus);
>> + if (ret < 0)
>> + return ret;
>> }
>>
>> + /* Parse the device-tree to get the resource information */
>> + ret = exynos_bus_parse_of(np, bus);
>> if (ret < 0)
>> - goto err;
>> + goto err_reg;
>> +
>> + if (passive)
>> + goto passive;
>>
>> /* Initialize the struct profile and governor data for parent device
>> */
>> profile->polling_ms = 50;
>> @@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
>> err:
>> dev_pm_opp_of_remove_table(dev);
>> clk_disable_unprepare(bus->clk);
>> +err_reg:
>> + if (!passive)
>> + regulator_disable(bus->regulator);
>>
>> return ret;
>> }
>> --
>> 2.22.0
>>
>
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
Hi,
On 25.07.2019 16:53, Chanwoo Choi wrote:
> 2019년 7월 25일 (목) 오후 11:19, Kamil Konieczny
> 님이 작성:
>>
>> Hi Chanwoo,
>>
>> On 25.07.2019 12:17, Chanwoo Choi wrote:
>>> Hi Kamil,
>>>
>>> Looks good to me. But, I have some comment. Please check them.
>>
>> Thank you for review, please see answers below.
>>
>>> After this patch, exynos_bus_target is perfectly same with
>>> exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
>>
>> Ok, I will make it in separate patch to simplify review process.
>
> I think you can just modify this patch without any separate patch.
Do you want me to send v5 with patch 5 squashed in patch 3 ?
>>> On 19. 7. 20. 오전 12:05, k.koniec...@partner.samsung.com wrote:
>>>> Reuse opp core code for setting bus clock and voltage. As a side
>>>> effect this allow useage of coupled regulators feature (required
>>>
>>> s/useage/usage ?
>>
>> Good catch, I will change it.
>>
>>>> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
>>>> uses regulator_set_voltage_triplet() for setting regulator voltage
>>>> while the old code used regulator_set_voltage_tol() with fixed
>>>> tolerance. This patch also removes no longer needed parsing of DT
>>>> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
>>>> it).
>>>>
>>>> Signed-off-by: Kamil Konieczny
>>>> ---
>>>> drivers/devfreq/exynos-bus.c | 143 +--
>>>> 1 file changed, 37 insertions(+), 106 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>>> index f391044aa39d..c2147b0912a0 100644
>>>> --- a/drivers/devfreq/exynos-bus.c
>>>> +++ b/drivers/devfreq/exynos-bus.c
>>>> @@ -25,7 +25,6 @@
>>>> #include
>>>>
>>>> #define DEFAULT_SATURATION_RATIO40
>>>> -#define DEFAULT_VOLTAGE_TOLERANCE 2
>>>>
>>>> struct exynos_bus {
>>>> struct device *dev;
>>>> @@ -37,9 +36,9 @@ struct exynos_bus {
>>>>
>>>> unsigned long curr_freq;
>>>>
>>>> -struct regulator *regulator;
>>>> +struct opp_table *opp_table;
>>>> +
>>>> struct clk *clk;
>>>> -unsigned int voltage_tolerance;
>>>> unsigned int ratio;
>>>> };
>>>>
>>>> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev,
>>>> unsigned long *freq, u32 flags)
>>>> {
>>>> struct exynos_bus *bus = dev_get_drvdata(dev);
>>>> struct dev_pm_opp *new_opp;
>>>> -unsigned long old_freq, new_freq, new_volt, tol;
>>>> int ret = 0;
>>>>
>>>> -/* Get new opp-bus instance according to new bus clock */
>>>> +/* Get correct frequency for bus. */
>>>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>> if (IS_ERR(new_opp)) {
>>>> dev_err(dev, "failed to get recommended opp instance\n");
>>>> return PTR_ERR(new_opp);
>>>> }
>>>>
>>>> -new_freq = dev_pm_opp_get_freq(new_opp);
>>>> -new_volt = dev_pm_opp_get_voltage(new_opp);
>>>> dev_pm_opp_put(new_opp);
>>>>
>>>> -old_freq = bus->curr_freq;
>>>> -
>>>> -if (old_freq == new_freq)
>>>> -return 0;
>>>> -tol = new_volt * bus->voltage_tolerance / 100;
>>>> -
>>>> /* Change voltage and frequency according to new OPP level */
>>>> mutex_lock(&bus->lock);
>>>> +ret = dev_pm_opp_set_rate(dev, *freq);
>>>> +if (!ret)
>>>> +bus->curr_freq = *freq;
>>>>
>>>> -if (old_freq < new_freq) {
>>>> -ret = regulator_set_voltage_tol(bus->regulator, new_volt,
>>>> tol);
>>>> -if (ret < 0) {
>>>> -dev_err(bus->dev, "failed to set voltage\n");
>>>> -goto out;
>>>> -}
>>>> -}
>>>> -
>>>> -ret = clk_set_rate(bus->clk, new_freq);
>>>> -if (ret < 0) {
>>>> -dev_err(dev, "failed to change
Re: [PATCH v5 0/4] add coupled regulators for Exynos5422/5800
Hi Chanwoo, On 08.08.2019 03:47, Chanwoo Choi wrote: > Hi Kamil, > > When I applied them to testing branch, those don't have the author name > only just have the email address as following: > You have to edit the your git author information with your name. Sorry, I will resend with proper From: info. > authork.koniec...@partner.samsung.com >2019-08-07 15:38:36 +0200 > committer Chanwoo Choi 2019-08-08 10:35:16 > +0900 > commit4304f4ecec93cebd255463d56b0a4f112ee9dc50 (patch) > tree 2859e566d6f68219f71a61e7c412717c1adba4f5 > parent57d85421038b458dd87ec268404ff608f90c36ae (diff) > download linux-4304f4ecec93cebd255463d56b0a4f112ee9dc50.tar.gz > > Regards, > Chanwoo Choi > > On 19. 8. 7. 오후 10:38, k.koniec...@partner.samsung.com wrote: >> [...] -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
[RESEND PATCH v5 0/4] add coupled regulators for Exynos5422/5800
Hi, The main purpose of this patch series is to add coupled regulators for Exynos5422/5800 to keep constrain on voltage difference between vdd_arm and vdd_int to be at most 300mV. In exynos-bus instead of using regulator_set_voltage_tol() with default voltage tolerance it should be used regulator_set_voltage_triplet() with volatege range, and this is already present in opp/core.c code, so it can be reused. While at this, move setting regulators into opp/core. This patchset was tested on Odroid XU3. The DTS coupled regulators patch depends on previous patches. Changes: v5: - squashed last patch "remove exynos_bus_passive_target()" into second - added Acked-by to patch "correct clock enable sequence" v4: - removed "opp: core: add regulators enable and disable" from patchset as it was applied by Viresh Kumar and changed cover letter - fix patch "devfreq: exynos-bus: correct clock enable sequence" to correct order of enable/disable - removed unrelated changes in "devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()" - added new patch "devfreq: exynos-bus: remove exynos_bus_passive_target()" as suggested by Chanwoo Choi v3: - added new exynos-bus patch to correct clock and regulator enabling and disabling sequence as suggested by Chanwoo Choi - corrected error path in enable and improved commit message in opp/core - improve comment in devfreq/exynos-bus.c before devfreq_recommended_opp() - change cover letter as there is new patch - added note before Signed-off-by in 4th patch v2: - improve regulators enable/disable code in opp/core as suggested by Viresh Kumar - add new patch for remove unused dt-bindings as suggested by Krzysztof Kozlowski Kamil Konieczny (3): devfreq: exynos-bus: correct clock enable sequence devfreq: exynos-bus: convert to use dev_pm_opp_set_rate() dt-bindings: devfreq: exynos-bus: remove unused property Marek Szyprowski (1): ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800 .../bindings/devfreq/exynos-bus.txt | 2 - arch/arm/boot/dts/exynos5420.dtsi | 34 ++-- arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 + arch/arm/boot/dts/exynos5800.dtsi | 32 ++-- drivers/devfreq/exynos-bus.c | 153 +- 6 files changed, 78 insertions(+), 151 deletions(-) -- 2.22.0
[RESEND PATCH v5 4/4] dt-bindings: devfreq: exynos-bus: remove unused property
Remove unused DT property "exynos,voltage-tolerance". Signed-off-by: Kamil Konieczny Acked-by: Chanwoo Choi --- Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt index f8e946471a58..e71f752cc18f 100644 --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt @@ -50,8 +50,6 @@ Required properties only for passive bus device: Optional properties only for parent bus device: - exynos,saturation-ratio: the percentage value which is used to calibrate the performance count against total cycle count. -- exynos,voltage-tolerance: the percentage value for bus voltage tolerance - which is used to calculate the max voltage. Detailed correlation between sub-blocks and power line according to Exynos SoC: - In case of Exynos3250, there are two power line as following: -- 2.22.0
[RESEND PATCH v5 3/4] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800
From: Marek Szyprowski
Declare Exynos5422/5800 voltage ranges for opp points for big cpu core and
bus wcore and couple their voltage supllies as vdd_arm and vdd_int should
be in 300mV range.
Signed-off-by: Marek Szyprowski
[k.konieczny: add missing patch description]
Signed-off-by: Kamil Konieczny
Reviewed-by: Chanwoo Choi
---
arch/arm/boot/dts/exynos5420.dtsi | 34 +--
arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +++
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +++
arch/arm/boot/dts/exynos5800.dtsi | 32 -
4 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi
b/arch/arm/boot/dts/exynos5420.dtsi
index 5fb2326875dc..0cbf74750553 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -48,62 +48,62 @@
opp-shared;
opp-18 {
opp-hz = /bits/ 64 <18>;
- opp-microvolt = <125>;
+ opp-microvolt = <125 125 150>;
clock-latency-ns = <14>;
};
opp-17 {
opp-hz = /bits/ 64 <17>;
- opp-microvolt = <1212500>;
+ opp-microvolt = <1212500 1212500 150>;
clock-latency-ns = <14>;
};
opp-16 {
opp-hz = /bits/ 64 <16>;
- opp-microvolt = <1175000>;
+ opp-microvolt = <1175000 1175000 150>;
clock-latency-ns = <14>;
};
opp-15 {
opp-hz = /bits/ 64 <15>;
- opp-microvolt = <1137500>;
+ opp-microvolt = <1137500 1137500 150>;
clock-latency-ns = <14>;
};
opp-14 {
opp-hz = /bits/ 64 <14>;
- opp-microvolt = <1112500>;
+ opp-microvolt = <1112500 1112500 150>;
clock-latency-ns = <14>;
};
opp-13 {
opp-hz = /bits/ 64 <13>;
- opp-microvolt = <1062500>;
+ opp-microvolt = <1062500 1062500 150>;
clock-latency-ns = <14>;
};
opp-12 {
opp-hz = /bits/ 64 <12>;
- opp-microvolt = <1037500>;
+ opp-microvolt = <1037500 1037500 150>;
clock-latency-ns = <14>;
};
opp-11 {
opp-hz = /bits/ 64 <11>;
- opp-microvolt = <1012500>;
+ opp-microvolt = <1012500 1012500 150>;
clock-latency-ns = <14>;
};
opp-10 {
opp-hz = /bits/ 64 <10>;
- opp-microvolt = < 987500>;
+ opp-microvolt = < 987500 987500 150>;
clock-latency-ns = <14>;
};
opp-9 {
opp-hz = /bits/ 64 <9>;
- opp-microvolt = < 962500>;
+ opp-microvolt = < 962500 962500 150>;
clock-latency-ns = <14>;
};
opp-8 {
opp-hz = /bits/ 64 <8>;
- opp-microvolt = < 937500>;
+ opp-microvolt = < 937500 937500 150>;
clock-latency-ns = <14>;
};
opp-7 {
opp-hz = /bits/ 64 <7>;
- opp-microvolt = < 912500>;
+
[RESEND PATCH v5 2/4] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
Reuse opp core code for setting bus clock and voltage. As a side
effect this allow usage of coupled regulators feature (required
for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
uses regulator_set_voltage_triplet() for setting regulator voltage
while the old code used regulator_set_voltage_tol() with fixed
tolerance. This patch also removes no longer needed parsing of DT
property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
it). After applying changes both functions exynos_bus_passive_target()
and exynos_bus_target() have the same code, so remove
exynos_bus_passive_target(). In exynos_bus_probe() replace it with
exynos_bus_target.
Signed-off-by: Kamil Konieczny
Acked-by: Chanwoo Choi
---
Changes:
v5:
- squashed last patch into this one, as suggested by Chanwoo Choi
v4:
- remove unrelated changes, add newline before comment
---
drivers/devfreq/exynos-bus.c | 130 +++
1 file changed, 24 insertions(+), 106 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index f34fa26f00d0..2aeb6cc07960 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -25,7 +25,6 @@
#include
#define DEFAULT_SATURATION_RATIO 40
-#define DEFAULT_VOLTAGE_TOLERANCE 2
struct exynos_bus {
struct device *dev;
@@ -37,9 +36,8 @@ struct exynos_bus {
unsigned long curr_freq;
- struct regulator *regulator;
+ struct opp_table *opp_table;
struct clk *clk;
- unsigned int voltage_tolerance;
unsigned int ratio;
};
@@ -93,62 +91,29 @@ static int exynos_bus_get_event(struct exynos_bus *bus,
}
/*
- * Must necessary function for devfreq simple-ondemand governor
+ * devfreq function for both simple-ondemand and passive governor
*/
static int exynos_bus_target(struct device *dev, unsigned long *freq, u32
flags)
{
struct exynos_bus *bus = dev_get_drvdata(dev);
struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq, new_volt, tol;
int ret = 0;
- /* Get new opp-bus instance according to new bus clock */
+ /* Get correct frequency for bus. */
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
dev_err(dev, "failed to get recommended opp instance\n");
return PTR_ERR(new_opp);
}
- new_freq = dev_pm_opp_get_freq(new_opp);
- new_volt = dev_pm_opp_get_voltage(new_opp);
dev_pm_opp_put(new_opp);
- old_freq = bus->curr_freq;
-
- if (old_freq == new_freq)
- return 0;
- tol = new_volt * bus->voltage_tolerance / 100;
-
/* Change voltage and frequency according to new OPP level */
mutex_lock(&bus->lock);
+ ret = dev_pm_opp_set_rate(dev, *freq);
+ if (!ret)
+ bus->curr_freq = *freq;
- if (old_freq < new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
-
- ret = clk_set_rate(bus->clk, new_freq);
- if (ret < 0) {
- dev_err(dev, "failed to change clock of bus\n");
- clk_set_rate(bus->clk, old_freq);
- goto out;
- }
-
- if (old_freq > new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
- bus->curr_freq = new_freq;
-
- dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
- old_freq, new_freq, clk_get_rate(bus->clk));
-out:
mutex_unlock(&bus->lock);
return ret;
@@ -196,54 +161,10 @@ static void exynos_bus_exit(struct device *dev)
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
- if (bus->regulator)
- regulator_disable(bus->regulator);
-}
-
-/*
- * Must necessary function for devfreq passive governor
- */
-static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
- u32 flags)
-{
- struct exynos_bus *bus = dev_get_drvdata(dev);
- struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq;
- int ret = 0;
-
- /* Get new opp-bus instance according to new bus clock */
- new_opp = devfreq_recommended_opp(dev, freq, flags);
- if (IS_ERR(new_opp)) {
- dev_err(dev, "failed to get recommended opp instance\n");
- return PTR_ERR(new_opp);
- }
-
- new_freq = dev_pm_opp_get_f
[RESEND PATCH v5 1/4] devfreq: exynos-bus: correct clock enable sequence
Regulators should be enabled before clocks to avoid h/w hang. This
require change in exynos_bus_probe() to move exynos_bus_parse_of()
after exynos_bus_parent_parse_of() and change in error handling.
Similar change is needed in exynos_bus_exit() where clock should be
disabled before regulators.
Signed-off-by: Kamil Konieczny
Acked-by: Chanwoo Choi
---
Changes:
v5:
- added Acked-by tag
v4:
- move regulator disable after clock disable
- remove unrelated changes
- add disabling regulators in error path in exynos_bus_probe()
---
drivers/devfreq/exynos-bus.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 486cc5b422f1..f34fa26f00d0 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
if (ret < 0)
dev_warn(dev, "failed to disable the devfreq-event devices\n");
- if (bus->regulator)
- regulator_disable(bus->regulator);
-
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
+ if (bus->regulator)
+ regulator_disable(bus->regulator);
}
/*
@@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
struct exynos_bus *bus;
int ret, max_state;
unsigned long min_freq, max_freq;
+ bool passive = false;
if (!np) {
dev_err(dev, "failed to find devicetree node\n");
@@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device *pdev)
bus->dev = &pdev->dev;
platform_set_drvdata(pdev, bus);
- /* Parse the device-tree to get the resource information */
- ret = exynos_bus_parse_of(np, bus);
- if (ret < 0)
- return ret;
-
profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
- if (!profile) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!profile)
+ return -ENOMEM;
node = of_parse_phandle(dev->of_node, "devfreq", 0);
if (node) {
of_node_put(node);
- goto passive;
+ passive = true;
} else {
ret = exynos_bus_parent_parse_of(np, bus);
+ if (ret < 0)
+ return ret;
}
+ /* Parse the device-tree to get the resource information */
+ ret = exynos_bus_parse_of(np, bus);
if (ret < 0)
- goto err;
+ goto err_reg;
+
+ if (passive)
+ goto passive;
/* Initialize the struct profile and governor data for parent device */
profile->polling_ms = 50;
@@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
err:
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
+err_reg:
+ if (!passive)
+ regulator_disable(bus->regulator);
return ret;
}
--
2.22.0
Re: [PATCH 0/3] add coupled regulators for Exynos5422/5800
On 10.07.2019 11:00, Krzysztof Kozlowski wrote: > On Mon, 8 Jul 2019 at 16:12, wrote: >> >> From: Kamil Konieczny >> >> Hi, >> >> The main purpose of this patch series is to add coupled regulators for >> Exynos5422/5800 to keep constrain on voltage difference between vdd_arm >> and vdd_int to be at most 300mV. In exynos-bus instead of using >> regulator_set_voltage_tol() with default voltage tolerance it should be >> used regulator_set_voltage_triplet() with volatege range, and this is >> already present in opp/core.c code, so it can be reused. While at this, >> move setting regulators into opp/core. >> >> This patchset was tested on Odroid XU3. >> >> The last patch depends on two previous. > > So you break the ABI... I assume that patchset maintains > bisectability. However there is no explanation why ABI break is needed > so this does not look good... Patchset is bisectable, first one is simple and do not depends on others, second depends on first, last depends on first and second. What do you mean by breaking ABI ? > Best regards, > Krzysztof > >> >> Regards, >> Kamil >> >> Kamil Konieczny (2): >> opp: core: add regulators enable and disable >> devfreq: exynos-bus: convert to use dev_pm_opp_set_rate() >> >> Marek Szyprowski (1): >> ARM: dts: exynos: add initial data for coupled regulators for >> Exynos5422/5800 >> >> arch/arm/boot/dts/exynos5420.dtsi | 34 ++-- >> arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 + >> arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 + >> arch/arm/boot/dts/exynos5800.dtsi | 32 ++-- >> drivers/devfreq/exynos-bus.c | 172 +++--- >> drivers/opp/core.c| 13 ++ >> 6 files changed, 120 insertions(+), 139 deletions(-) >> >> -- >> 2.22.0 -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH 1/3] opp: core: add regulators enable and disable
On 09.07.2019 07:40, Viresh Kumar wrote:
> On 08-07-19, 16:11, k.koniec...@partner.samsung.com wrote:
>> From: Kamil Konieczny
>>
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> drivers/opp/core.c | 13 +
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..947cac452854 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>> if (ret)
>> goto free_regulators;
>>
>> +for (i = 0; i < opp_table->regulator_count; i++) {
>> +ret = regulator_enable(opp_table->regulators[i]);
>> +if (ret < 0)
>> +goto disable;
>> +}
>
> I am wondering on why is this really required as this isn't done for
> any other platform, probably because the regulators are enabled by
> bootloader and are always on.
It was not enabled for historical reasons, from design point regualtors
should be enabled before use.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH 1/3] opp: core: add regulators enable and disable
On 10.07.2019 12:16, Kamil Konieczny wrote:
>
>
> On 09.07.2019 07:40, Viresh Kumar wrote:
>> On 08-07-19, 16:11, k.koniec...@partner.samsung.com wrote:
>>> From: Kamil Konieczny
>>>
>>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>>> regulators to dev_pm_opp_put_regulators(). This prepares for
>>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>>
>>> Signed-off-by: Kamil Konieczny
>>> ---
>>> drivers/opp/core.c | 13 +
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>>> index 0e7703fe733f..947cac452854 100644
>>> --- a/drivers/opp/core.c
>>> +++ b/drivers/opp/core.c
>>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>>> device *dev,
>>> if (ret)
>>> goto free_regulators;
>>>
>>> + for (i = 0; i < opp_table->regulator_count; i++) {
>>> + ret = regulator_enable(opp_table->regulators[i]);
>>> + if (ret < 0)
>>> + goto disable;
>>> + }
>>
>> I am wondering on why is this really required as this isn't done for
>> any other platform, probably because the regulators are enabled by
>> bootloader and are always on.
>
> It was not enabled for historical reasons, from design point regualtors
> should be enabled before use.
On Exynos platform devfreq driver (exynos-bus) always enabled them,
so I wanted to preserve the current behaviour.
I've also checked the change with cpufreq-dt driver and it doesn't cause
issues.
Do you find this change acceptable?
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH 1/3] opp: core: add regulators enable and disable
On 09.07.2019 07:40, Viresh Kumar wrote:
> On 08-07-19, 16:11, k.koniec...@partner.samsung.com wrote:
>> From: Kamil Konieczny
>>
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny
>> ---
>> drivers/opp/core.c | 13 +
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..947cac452854 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>> device *dev,
>> if (ret)
>> goto free_regulators;
>>
>> +for (i = 0; i < opp_table->regulator_count; i++) {
>> +ret = regulator_enable(opp_table->regulators[i]);
>> +if (ret < 0)
>> +goto disable;
>> +}
>
> I am wondering on why is this really required as this isn't done for
> any other platform, probably because the regulators are enabled by
> bootloader and are always on.
It is not ABI break, it should work with existing DTBs
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
Re: [PATCH 0/3] add coupled regulators for Exynos5422/5800
On 10.07.2019 12:14, Krzysztof Kozlowski wrote: > On Wed, 10 Jul 2019 at 12:03, Kamil Konieczny > wrote: >> >> On 10.07.2019 11:00, Krzysztof Kozlowski wrote: >>> On Mon, 8 Jul 2019 at 16:12, wrote: >>>> >>>> From: Kamil Konieczny >>>> >>>> Hi, >>>> >>>> The main purpose of this patch series is to add coupled regulators for >>>> Exynos5422/5800 to keep constrain on voltage difference between vdd_arm >>>> and vdd_int to be at most 300mV. In exynos-bus instead of using >>>> regulator_set_voltage_tol() with default voltage tolerance it should be >>>> used regulator_set_voltage_triplet() with volatege range, and this is >>>> already present in opp/core.c code, so it can be reused. While at this, >>>> move setting regulators into opp/core. >>>> >>>> This patchset was tested on Odroid XU3. >>>> >>>> The last patch depends on two previous. >>> >>> So you break the ABI... I assume that patchset maintains >>> bisectability. However there is no explanation why ABI break is needed >>> so this does not look good... >> >> Patchset is bisectable, first one is simple and do not depends on others, >> second depends on first, last depends on first and second. >> >> What do you mean by breaking ABI ? > > I mean, that Linux kernel stops working with existing DTBs... or am I > mistaken and there is no problem? Maybe I confused the order... It is not ABI break, it should work with existing DTBs -- Best regards, Kamil Konieczny Samsung R&D Institute Poland
Re: [PATCH 1/3] opp: core: add regulators enable and disable
On 10.07.2019 12:43, Kamil Konieczny wrote:
> On 09.07.2019 07:40, Viresh Kumar wrote:
>> On 08-07-19, 16:11, k.koniec...@partner.samsung.com wrote:
>>> From: Kamil Konieczny
>>>
>>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>>> regulators to dev_pm_opp_put_regulators(). This prepares for
>>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>>
>>> Signed-off-by: Kamil Konieczny
>>> ---
>>> drivers/opp/core.c | 13 +
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>>> index 0e7703fe733f..947cac452854 100644
>>> --- a/drivers/opp/core.c
>>> +++ b/drivers/opp/core.c
>>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct
>>> device *dev,
>>> if (ret)
>>> goto free_regulators;
>>>
>>> + for (i = 0; i < opp_table->regulator_count; i++) {
>>> + ret = regulator_enable(opp_table->regulators[i]);
>>> + if (ret < 0)
>>> + goto disable;
>>> + }
>>
>> I am wondering on why is this really required as this isn't done for
>> any other platform, probably because the regulators are enabled by
>> bootloader and are always on.
>
> It is not ABI break, it should work with existing DTBs
Sorry, this answer should go to question by Krzysztof
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
