* [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support
@ 2026-05-12 22:43 Lothar Rubusch
2026-05-12 22:43 ` [PATCH 01/12] crypto: atmel - introduce shared I2C client management Lothar Rubusch
` (12 more replies)
0 siblings, 13 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
This series restructures the Atmel secure element drivers around a
shared atmel-i2c core and adds SHA256 ahash support for ATSHA204A and
ECC based devices.
The existing drivers duplicated substantial parts of the transport,
RNG, EEPROM and device management logic. This series consolidates the
common functionality into the shared i2c core and converts the client
drivers to capability based allocation.
The series also introduces per-device timing configuration through
match data, moves sanity checks and RNG handling into the core driver,
updates workqueue handling and cleans up internal constants and helper
definitions.
The final patch adds SHA256 ahash support using the hardware SHA engine
provided by the devices.
ATSHA204A devices require software-side SHA256 padding according to
FIPS 180-4, while newer ECC devices provide a dedicated SHA final
command and perform padding internally in hardware.
Supporting the SHA engine also requires changes to the command
transport path. SHA operations must execute as a strict uninterrupted
sequence consisting of SHA INIT, one or more SHA COMPUTE commands and,
for ECC devices, a terminating SHA FINAL command. The device loses its
internal SHA state if it enters sleep mode or if unrelated commands
are interleaved during the transaction.
To satisfy these hardware requirements, the send/receive path is split
into a low-level transfer helper and a higher-level wrapper managing
wakeup, sleep and locking. SHA operations keep the device awake and
hold the i2c lock for the full duration of the hashing transaction.
The series has been tested on ATSHA204A and ATECC508A devices.
Tests are ongoing/pending on ATECC608A and ATECC608B.
---
Lothar Rubusch (12):
crypto: atmel - introduce shared I2C client management
crypto: atmel - move capability-based client allocation into i2c core
crypto: atmel - remove obsolete CONFIG_OF guard
crypto: atmel - add per-device timing and match-data driven
configuration
crypto: atmel - move RNG support into common i2c core
crypto: atmel - move EEPROM access support into common i2c core
crypto: atmel - expose CONFIG zone through sysfs
crypto: atmel - move device sanity check to core driver
crypto: atmel - check client data in remove callbacks
crypto: atmel - update workqueue flags and add flush on exit
crypto: atmel - refactor and localize driver constants
crypto: atmel - add SHA256 ahash support
drivers/crypto/atmel-ecc.c | 252 +++++++-----
drivers/crypto/atmel-i2c.c | 679 +++++++++++++++++++++++++++++----
drivers/crypto/atmel-i2c.h | 180 +++++----
drivers/crypto/atmel-sha204a.c | 284 +++++++-------
4 files changed, 1010 insertions(+), 385 deletions(-)
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
base-commit: f7dd32c5179d7755de18e21d5674b08f9e5cb180
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/12] crypto: atmel - introduce shared I2C client management
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 02/12] crypto: atmel - move capability-based client allocation into i2c core Lothar Rubusch
` (11 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Introduce a shared I2C client management infrastructure in the
atmel-i2c core and convert the atmel-ecc and atmel-sha204a drivers
to use it.
Replace the driver-local atmel_ecc_driver_data structure with the
common atmel_i2c_mgmt instance, providing a shared client list and
locking for compatible Atmel secure element devices. Add a common
atmel_i2c_unregister_client() helper to centralize client removal
handling.
Refactor both drivers to use module_i2c_driver() and move duplicated
client list handling into the shared infrastructure. Probe and remove
paths are updated accordingly, including consistent error unwinding
for client registration failures.
Subsequent patches will build on the shared client infrastructure.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 58 ++++++++++++++--------------------
drivers/crypto/atmel-i2c.c | 15 +++++++++
drivers/crypto/atmel-i2c.h | 5 ++-
drivers/crypto/atmel-sha204a.c | 38 ++++++++++++----------
4 files changed, 65 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 3738a4eb8701..cba4238735cc 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -23,8 +23,6 @@
#include <crypto/kpp.h>
#include "atmel-i2c.h"
-static struct atmel_ecc_driver_data driver_data;
-
/**
* struct atmel_ecdh_ctx - transformation context
* @client : pointer to i2c client device
@@ -209,14 +207,14 @@ static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
int min_tfm_cnt = INT_MAX;
int tfm_cnt;
- spin_lock(&driver_data.i2c_list_lock);
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
- if (list_empty(&driver_data.i2c_client_list)) {
- spin_unlock(&driver_data.i2c_list_lock);
+ if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
return ERR_PTR(-ENODEV);
}
- list_for_each_entry(i2c_priv, &driver_data.i2c_client_list,
+ list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
i2c_client_list_node) {
tfm_cnt = atomic_read(&i2c_priv->tfm_count);
if (tfm_cnt < min_tfm_cnt) {
@@ -232,7 +230,7 @@ static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
client = min_i2c_priv->client;
}
- spin_unlock(&driver_data.i2c_list_lock);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
return client;
}
@@ -319,27 +317,34 @@ static int atmel_ecc_probe(struct i2c_client *client)
ret = atmel_i2c_probe(client);
if (ret)
- return ret;
+ goto done;
i2c_priv = i2c_get_clientdata(client);
- spin_lock(&driver_data.i2c_list_lock);
+ /* add to client list */
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
- &driver_data.i2c_client_list);
- spin_unlock(&driver_data.i2c_list_lock);
+ &atmel_i2c_mgmt.i2c_client_list);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ /* register algorithms */
ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
if (ret) {
- spin_lock(&driver_data.i2c_list_lock);
- list_del(&i2c_priv->i2c_client_list_node);
- spin_unlock(&driver_data.i2c_list_lock);
-
dev_err(&client->dev, "%s alg registration failed\n",
atmel_ecdh_nist_p256.base.cra_driver_name);
+ goto err_list_del;
} else {
dev_info(&client->dev, "atmel ecc algorithms registered in /proc/crypto\n");
}
+ goto done;
+
+err_list_del:
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ list_del(&i2c_priv->i2c_client_list_node);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+done:
return ret;
}
@@ -361,11 +366,10 @@ static void atmel_ecc_remove(struct i2c_client *client)
return;
}
- crypto_unregister_kpp(&atmel_ecdh_nist_p256);
+ atmel_i2c_unregister_client(i2c_priv);
+ atmel_i2c_flush_queue();
- spin_lock(&driver_data.i2c_list_lock);
- list_del(&i2c_priv->i2c_client_list_node);
- spin_unlock(&driver_data.i2c_list_lock);
+ crypto_unregister_kpp(&atmel_ecdh_nist_p256);
}
#ifdef CONFIG_OF
@@ -398,21 +402,7 @@ static struct i2c_driver atmel_ecc_driver = {
.id_table = atmel_ecc_id,
};
-static int __init atmel_ecc_init(void)
-{
- spin_lock_init(&driver_data.i2c_list_lock);
- INIT_LIST_HEAD(&driver_data.i2c_client_list);
- return i2c_add_driver(&atmel_ecc_driver);
-}
-
-static void __exit atmel_ecc_exit(void)
-{
- atmel_i2c_flush_queue();
- i2c_del_driver(&atmel_ecc_driver);
-}
-
-module_init(atmel_ecc_init);
-module_exit(atmel_ecc_exit);
+module_i2c_driver(atmel_ecc_driver);
MODULE_AUTHOR("Tudor Ambarus");
MODULE_DESCRIPTION("Microchip / Atmel ECC (I2C) driver");
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 0e275dbdc8c5..861af52d7a88 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -21,6 +21,12 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
+struct atmel_i2c_client_mgmt atmel_i2c_mgmt = {
+ .i2c_list_lock = __SPIN_LOCK_UNLOCKED(atmel_i2c_mgmt.i2c_list_lock),
+ .i2c_client_list = LIST_HEAD_INIT(atmel_i2c_mgmt.i2c_client_list),
+};
+EXPORT_SYMBOL_GPL(atmel_i2c_mgmt);
+
static const struct {
u8 value;
const char *error_text;
@@ -348,6 +354,15 @@ static int device_sanity_check(struct i2c_client *client)
return ret;
}
+void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv)
+{
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ if (!list_empty(&i2c_priv->i2c_client_list_node))
+ list_del_init(&i2c_priv->i2c_client_list_node);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+}
+EXPORT_SYMBOL(atmel_i2c_unregister_client);
+
int atmel_i2c_probe(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv;
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 72f04c15682f..43a0c1cfcd94 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -115,10 +115,11 @@ struct atmel_i2c_cmd {
#define ECDH_PREFIX_MODE 0x00
/* Used for binding tfm objects to i2c clients. */
-struct atmel_ecc_driver_data {
+struct atmel_i2c_client_mgmt {
struct list_head i2c_client_list;
spinlock_t i2c_list_lock;
} ____cacheline_aligned;
+extern struct atmel_i2c_client_mgmt atmel_i2c_mgmt;
/**
* atmel_i2c_client_priv - i2c_client private data
@@ -189,4 +190,6 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey);
+void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
+
#endif /* __ATMEL_I2C_H__ */
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index ed7d69bf6890..e6808c2bc891 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -169,10 +169,17 @@ static int atmel_sha204a_probe(struct i2c_client *client)
ret = atmel_i2c_probe(client);
if (ret)
- return ret;
+ goto done;
i2c_priv = i2c_get_clientdata(client);
+ /* add to client list */
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ list_add_tail(&i2c_priv->i2c_client_list_node,
+ &atmel_i2c_mgmt.i2c_client_list);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ /* register rng */
memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
i2c_priv->hwrng.name = dev_name(&client->dev);
@@ -183,15 +190,26 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv->hwrng.quality = *quality;
ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
- if (ret)
+ if (ret) {
dev_warn(&client->dev, "failed to register RNG (%d)\n", ret);
+ goto err_list_del;
+ }
ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
if (ret) {
dev_err(&client->dev, "failed to register sysfs entry\n");
- return ret;
+ goto err_list_del;
}
+ goto done;
+
+err_list_del:
+ sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ list_del(&i2c_priv->i2c_client_list_node);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+done:
return ret;
}
@@ -230,19 +248,7 @@ static struct i2c_driver atmel_sha204a_driver = {
.driver.of_match_table = of_match_ptr(atmel_sha204a_dt_ids),
};
-static int __init atmel_sha204a_init(void)
-{
- return i2c_add_driver(&atmel_sha204a_driver);
-}
-
-static void __exit atmel_sha204a_exit(void)
-{
- atmel_i2c_flush_queue();
- i2c_del_driver(&atmel_sha204a_driver);
-}
-
-module_init(atmel_sha204a_init);
-module_exit(atmel_sha204a_exit);
+module_i2c_driver(atmel_sha204a_driver);
MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro•org>");
MODULE_DESCRIPTION("Microchip / Atmel SHA204A (I2C) driver");
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/12] crypto: atmel - move capability-based client allocation into i2c core
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
2026-05-12 22:43 ` [PATCH 01/12] crypto: atmel - introduce shared I2C client management Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 03/12] crypto: atmel - remove obsolete CONFIG_OF guard Lothar Rubusch
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Move the i2c client allocation logic from atmel-ecc into the shared
atmel-i2c core and extend it to support capability-based client
selection.
Introduce enum atmel_i2c_capability and add capability flags to
struct atmel_i2c_client_priv. Devices now advertise their supported
features during probe, allowing atmel_i2c_client_alloc() to select a
compatible client from the shared i2c client list.
The allocation logic continues to balance crypto transformation usage
across devices by selecting the client with the lowest tfm_count, but
is no longer limited to ECC-capable devices.
This centralizes shared client management in the common atmel-i2c core
and prepares the infrastructure for additional shared crypto features
across compatible Atmel devices.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 39 +++-------------------------------
drivers/crypto/atmel-i2c.c | 39 ++++++++++++++++++++++++++++++++++
drivers/crypto/atmel-i2c.h | 7 ++++++
drivers/crypto/atmel-sha204a.c | 2 ++
4 files changed, 51 insertions(+), 36 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index cba4238735cc..c63d30947bd7 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -200,41 +200,6 @@ static int atmel_ecdh_compute_shared_secret(struct kpp_request *req)
return ret;
}
-static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
-{
- struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
- struct i2c_client *client = ERR_PTR(-ENODEV);
- int min_tfm_cnt = INT_MAX;
- int tfm_cnt;
-
- spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
-
- if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
- return ERR_PTR(-ENODEV);
- }
-
- list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
- i2c_client_list_node) {
- tfm_cnt = atomic_read(&i2c_priv->tfm_count);
- if (tfm_cnt < min_tfm_cnt) {
- min_tfm_cnt = tfm_cnt;
- min_i2c_priv = i2c_priv;
- }
- if (!min_tfm_cnt)
- break;
- }
-
- if (min_i2c_priv) {
- atomic_inc(&min_i2c_priv->tfm_count);
- client = min_i2c_priv->client;
- }
-
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
-
- return client;
-}
-
static void atmel_ecc_i2c_client_free(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
@@ -249,7 +214,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
struct atmel_ecdh_ctx *ctx = kpp_tfm_ctx(tfm);
ctx->curve_id = ECC_CURVE_NIST_P256;
- ctx->client = atmel_ecc_i2c_client_alloc();
+ ctx->client = atmel_i2c_client_alloc(ATMEL_CAP_ECDH);
if (IS_ERR(ctx->client)) {
pr_err("tfm - i2c_client binding failed\n");
return PTR_ERR(ctx->client);
@@ -321,6 +286,8 @@ static int atmel_ecc_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
+
/* add to client list */
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 861af52d7a88..b7ee2ec37531 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -57,6 +57,45 @@ static void atmel_i2c_checksum(struct atmel_i2c_cmd *cmd)
*__crc16 = cpu_to_le16(bitrev16(crc16(0, data, len)));
}
+struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap)
+{
+ struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
+ struct i2c_client *client = ERR_PTR(-ENODEV);
+ int min_tfm_cnt = INT_MAX;
+ int tfm_cnt;
+
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ return ERR_PTR(-ENODEV);
+ }
+
+ list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
+ i2c_client_list_node) {
+ if (!(i2c_priv->caps & BIT(cap)))
+ continue;
+
+ tfm_cnt = atomic_read(&i2c_priv->tfm_count);
+ if (tfm_cnt < min_tfm_cnt) {
+ min_tfm_cnt = tfm_cnt;
+ min_i2c_priv = i2c_priv;
+ }
+ if (!min_tfm_cnt)
+ break;
+ }
+
+ if (min_i2c_priv) {
+ atomic_inc(&min_i2c_priv->tfm_count);
+ client = min_i2c_priv->client;
+ }
+
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ return client;
+}
+EXPORT_SYMBOL(atmel_i2c_client_alloc);
+
void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd)
{
cmd->word_addr = COMMAND;
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 43a0c1cfcd94..70579b438256 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -115,6 +115,10 @@ struct atmel_i2c_cmd {
#define ECDH_PREFIX_MODE 0x00
/* Used for binding tfm objects to i2c clients. */
+enum atmel_i2c_capability {
+ ATMEL_CAP_ECDH = 0,
+};
+
struct atmel_i2c_client_mgmt {
struct list_head i2c_client_list;
spinlock_t i2c_list_lock;
@@ -130,6 +134,7 @@ extern struct atmel_i2c_client_mgmt atmel_i2c_mgmt;
* @wake_token_sz : size in bytes of the wake_token
* @tfm_count : number of active crypto transformations on i2c client
* @hwrng : hold the hardware generated rng
+ * @caps : feature capability of the particular driver
*
* Reads and writes from/to the i2c client are sequential. The first byte
* transmitted to the device is treated as the byte size. Any attempt to send
@@ -146,6 +151,7 @@ struct atmel_i2c_client_priv {
size_t wake_token_sz;
atomic_t tfm_count ____cacheline_aligned;
struct hwrng hwrng;
+ u32 caps;
};
/**
@@ -190,6 +196,7 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey);
+struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
#endif /* __ATMEL_I2C_H__ */
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index e6808c2bc891..ab758c9cd410 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -173,6 +173,8 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->caps = 0;
+
/* add to client list */
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/12] crypto: atmel - remove obsolete CONFIG_OF guard
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
2026-05-12 22:43 ` [PATCH 01/12] crypto: atmel - introduce shared I2C client management Lothar Rubusch
2026-05-12 22:43 ` [PATCH 02/12] crypto: atmel - move capability-based client allocation into i2c core Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 04/12] crypto: atmel - add per-device timing and match-data driven configuration Lothar Rubusch
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Remove the CONFIG_OF preprocessor guard around the OF device match
table in atmel-ecc.
OF match tables are expected to be present unconditionally and the
MODULE_DEVICE_TABLE(of, ...) handling already accounts for
configurations where OF support is disabled. Keeping the additional
guard provides no benefit and only adds unnecessary conditional
compilation.
Also compact the match table formatting while touching the code.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index c63d30947bd7..0dede3707b73 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -339,18 +339,12 @@ static void atmel_ecc_remove(struct i2c_client *client)
crypto_unregister_kpp(&atmel_ecdh_nist_p256);
}
-#ifdef CONFIG_OF
static const struct of_device_id atmel_ecc_dt_ids[] = {
- {
- .compatible = "atmel,atecc508a",
- }, {
- .compatible = "atmel,atecc608b",
- }, {
- /* sentinel */
- }
+ { .compatible = "atmel,atecc508a", },
+ { .compatible = "atmel,atecc608b", },
+ { }
};
MODULE_DEVICE_TABLE(of, atmel_ecc_dt_ids);
-#endif
static const struct i2c_device_id atmel_ecc_id[] = {
{ "atecc508a" },
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/12] crypto: atmel - add per-device timing and match-data driven configuration
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (2 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 03/12] crypto: atmel - remove obsolete CONFIG_OF guard Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 05/12] crypto: atmel - move RNG support into common i2c core Lothar Rubusch
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
The ATSHA204(A) and ATECC device families define different maximum
command execution times in their datasheets. The current driver uses a
mixed set of timing constants, which can result in insufficient wait
times for some devices.
Introduce struct atmel_i2c_of_match_data to provide per-device timing
information through the device match tables. Store the match data in the
client private structure and pass the timing parameters to the command
initialization helpers instead of relying on global timing constants.
This allows the common atmel-i2c core to use device-specific command
timeouts for operations such as READ, RANDOM, GENKEY, and ECDH.
Also move the legacy hwrng quality information into the match data
structure to consolidate per-device configuration in a single place.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 32 +++++++++++++---
drivers/crypto/atmel-i2c.c | 29 ++++++++------
drivers/crypto/atmel-i2c.h | 36 ++++++++++++------
drivers/crypto/atmel-sha204a.c | 69 ++++++++++++++++++++++++----------
4 files changed, 120 insertions(+), 46 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 0dede3707b73..7793f7b4e97e 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -76,6 +76,8 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
unsigned int len)
{
struct atmel_ecdh_ctx *ctx = kpp_tfm_ctx(tfm);
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(ctx->client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_cmd *cmd;
void *public_key;
struct ecdh params;
@@ -112,7 +114,7 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
ctx->do_fallback = false;
- atmel_i2c_init_genkey_cmd(cmd, DATA_SLOT_2);
+ atmel_i2c_init_genkey_cmd(cmd, DATA_SLOT_2, &data->timings);
ret = atmel_i2c_send_receive(ctx->client, cmd);
if (ret)
@@ -164,6 +166,8 @@ static int atmel_ecdh_compute_shared_secret(struct kpp_request *req)
{
struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
struct atmel_ecdh_ctx *ctx = kpp_tfm_ctx(tfm);
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(ctx->client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_work_data *work_data;
gfp_t gfp;
int ret;
@@ -187,7 +191,7 @@ static int atmel_ecdh_compute_shared_secret(struct kpp_request *req)
work_data->ctx = ctx;
work_data->client = ctx->client;
- ret = atmel_i2c_init_ecdh_cmd(&work_data->cmd, req->src);
+ ret = atmel_i2c_init_ecdh_cmd(&work_data->cmd, req->src, &data->timings);
if (ret)
goto free_work_data;
@@ -278,14 +282,22 @@ static struct kpp_alg atmel_ecdh_nist_p256 = {
static int atmel_ecc_probe(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv;
+ const struct atmel_i2c_of_match_data *data;
int ret;
ret = atmel_i2c_probe(client);
if (ret)
goto done;
- i2c_priv = i2c_get_clientdata(client);
+ data = device_get_match_data(&client->dev);
+ if (!data) {
+ dev_err(&client->dev, "no match data found via OF or ID table\n");
+ ret = -ENODEV;
+ goto done;
+ }
+ i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->data = data;
i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
/* add to client list */
@@ -339,9 +351,19 @@ static void atmel_ecc_remove(struct i2c_client *client)
crypto_unregister_kpp(&atmel_ecdh_nist_p256);
}
+static const struct atmel_i2c_of_match_data atecc508a_match_data = {
+ .timings = {
+ .max_exec_time_ecdh = 58,
+ .max_exec_time_genkey = 115,
+ .max_exec_time_random = 23,
+ .max_exec_time_read = 1,
+ .max_exec_time_write = 42,
+ },
+};
+
static const struct of_device_id atmel_ecc_dt_ids[] = {
- { .compatible = "atmel,atecc508a", },
- { .compatible = "atmel,atecc608b", },
+ { .compatible = "atmel,atecc508a", .data = &atecc508a_match_data, },
+ { .compatible = "atmel,atecc608b", .data = &atecc508a_match_data, },
{ }
};
MODULE_DEVICE_TABLE(of, atmel_ecc_dt_ids);
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index b7ee2ec37531..7fa7cf9ab3c1 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -96,7 +96,8 @@ struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap)
}
EXPORT_SYMBOL(atmel_i2c_client_alloc);
-void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd)
+void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
+ const struct atmel_i2c_max_exec_timings *timings)
{
cmd->word_addr = COMMAND;
cmd->opcode = OPCODE_READ;
@@ -110,12 +111,13 @@ void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd)
atmel_i2c_checksum(cmd);
- cmd->msecs = MAX_EXEC_TIME_READ;
+ cmd->msecs = timings->max_exec_time_read;
cmd->rxsize = READ_RSP_SIZE;
}
EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
-int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr)
+int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
+ const struct atmel_i2c_max_exec_timings *timings)
{
if (addr >= OTP_ZONE_SIZE / 4)
return -EINVAL;
@@ -132,14 +134,15 @@ int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr)
atmel_i2c_checksum(cmd);
- cmd->msecs = MAX_EXEC_TIME_READ;
+ cmd->msecs = timings->max_exec_time_read;
cmd->rxsize = READ_RSP_SIZE;
return 0;
}
EXPORT_SYMBOL(atmel_i2c_init_read_otp_cmd);
-void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd)
+void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
+ const struct atmel_i2c_max_exec_timings *timings)
{
cmd->word_addr = COMMAND;
cmd->opcode = OPCODE_RANDOM;
@@ -149,12 +152,13 @@ void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd)
atmel_i2c_checksum(cmd);
- cmd->msecs = MAX_EXEC_TIME_RANDOM;
+ cmd->msecs = timings->max_exec_time_random;
cmd->rxsize = RANDOM_RSP_SIZE;
}
EXPORT_SYMBOL(atmel_i2c_init_random_cmd);
-void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid)
+void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
+ const struct atmel_i2c_max_exec_timings *timings)
{
cmd->word_addr = COMMAND;
cmd->count = GENKEY_COUNT;
@@ -165,13 +169,14 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid)
atmel_i2c_checksum(cmd);
- cmd->msecs = MAX_EXEC_TIME_GENKEY;
+ cmd->msecs = timings->max_exec_time_genkey;
cmd->rxsize = GENKEY_RSP_SIZE;
}
EXPORT_SYMBOL(atmel_i2c_init_genkey_cmd);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
- struct scatterlist *pubkey)
+ struct scatterlist *pubkey,
+ const struct atmel_i2c_max_exec_timings *timings)
{
size_t copied;
@@ -196,7 +201,7 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
atmel_i2c_checksum(cmd);
- cmd->msecs = MAX_EXEC_TIME_ECDH;
+ cmd->msecs = timings->max_exec_time_ecdh;
cmd->rxsize = ECDH_RSP_SIZE;
return 0;
@@ -363,6 +368,8 @@ static inline size_t atmel_i2c_wake_token_sz(u32 bus_clk_rate)
static int device_sanity_check(struct i2c_client *client)
{
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_cmd *cmd;
int ret;
@@ -370,7 +377,7 @@ static int device_sanity_check(struct i2c_client *client)
if (!cmd)
return -ENOMEM;
- atmel_i2c_init_read_config_cmd(cmd);
+ atmel_i2c_init_read_config_cmd(cmd, &data->timings);
ret = atmel_i2c_send_receive(client, cmd);
if (ret)
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 70579b438256..5224a62c16c9 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -57,6 +57,19 @@ struct atmel_i2c_cmd {
u16 rxsize;
} __packed;
+struct atmel_i2c_max_exec_timings {
+ unsigned int max_exec_time_genkey;
+ unsigned int max_exec_time_ecdh;
+ unsigned int max_exec_time_random;
+ unsigned int max_exec_time_read;
+ unsigned int max_exec_time_write;
+};
+
+struct atmel_i2c_of_match_data {
+ const unsigned short *legacy_hwrng;
+ struct atmel_i2c_max_exec_timings timings;
+};
+
/* Status/Error codes */
#define STATUS_SIZE 0x04
#define STATUS_NOERR 0x00
@@ -88,12 +101,6 @@ struct atmel_i2c_cmd {
/* Wake Low duration */
#define TWLO_USEC 60
-/* Command execution time (milliseconds) */
-#define MAX_EXEC_TIME_ECDH 58
-#define MAX_EXEC_TIME_GENKEY 115
-#define MAX_EXEC_TIME_READ 1
-#define MAX_EXEC_TIME_RANDOM 50
-
/* Command opcode */
#define OPCODE_ECDH 0x43
#define OPCODE_GENKEY 0x40
@@ -135,6 +142,7 @@ extern struct atmel_i2c_client_mgmt atmel_i2c_mgmt;
* @tfm_count : number of active crypto transformations on i2c client
* @hwrng : hold the hardware generated rng
* @caps : feature capability of the particular driver
+ * @data : preinitialized driver data
*
* Reads and writes from/to the i2c client are sequential. The first byte
* transmitted to the device is treated as the byte size. Any attempt to send
@@ -152,6 +160,7 @@ struct atmel_i2c_client_priv {
atomic_t tfm_count ____cacheline_aligned;
struct hwrng hwrng;
u32 caps;
+ const struct atmel_i2c_of_match_data *data;
};
/**
@@ -189,12 +198,17 @@ void atmel_i2c_flush_queue(void);
int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd);
-void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd);
-int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr);
-void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd);
-void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
+void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
+ const struct atmel_i2c_max_exec_timings *timings);
+int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
+ const struct atmel_i2c_max_exec_timings *timings);
+void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
+ const struct atmel_i2c_max_exec_timings *timings);
+void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
+ const struct atmel_i2c_max_exec_timings *timings);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
- struct scatterlist *pubkey);
+ struct scatterlist *pubkey,
+ const struct atmel_i2c_max_exec_timings *timings);
struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index ab758c9cd410..febf9891b167 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -40,14 +40,15 @@ static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
atomic_dec(&i2c_priv->tfm_count);
}
-static int atmel_sha204a_rng_read_nonblocking(struct hwrng *rng, void *data,
+static int atmel_sha204a_rng_read_nonblocking(struct hwrng *rng, void *buf,
size_t max)
{
- struct atmel_i2c_client_priv *i2c_priv;
+ struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
+ struct atmel_i2c_client_priv,
+ hwrng);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_work_data *work_data;
- i2c_priv = container_of(rng, struct atmel_i2c_client_priv, hwrng);
-
/* keep maximum 1 asynchronous read in flight at any time */
if (!atomic_add_unless(&i2c_priv->tfm_count, 1, 1))
return 0;
@@ -55,7 +56,7 @@ static int atmel_sha204a_rng_read_nonblocking(struct hwrng *rng, void *data,
if (rng->priv) {
work_data = (struct atmel_i2c_work_data *)rng->priv;
max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(data, &work_data->cmd.data[RSP_DATA_IDX], max);
+ memcpy(buf, &work_data->cmd.data[RSP_DATA_IDX], max);
rng->priv = 0;
} else {
work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
@@ -69,42 +70,45 @@ static int atmel_sha204a_rng_read_nonblocking(struct hwrng *rng, void *data,
max = 0;
}
- atmel_i2c_init_random_cmd(&work_data->cmd);
+ atmel_i2c_init_random_cmd(&work_data->cmd, &data->timings);
atmel_i2c_enqueue(work_data, atmel_sha204a_rng_done, rng);
return max;
}
-static int atmel_sha204a_rng_read(struct hwrng *rng, void *data, size_t max,
+static int atmel_sha204a_rng_read(struct hwrng *rng, void *buf, size_t max,
bool wait)
{
- struct atmel_i2c_client_priv *i2c_priv;
+ struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
+ struct atmel_i2c_client_priv,
+ hwrng);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_cmd cmd;
int ret;
if (!wait)
- return atmel_sha204a_rng_read_nonblocking(rng, data, max);
-
- i2c_priv = container_of(rng, struct atmel_i2c_client_priv, hwrng);
+ return atmel_sha204a_rng_read_nonblocking(rng, buf, max);
- atmel_i2c_init_random_cmd(&cmd);
+ atmel_i2c_init_random_cmd(&cmd, &data->timings);
ret = atmel_i2c_send_receive(i2c_priv->client, &cmd);
if (ret)
return ret;
max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(data, &cmd.data[RSP_DATA_IDX], max);
+ memcpy(buf, &cmd.data[RSP_DATA_IDX], max);
return max;
}
static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
{
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
struct atmel_i2c_cmd cmd;
int ret;
- ret = atmel_i2c_init_read_otp_cmd(&cmd, addr);
+ ret = atmel_i2c_init_read_otp_cmd(&cmd, addr, &data->timings);
if (ret < 0) {
dev_err(&client->dev, "failed, invalid otp address %04X\n",
addr);
@@ -164,6 +168,7 @@ static const struct attribute_group atmel_sha204a_groups = {
static int atmel_sha204a_probe(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv;
+ const struct atmel_i2c_of_match_data *data;
const unsigned short *quality;
int ret;
@@ -171,8 +176,15 @@ static int atmel_sha204a_probe(struct i2c_client *client)
if (ret)
goto done;
- i2c_priv = i2c_get_clientdata(client);
+ data = device_get_match_data(&client->dev);
+ if (!data) {
+ dev_err(&client->dev, "no match data found via OF or ID table\n");
+ ret = -ENODEV;
+ goto done;
+ }
+ i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->data = data;
i2c_priv->caps = 0;
/* add to client list */
@@ -187,7 +199,7 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv->hwrng.name = dev_name(&client->dev);
i2c_priv->hwrng.read = atmel_sha204a_rng_read;
- quality = i2c_get_match_data(client);
+ quality = i2c_priv->data->legacy_hwrng;
if (quality)
i2c_priv->hwrng.quality = *quality;
@@ -227,15 +239,34 @@ static void atmel_sha204a_remove(struct i2c_client *client)
kfree((void *)i2c_priv->hwrng.priv);
}
+static const struct atmel_i2c_of_match_data atsha204_match_data = {
+ .timings = {
+ .max_exec_time_genkey = 43,
+ .max_exec_time_random = 50,
+ .max_exec_time_read = 4,
+ .max_exec_time_write = 42,
+ },
+ .legacy_hwrng = &atsha204_quality,
+};
+
+static const struct atmel_i2c_of_match_data atsha204a_match_data = {
+ .timings = {
+ .max_exec_time_genkey = 43,
+ .max_exec_time_random = 50,
+ .max_exec_time_read = 4,
+ .max_exec_time_write = 42,
+ },
+};
+
static const struct of_device_id atmel_sha204a_dt_ids[] __maybe_unused = {
- { .compatible = "atmel,atsha204", .data = &atsha204_quality },
- { .compatible = "atmel,atsha204a", },
+ { .compatible = "atmel,atsha204", .data = &atsha204_match_data, },
+ { .compatible = "atmel,atsha204a", .data = &atsha204a_match_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
static const struct i2c_device_id atmel_sha204a_id[] = {
- { "atsha204", (kernel_ulong_t)&atsha204_quality },
+ { "atsha204" },
{ "atsha204a" },
{ /* sentinel */ }
};
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/12] crypto: atmel - move RNG support into common i2c core
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (3 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 04/12] crypto: atmel - add per-device timing and match-data driven configuration Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 06/12] crypto: atmel - move EEPROM access " Lothar Rubusch
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Move the hardware RNG implementation from atmel-sha204a into the
shared atmel-i2c core.
The ATSHA204(A) and ATECC devices provide compatible RANDOM commands
through the common Atmel I2C interface. Consolidate the RNG handling in
the core driver and provide a shared atmel_i2c_register_rng() helper for
registering the hwrng device.
This removes duplicated RNG code from atmel-sha204a and enables RNG
support for other compatible Atmel devices, including the ECC family.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 12 ++++
drivers/crypto/atmel-i2c.c | 93 ++++++++++++++++++++++++++
drivers/crypto/atmel-i2c.h | 4 +-
drivers/crypto/atmel-sha204a.c | 115 +++++----------------------------
4 files changed, 123 insertions(+), 101 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 7793f7b4e97e..67fa5975fa7f 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -306,6 +306,13 @@ static int atmel_ecc_probe(struct i2c_client *client)
&atmel_i2c_mgmt.i2c_client_list);
spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ /* register rng */
+ ret = atmel_i2c_register_rng(i2c_priv, &client->dev);
+ if (ret) {
+ dev_err(&client->dev, "failed to register hw_random\n");
+ goto err_list_del;
+ }
+
/* register algorithms */
ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
if (ret) {
@@ -349,6 +356,11 @@ static void atmel_ecc_remove(struct i2c_client *client)
atmel_i2c_flush_queue();
crypto_unregister_kpp(&atmel_ecdh_nist_p256);
+
+ if (i2c_priv->hwrng.priv) {
+ kfree((void *)i2c_priv->hwrng.priv);
+ i2c_priv->hwrng.priv = 0;
+ }
}
static const struct atmel_i2c_of_match_data atecc508a_match_data = {
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 7fa7cf9ab3c1..d451017171d8 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -208,6 +208,99 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
}
EXPORT_SYMBOL(atmel_i2c_init_ecdh_cmd);
+static void atmel_i2c_rng_done(struct atmel_i2c_work_data *work_data,
+ void *areq, int status)
+{
+ struct atmel_i2c_client_priv *i2c_priv = work_data->ctx;
+ struct hwrng *rng = areq;
+
+ if (status)
+ dev_warn_ratelimited(&i2c_priv->client->dev,
+ "i2c transaction failed (%d)\n",
+ status);
+
+ rng->priv = (unsigned long)work_data;
+ atomic_dec(&i2c_priv->tfm_count);
+}
+
+static int atmel_i2c_rng_read_nonblocking(struct hwrng *rng, void *buf,
+ size_t max)
+{
+ struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
+ struct atmel_i2c_client_priv,
+ hwrng);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_work_data *work_data;
+
+ /* keep maximum 1 asynchronous read in flight at any time */
+ if (!atomic_add_unless(&i2c_priv->tfm_count, 1, 1))
+ return 0;
+
+ if (rng->priv) {
+ work_data = (struct atmel_i2c_work_data *)rng->priv;
+ max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
+ memcpy(buf, &work_data->cmd.data[RSP_DATA_IDX], max);
+ rng->priv = 0;
+ } else {
+ work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
+ if (!work_data) {
+ atomic_dec(&i2c_priv->tfm_count);
+ return -ENOMEM;
+ }
+ work_data->ctx = i2c_priv;
+ work_data->client = i2c_priv->client;
+
+ max = 0;
+ }
+
+ atmel_i2c_init_random_cmd(&work_data->cmd, &data->timings);
+ atmel_i2c_enqueue(work_data, atmel_i2c_rng_done, rng);
+
+ return max;
+}
+
+static int atmel_i2c_rng_read(struct hwrng *rng, void *buf, size_t max,
+ bool wait)
+{
+ struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
+ struct atmel_i2c_client_priv,
+ hwrng);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_cmd cmd;
+ int ret;
+
+ if (!wait)
+ return atmel_i2c_rng_read_nonblocking(rng, buf, max);
+
+ atmel_i2c_init_random_cmd(&cmd, &data->timings);
+
+ ret = atmel_i2c_send_receive(i2c_priv->client, &cmd);
+ if (ret)
+ return ret;
+
+ max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
+ memcpy(buf, &cmd.data[RSP_DATA_IDX], max);
+
+ return max;
+}
+
+int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
+ struct device *dev)
+{
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+
+ memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
+
+ i2c_priv->hwrng.name = dev_name(dev);
+ i2c_priv->hwrng.read = atmel_i2c_rng_read;
+
+ if (data->needs_legacy_hwrng)
+ i2c_priv->hwrng.quality = data->needs_legacy_hwrng;
+
+ return devm_hwrng_register(dev, &i2c_priv->hwrng);
+}
+EXPORT_SYMBOL(atmel_i2c_register_rng);
+
/*
* After wake and after execution of a command, there will be error, status, or
* result bytes in the device's output register that can be retrieved by the
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 5224a62c16c9..5f6c9ff0cf64 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -66,7 +66,7 @@ struct atmel_i2c_max_exec_timings {
};
struct atmel_i2c_of_match_data {
- const unsigned short *legacy_hwrng;
+ const unsigned short needs_legacy_hwrng;
struct atmel_i2c_max_exec_timings timings;
};
@@ -209,6 +209,8 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey,
const struct atmel_i2c_max_exec_timings *timings);
+int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
+ struct device *dev);
struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index febf9891b167..ae24d8fbabf9 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -19,88 +19,6 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
-/*
- * According to review by Bill Cox [1], the ATSHA204 has very low entropy.
- * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
- */
-static const unsigned short atsha204_quality = 1;
-
-static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
- void *areq, int status)
-{
- struct atmel_i2c_client_priv *i2c_priv = work_data->ctx;
- struct hwrng *rng = areq;
-
- if (status)
- dev_warn_ratelimited(&i2c_priv->client->dev,
- "i2c transaction failed (%d)\n",
- status);
-
- rng->priv = (unsigned long)work_data;
- atomic_dec(&i2c_priv->tfm_count);
-}
-
-static int atmel_sha204a_rng_read_nonblocking(struct hwrng *rng, void *buf,
- size_t max)
-{
- struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
- struct atmel_i2c_client_priv,
- hwrng);
- const struct atmel_i2c_of_match_data *data = i2c_priv->data;
- struct atmel_i2c_work_data *work_data;
-
- /* keep maximum 1 asynchronous read in flight at any time */
- if (!atomic_add_unless(&i2c_priv->tfm_count, 1, 1))
- return 0;
-
- if (rng->priv) {
- work_data = (struct atmel_i2c_work_data *)rng->priv;
- max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(buf, &work_data->cmd.data[RSP_DATA_IDX], max);
- rng->priv = 0;
- } else {
- work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
- if (!work_data) {
- atomic_dec(&i2c_priv->tfm_count);
- return -ENOMEM;
- }
- work_data->ctx = i2c_priv;
- work_data->client = i2c_priv->client;
-
- max = 0;
- }
-
- atmel_i2c_init_random_cmd(&work_data->cmd, &data->timings);
- atmel_i2c_enqueue(work_data, atmel_sha204a_rng_done, rng);
-
- return max;
-}
-
-static int atmel_sha204a_rng_read(struct hwrng *rng, void *buf, size_t max,
- bool wait)
-{
- struct atmel_i2c_client_priv *i2c_priv = container_of(rng,
- struct atmel_i2c_client_priv,
- hwrng);
- const struct atmel_i2c_of_match_data *data = i2c_priv->data;
- struct atmel_i2c_cmd cmd;
- int ret;
-
- if (!wait)
- return atmel_sha204a_rng_read_nonblocking(rng, buf, max);
-
- atmel_i2c_init_random_cmd(&cmd, &data->timings);
-
- ret = atmel_i2c_send_receive(i2c_priv->client, &cmd);
- if (ret)
- return ret;
-
- max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(buf, &cmd.data[RSP_DATA_IDX], max);
-
- return max;
-}
-
static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
@@ -169,7 +87,6 @@ static int atmel_sha204a_probe(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv;
const struct atmel_i2c_of_match_data *data;
- const unsigned short *quality;
int ret;
ret = atmel_i2c_probe(client);
@@ -193,25 +110,16 @@ static int atmel_sha204a_probe(struct i2c_client *client)
&atmel_i2c_mgmt.i2c_client_list);
spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
- /* register rng */
- memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
-
- i2c_priv->hwrng.name = dev_name(&client->dev);
- i2c_priv->hwrng.read = atmel_sha204a_rng_read;
-
- quality = i2c_priv->data->legacy_hwrng;
- if (quality)
- i2c_priv->hwrng.quality = *quality;
-
- ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
+ ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
if (ret) {
- dev_warn(&client->dev, "failed to register RNG (%d)\n", ret);
+ dev_err(&client->dev, "failed to register sysfs entry\n");
goto err_list_del;
}
- ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
+ /* register rng */
+ ret = atmel_i2c_register_rng(i2c_priv, &client->dev);
if (ret) {
- dev_err(&client->dev, "failed to register sysfs entry\n");
+ dev_err(&client->dev, "failed to register hw_random\n");
goto err_list_del;
}
@@ -234,9 +142,12 @@ static void atmel_sha204a_remove(struct i2c_client *client)
devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
atmel_i2c_flush_queue();
- sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
+ if (i2c_priv->hwrng.priv) {
+ kfree((void *)i2c_priv->hwrng.priv);
+ i2c_priv->hwrng.priv = 0;
+ }
- kfree((void *)i2c_priv->hwrng.priv);
+ sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
}
static const struct atmel_i2c_of_match_data atsha204_match_data = {
@@ -246,7 +157,11 @@ static const struct atmel_i2c_of_match_data atsha204_match_data = {
.max_exec_time_read = 4,
.max_exec_time_write = 42,
},
- .legacy_hwrng = &atsha204_quality,
+ /*
+ * According to review by Bill Cox [1], the ATSHA204 has very low entropy.
+ * [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
+ */
+ .needs_legacy_hwrng = 1,
};
static const struct atmel_i2c_of_match_data atsha204a_match_data = {
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/12] crypto: atmel - move EEPROM access support into common i2c core
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (4 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 05/12] crypto: atmel - move RNG support into common i2c core Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 07/12] crypto: atmel - expose CONFIG zone through sysfs Lothar Rubusch
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Move EEPROM read support from atmel-sha204a and atmel-ecc into the
shared atmel-i2c core and provide a generic interface for accessing
EEPROM zones on compatible Atmel devices.
Introduce enum atmel_i2c_eeprom_zones together with per-device EEPROM
zone sizing in struct atmel_i2c_of_match_data. Add common helpers for
EEPROM readout and sysfs formatting, and convert existing OTP sysfs
handling to use the shared infrastructure.
This removes duplicated EEPROM access logic from individual drivers and
extends support to ECC devices. The common implementation supports
CONFIG, OTP, and DATA zones using device-specific layout information
supplied via match data tables.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 36 ++++++++
drivers/crypto/atmel-i2c.c | 153 +++++++++++++++++++++++++--------
drivers/crypto/atmel-i2c.h | 30 +++----
drivers/crypto/atmel-sha204a.c | 65 ++++----------
4 files changed, 186 insertions(+), 98 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 67fa5975fa7f..b5f2d44ec74c 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -23,6 +23,22 @@
#include <crypto/kpp.h>
#include "atmel-i2c.h"
+static ssize_t otp_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_OTP_ZONE);
+}
+static DEVICE_ATTR_RO(otp);
+
+static struct attribute *atmel_ecc508a_attrs[] = {
+ &dev_attr_otp.attr,
+ NULL
+};
+
+static const struct attribute_group atmel_ecc508a_groups = {
+ .name = "atecc508a",
+ .attrs = atmel_ecc508a_attrs,
+};
+
/**
* struct atmel_ecdh_ctx - transformation context
* @client : pointer to i2c client device
@@ -306,6 +322,18 @@ static int atmel_ecc_probe(struct i2c_client *client)
&atmel_i2c_mgmt.i2c_client_list);
spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ /* EEPROM read out */
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ ret = -ENODEV;
+ goto err_list_del;
+ }
+
+ ret = sysfs_create_group(&client->dev.kobj, &atmel_ecc508a_groups);
+ if (ret) {
+ dev_err(&client->dev, "failed to register sysfs entry\n");
+ goto err_list_del;
+ }
+
/* register rng */
ret = atmel_i2c_register_rng(i2c_priv, &client->dev);
if (ret) {
@@ -326,6 +354,7 @@ static int atmel_ecc_probe(struct i2c_client *client)
goto done;
err_list_del:
+ sysfs_remove_group(&client->dev.kobj, &atmel_ecc508a_groups);
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_del(&i2c_priv->i2c_client_list_node);
spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
@@ -361,6 +390,8 @@ static void atmel_ecc_remove(struct i2c_client *client)
kfree((void *)i2c_priv->hwrng.priv);
i2c_priv->hwrng.priv = 0;
}
+
+ sysfs_remove_group(&client->dev.kobj, &atmel_ecc508a_groups);
}
static const struct atmel_i2c_of_match_data atecc508a_match_data = {
@@ -371,6 +402,11 @@ static const struct atmel_i2c_of_match_data atecc508a_match_data = {
.max_exec_time_read = 1,
.max_exec_time_write = 42,
},
+ .eeprom_zone_size = {
+ [ATMEL_EEPROM_CONFIG_ZONE] = 128,
+ [ATMEL_EEPROM_OTP_ZONE] = 64,
+ [ATMEL_EEPROM_DATA_ZONE] = 1208
+ },
};
static const struct of_device_id atmel_ecc_dt_ids[] = {
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index d451017171d8..26863573a10f 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -21,6 +21,15 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
+#define ATMEL_I2C_COMMAND 0x03 /* packet function */
+
+/* Command opcode */
+#define ATMEL_I2C_OPCODE_ECDH 0x43
+#define ATMEL_I2C_OPCODE_GENKEY 0x40
+#define ATMEL_I2C_OPCODE_READ 0x02
+#define ATMEL_I2C_OPCODE_RANDOM 0x1b
+#define ATMEL_I2C_OPCODE_WRITE 0x12
+
struct atmel_i2c_client_mgmt atmel_i2c_mgmt = {
.i2c_list_lock = __SPIN_LOCK_UNLOCKED(atmel_i2c_mgmt.i2c_list_lock),
.i2c_client_list = LIST_HEAD_INIT(atmel_i2c_mgmt.i2c_client_list),
@@ -96,56 +105,55 @@ struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap)
}
EXPORT_SYMBOL(atmel_i2c_client_alloc);
-void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
- const struct atmel_i2c_max_exec_timings *timings)
+static int atmel_i2c_init_read_eeprom_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
+ enum atmel_i2c_eeprom_zones zone,
+ const struct atmel_i2c_of_match_data *data)
{
- cmd->word_addr = COMMAND;
- cmd->opcode = OPCODE_READ;
- /*
- * Read the word from Configuration zone that contains the lock bytes
- * (UserExtra, Selector, LockValue, LockConfig).
- */
- cmd->param1 = CONFIGURATION_ZONE;
- cmd->param2 = cpu_to_le16(DEVICE_LOCK_ADDR);
- cmd->count = READ_COUNT;
+ const struct atmel_i2c_max_exec_timings *timings = &data->timings;
+ size_t zone_size = data->eeprom_zone_size[zone];
+
+ if (addr > zone_size)
+ return -EINVAL;
+
+ cmd->word_addr = ATMEL_I2C_COMMAND;
+ cmd->opcode = ATMEL_I2C_OPCODE_READ;
+ cmd->param1 = zone;
+ cmd->param2 = cpu_to_le16(addr);
+ cmd->count = ATMEL_I2C_READ_COUNT;
atmel_i2c_checksum(cmd);
cmd->msecs = timings->max_exec_time_read;
- cmd->rxsize = READ_RSP_SIZE;
+ cmd->rxsize = ATMEL_I2C_READ_RSP_SIZE;
+
+ return 0;
}
-EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
-int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
- const struct atmel_i2c_max_exec_timings *timings)
+void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
+ const struct atmel_i2c_max_exec_timings *timings)
{
- if (addr >= OTP_ZONE_SIZE / 4)
- return -EINVAL;
-
- cmd->word_addr = COMMAND;
- cmd->opcode = OPCODE_READ;
+ cmd->word_addr = ATMEL_I2C_COMMAND;
+ cmd->opcode = ATMEL_I2C_OPCODE_READ;
/*
- * Read the word from OTP zone that may contain e.g. serial
- * numbers or similar if persistently pre-initialized and locked
+ * Read the word from Configuration zone that contains the lock bytes
+ * (UserExtra, Selector, LockValue, LockConfig).
*/
- cmd->param1 = OTP_ZONE;
- cmd->param2 = cpu_to_le16(addr);
- cmd->count = READ_COUNT;
+ cmd->param1 = CONFIGURATION_ZONE;
+ cmd->param2 = cpu_to_le16(DEVICE_LOCK_ADDR);
+ cmd->count = ATMEL_I2C_READ_COUNT;
atmel_i2c_checksum(cmd);
cmd->msecs = timings->max_exec_time_read;
- cmd->rxsize = READ_RSP_SIZE;
-
- return 0;
+ cmd->rxsize = ATMEL_I2C_READ_RSP_SIZE;
}
-EXPORT_SYMBOL(atmel_i2c_init_read_otp_cmd);
+EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
const struct atmel_i2c_max_exec_timings *timings)
{
- cmd->word_addr = COMMAND;
- cmd->opcode = OPCODE_RANDOM;
+ cmd->word_addr = ATMEL_I2C_COMMAND;
+ cmd->opcode = ATMEL_I2C_OPCODE_RANDOM;
cmd->param1 = 0;
cmd->param2 = 0;
cmd->count = RANDOM_COUNT;
@@ -160,9 +168,9 @@ EXPORT_SYMBOL(atmel_i2c_init_random_cmd);
void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
const struct atmel_i2c_max_exec_timings *timings)
{
- cmd->word_addr = COMMAND;
+ cmd->word_addr = ATMEL_I2C_COMMAND;
cmd->count = GENKEY_COUNT;
- cmd->opcode = OPCODE_GENKEY;
+ cmd->opcode = ATMEL_I2C_OPCODE_GENKEY;
cmd->param1 = GENKEY_MODE_PRIVATE;
/* a random private key will be generated and stored in slot keyID */
cmd->param2 = cpu_to_le16(keyid);
@@ -180,9 +188,9 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
{
size_t copied;
- cmd->word_addr = COMMAND;
+ cmd->word_addr = ATMEL_I2C_COMMAND;
cmd->count = ECDH_COUNT;
- cmd->opcode = OPCODE_ECDH;
+ cmd->opcode = ATMEL_I2C_OPCODE_ECDH;
cmd->param1 = ECDH_PREFIX_MODE;
/* private key slot */
cmd->param2 = cpu_to_le16(DATA_SLOT_2);
@@ -301,6 +309,81 @@ int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
}
EXPORT_SYMBOL(atmel_i2c_register_rng);
+static int atmel_i2c_eeprom_read(struct i2c_client *client, u16 addr,
+ enum atmel_i2c_eeprom_zones zone, u8 *buf)
+{
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_cmd *cmd;
+ int ret = -1;
+
+ cmd = kmalloc_obj(*cmd);
+ if (!cmd)
+ return -ENOMEM;
+
+ ret = atmel_i2c_init_read_eeprom_cmd(cmd, addr, zone, data);
+ if (ret < 0) {
+ dev_err(&client->dev, "failed, invalid eeprom address %04X\n",
+ addr);
+ goto err;
+ }
+
+ ret = atmel_i2c_send_receive(client, cmd);
+ if (ret)
+ goto err;
+
+ if (cmd->data[0] == 0xff) {
+ dev_err(&client->dev, "failed, device not ready\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ memcpy(buf, cmd->data + RSP_DATA_IDX, 4);
+
+err:
+ kfree(cmd);
+ return ret;
+}
+
+ssize_t atmel_i2c_eeprom_display(struct device *dev,
+ struct device_attribute *attr,
+ char *buf,
+ enum atmel_i2c_eeprom_zones zone)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ const struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ const size_t *eeprom = data->eeprom_zone_size;
+ u16 block_addr;
+ u8 *eeprom_buf;
+ ssize_t len = 0;
+ int i, ret = 0;
+
+ eeprom_buf = kcalloc(eeprom[zone], sizeof(*eeprom_buf), GFP_KERNEL);
+ if (!eeprom_buf)
+ return -ENOMEM;
+
+ for (block_addr = 0; block_addr < eeprom[zone] / 4; block_addr++) {
+ ret = atmel_i2c_eeprom_read(client, block_addr, zone,
+ eeprom_buf + block_addr * 4);
+ if (ret < 0) {
+ dev_err(dev, "failed to read %s zone\n",
+ zone == ATMEL_EEPROM_CONFIG_ZONE ? "CONFIG"
+ : (zone == ATMEL_EEPROM_OTP_ZONE ? "OTP" : "DATA"));
+ goto err;
+ }
+ }
+
+ for (i = 0; i < eeprom[zone]; i++)
+ len += sysfs_emit_at(buf, len, "%02X", eeprom_buf[i]);
+ len += sysfs_emit_at(buf, len, "\n");
+ ret = len;
+err:
+ kfree(eeprom_buf);
+ return ret;
+}
+EXPORT_SYMBOL(atmel_i2c_eeprom_display);
+
/*
* After wake and after execution of a command, there will be error, status, or
* result bytes in the device's output register that can be retrieved by the
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 5f6c9ff0cf64..e30e0c417de2 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -12,7 +12,6 @@
#define ATMEL_ECC_PRIORITY 300
-#define COMMAND 0x03 /* packet function */
#define SLEEP_TOKEN 0x01
#define WAKE_TOKEN_MAX_SIZE 8
@@ -30,7 +29,7 @@
#define ECDH_RSP_SIZE (32 + CMD_OVERHEAD_SIZE)
#define GENKEY_RSP_SIZE (ATMEL_ECC_PUBKEY_SIZE + \
CMD_OVERHEAD_SIZE)
-#define READ_RSP_SIZE (4 + CMD_OVERHEAD_SIZE)
+#define ATMEL_I2C_READ_RSP_SIZE (4 + CMD_OVERHEAD_SIZE)
#define RANDOM_RSP_SIZE (32 + CMD_OVERHEAD_SIZE)
#define MAX_RSP_SIZE GENKEY_RSP_SIZE
@@ -57,6 +56,13 @@ struct atmel_i2c_cmd {
u16 rxsize;
} __packed;
+/* Definitions for eeprom organization */
+enum atmel_i2c_eeprom_zones {
+ ATMEL_EEPROM_CONFIG_ZONE = 0,
+ ATMEL_EEPROM_OTP_ZONE = 1,
+ ATMEL_EEPROM_DATA_ZONE = 2,
+};
+
struct atmel_i2c_max_exec_timings {
unsigned int max_exec_time_genkey;
unsigned int max_exec_time_ecdh;
@@ -68,6 +74,7 @@ struct atmel_i2c_max_exec_timings {
struct atmel_i2c_of_match_data {
const unsigned short needs_legacy_hwrng;
struct atmel_i2c_max_exec_timings timings;
+ size_t eeprom_zone_size[3]; /* all atmel devices have three zones */
};
/* Status/Error codes */
@@ -77,10 +84,6 @@ struct atmel_i2c_of_match_data {
/* Definitions for eeprom organization */
#define CONFIGURATION_ZONE 0
-#define OTP_ZONE 1
-
-/* Definitions for eeprom zone sizes */
-#define OTP_ZONE_SIZE 64
/* Definitions for Indexes common to all commands */
#define RSP_DATA_IDX 1 /* buffer index of data in response */
@@ -101,14 +104,8 @@ struct atmel_i2c_of_match_data {
/* Wake Low duration */
#define TWLO_USEC 60
-/* Command opcode */
-#define OPCODE_ECDH 0x43
-#define OPCODE_GENKEY 0x40
-#define OPCODE_READ 0x02
-#define OPCODE_RANDOM 0x1b
-
/* Definitions for the READ Command */
-#define READ_COUNT 7
+#define ATMEL_I2C_READ_COUNT 7
/* Definitions for the RANDOM Command */
#define RANDOM_COUNT 7
@@ -200,8 +197,6 @@ int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
const struct atmel_i2c_max_exec_timings *timings);
-int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
- const struct atmel_i2c_max_exec_timings *timings);
void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
const struct atmel_i2c_max_exec_timings *timings);
void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
@@ -212,6 +207,11 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
struct device *dev);
+ssize_t atmel_i2c_eeprom_display(struct device *dev,
+ struct device_attribute *attr,
+ char *buf,
+ enum atmel_i2c_eeprom_zones zone);
+
struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index ae24d8fbabf9..4f10e826e675 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -19,57 +19,10 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
-static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
-{
- struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
- const struct atmel_i2c_of_match_data *data = i2c_priv->data;
- struct atmel_i2c_cmd cmd;
- int ret;
-
- ret = atmel_i2c_init_read_otp_cmd(&cmd, addr, &data->timings);
- if (ret < 0) {
- dev_err(&client->dev, "failed, invalid otp address %04X\n",
- addr);
- return ret;
- }
-
- ret = atmel_i2c_send_receive(client, &cmd);
- if (ret < 0) {
- dev_err(&client->dev, "failed to read otp at %04X\n", addr);
- return ret;
- }
-
- if (cmd.data[0] == 0xff) {
- dev_err(&client->dev, "failed, device not ready\n");
- return -EIO;
- }
-
- memcpy(otp, cmd.data+1, 4);
-
- return ret;
-}
-
static ssize_t otp_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- u16 addr;
- u8 otp[OTP_ZONE_SIZE];
- struct i2c_client *client = to_i2c_client(dev);
- ssize_t len = 0;
- int i, ret;
-
- for (addr = 0; addr < OTP_ZONE_SIZE / 4; addr++) {
- ret = atmel_sha204a_otp_read(client, addr, otp + addr * 4);
- if (ret < 0) {
- dev_err(dev, "failed to read otp zone\n");
- return ret;
- }
- }
-
- for (i = 0; i < OTP_ZONE_SIZE; i++)
- len += sysfs_emit_at(buf, len, "%02X", otp[i]);
- len += sysfs_emit_at(buf, len, "\n");
- return len;
+ return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_OTP_ZONE);
}
static DEVICE_ATTR_RO(otp);
@@ -110,6 +63,12 @@ static int atmel_sha204a_probe(struct i2c_client *client)
&atmel_i2c_mgmt.i2c_client_list);
spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ /* EEPROM read out */
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ ret = -ENODEV;
+ goto err_list_del;
+ }
+
ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
if (ret) {
dev_err(&client->dev, "failed to register sysfs entry\n");
@@ -157,6 +116,11 @@ static const struct atmel_i2c_of_match_data atsha204_match_data = {
.max_exec_time_read = 4,
.max_exec_time_write = 42,
},
+ .eeprom_zone_size = {
+ [ATMEL_EEPROM_CONFIG_ZONE] = 88,
+ [ATMEL_EEPROM_OTP_ZONE] = 64,
+ [ATMEL_EEPROM_DATA_ZONE] = 512
+ },
/*
* According to review by Bill Cox [1], the ATSHA204 has very low entropy.
* [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
@@ -171,6 +135,11 @@ static const struct atmel_i2c_of_match_data atsha204a_match_data = {
.max_exec_time_read = 4,
.max_exec_time_write = 42,
},
+ .eeprom_zone_size = {
+ [ATMEL_EEPROM_CONFIG_ZONE] = 88,
+ [ATMEL_EEPROM_OTP_ZONE] = 64,
+ [ATMEL_EEPROM_DATA_ZONE] = 512
+ },
};
static const struct of_device_id atmel_sha204a_dt_ids[] __maybe_unused = {
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/12] crypto: atmel - expose CONFIG zone through sysfs
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (5 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 06/12] crypto: atmel - move EEPROM access " Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 08/12] crypto: atmel - move device sanity check to core driver Lothar Rubusch
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Expose the CONFIG EEPROM zone through a read-only sysfs attribute for
Atmel I2C crypto devices.
The CONFIG zone contains device configuration state, including slot
configuration and lock status, which is useful for debugging and
verifying provisioning state.
Reuse the generic EEPROM display helper provided by the Atmel I2C core
driver to expose the CONFIG zone for both SHA204A and ECC devices.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 7 +++++++
drivers/crypto/atmel-sha204a.c | 7 +++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index b5f2d44ec74c..f08fdf284b60 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -23,6 +23,12 @@
#include <crypto/kpp.h>
#include "atmel-i2c.h"
+static ssize_t config_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_CONFIG_ZONE);
+}
+static DEVICE_ATTR_ADMIN_RO(config);
+
static ssize_t otp_show(struct device *dev, struct device_attribute *attr, char *buf)
{
return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_OTP_ZONE);
@@ -30,6 +36,7 @@ static ssize_t otp_show(struct device *dev, struct device_attribute *attr, char
static DEVICE_ATTR_RO(otp);
static struct attribute *atmel_ecc508a_attrs[] = {
+ &dev_attr_config.attr,
&dev_attr_otp.attr,
NULL
};
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 4f10e826e675..341554b7b7a2 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -19,6 +19,12 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
+static ssize_t config_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_CONFIG_ZONE);
+}
+static DEVICE_ATTR_ADMIN_RO(config);
+
static ssize_t otp_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -27,6 +33,7 @@ static ssize_t otp_show(struct device *dev,
static DEVICE_ATTR_RO(otp);
static struct attribute *atmel_sha204a_attrs[] = {
+ &dev_attr_config.attr,
&dev_attr_otp.attr,
NULL
};
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/12] crypto: atmel - move device sanity check to core driver
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (6 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 07/12] crypto: atmel - expose CONFIG zone through sysfs Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 09/12] crypto: atmel - check client data in remove callbacks Lothar Rubusch
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Move the device sanity check implementation into the shared Atmel
I2C core driver and reuse the generic EEPROM access helpers for
reading the CONFIG zone lock state.
This removes duplicate CONFIG zone handling and consolidates common
response index and lock state definitions under the Atmel I2C core
namespace.
Update both SHA204A and ECC drivers to invoke the shared sanity
check helper during probe.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 10 ++++++--
drivers/crypto/atmel-i2c.c | 43 ++++++++++++----------------------
drivers/crypto/atmel-i2c.h | 14 +++--------
drivers/crypto/atmel-sha204a.c | 6 +++++
4 files changed, 32 insertions(+), 41 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index f08fdf284b60..f6d1a9694d63 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -81,7 +81,7 @@ static void atmel_ecdh_done(struct atmel_i2c_work_data *work_data, void *areq,
/* copy the shared secret */
copied = sg_copy_from_buffer(req->dst, sg_nents_for_len(req->dst, n_sz),
- &cmd->data[RSP_DATA_IDX], n_sz);
+ &cmd->data[ATMEL_I2C_RSP_DATA_IDX], n_sz);
if (copied != n_sz)
status = -EINVAL;
@@ -144,7 +144,7 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
goto free_public_key;
/* save the public key */
- memcpy(public_key, &cmd->data[RSP_DATA_IDX], ATMEL_ECC_PUBKEY_SIZE);
+ memcpy(public_key, &cmd->data[ATMEL_I2C_RSP_DATA_IDX], ATMEL_ECC_PUBKEY_SIZE);
ctx->public_key = public_key;
kfree(cmd);
@@ -323,6 +323,12 @@ static int atmel_ecc_probe(struct i2c_client *client)
i2c_priv->data = data;
i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
+ ret = atmel_i2c_device_sanity_check(client);
+ if (ret) {
+ dev_err(&client->dev, "failed to read EEPROM, is hardware attached?\n");
+ goto done;
+ }
+
/* add to client list */
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 26863573a10f..50b6bce478d2 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -23,6 +23,11 @@
#define ATMEL_I2C_COMMAND 0x03 /* packet function */
+/* Definitions for the device lock state */
+#define ATMEL_I2C_DEVICE_LOCK_ADDR 0x15
+#define ATMEL_I2C_LOCK_VALUE_IDX (ATMEL_I2C_RSP_DATA_IDX + 2)
+#define ATMEL_I2C_LOCK_CONFIG_IDX (ATMEL_I2C_RSP_DATA_IDX + 3)
+
/* Command opcode */
#define ATMEL_I2C_OPCODE_ECDH 0x43
#define ATMEL_I2C_OPCODE_GENKEY 0x40
@@ -129,26 +134,6 @@ static int atmel_i2c_init_read_eeprom_cmd(struct atmel_i2c_cmd *cmd, u16 addr,
return 0;
}
-void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
- const struct atmel_i2c_max_exec_timings *timings)
-{
- cmd->word_addr = ATMEL_I2C_COMMAND;
- cmd->opcode = ATMEL_I2C_OPCODE_READ;
- /*
- * Read the word from Configuration zone that contains the lock bytes
- * (UserExtra, Selector, LockValue, LockConfig).
- */
- cmd->param1 = CONFIGURATION_ZONE;
- cmd->param2 = cpu_to_le16(DEVICE_LOCK_ADDR);
- cmd->count = ATMEL_I2C_READ_COUNT;
-
- atmel_i2c_checksum(cmd);
-
- cmd->msecs = timings->max_exec_time_read;
- cmd->rxsize = ATMEL_I2C_READ_RSP_SIZE;
-}
-EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
-
void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
const struct atmel_i2c_max_exec_timings *timings)
{
@@ -247,7 +232,7 @@ static int atmel_i2c_rng_read_nonblocking(struct hwrng *rng, void *buf,
if (rng->priv) {
work_data = (struct atmel_i2c_work_data *)rng->priv;
max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(buf, &work_data->cmd.data[RSP_DATA_IDX], max);
+ memcpy(buf, &work_data->cmd.data[ATMEL_I2C_RSP_DATA_IDX], max);
rng->priv = 0;
} else {
work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
@@ -287,7 +272,7 @@ static int atmel_i2c_rng_read(struct hwrng *rng, void *buf, size_t max,
return ret;
max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
- memcpy(buf, &cmd.data[RSP_DATA_IDX], max);
+ memcpy(buf, &cmd.data[ATMEL_I2C_RSP_DATA_IDX], max);
return max;
}
@@ -338,7 +323,7 @@ static int atmel_i2c_eeprom_read(struct i2c_client *client, u16 addr,
goto err;
}
- memcpy(buf, cmd->data + RSP_DATA_IDX, 4);
+ memcpy(buf, cmd->data + ATMEL_I2C_RSP_DATA_IDX, 4);
err:
kfree(cmd);
@@ -542,7 +527,7 @@ static inline size_t atmel_i2c_wake_token_sz(u32 bus_clk_rate)
return DIV_ROUND_UP(no_of_bits, 8);
}
-static int device_sanity_check(struct i2c_client *client)
+int atmel_i2c_device_sanity_check(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
const struct atmel_i2c_of_match_data *data = i2c_priv->data;
@@ -553,7 +538,8 @@ static int device_sanity_check(struct i2c_client *client)
if (!cmd)
return -ENOMEM;
- atmel_i2c_init_read_config_cmd(cmd, &data->timings);
+ atmel_i2c_init_read_eeprom_cmd(cmd, ATMEL_I2C_DEVICE_LOCK_ADDR,
+ ATMEL_EEPROM_CONFIG_ZONE, data);
ret = atmel_i2c_send_receive(client, cmd);
if (ret)
@@ -565,8 +551,8 @@ static int device_sanity_check(struct i2c_client *client)
* Failure to lock these zones may permit modification of any secret
* keys and may lead to other security problems.
*/
- if (cmd->data[LOCK_CONFIG_IDX] || cmd->data[LOCK_VALUE_IDX]) {
- dev_err(&client->dev, "Configuration or Data and OTP zones are unlocked!\n");
+ if (cmd->data[ATMEL_I2C_LOCK_CONFIG_IDX] || cmd->data[ATMEL_I2C_LOCK_VALUE_IDX]) {
+ dev_err(&client->dev, "Config, Data and OTP zones are unlocked!\n");
ret = -ENOTSUPP;
}
@@ -575,6 +561,7 @@ static int device_sanity_check(struct i2c_client *client)
kfree(cmd);
return ret;
}
+EXPORT_SYMBOL(atmel_i2c_device_sanity_check);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv)
{
@@ -633,7 +620,7 @@ int atmel_i2c_probe(struct i2c_client *client)
i2c_set_clientdata(client, i2c_priv);
- return device_sanity_check(client);
+ return 0;
}
EXPORT_SYMBOL(atmel_i2c_probe);
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index e30e0c417de2..2f76e107340e 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -82,18 +82,10 @@ struct atmel_i2c_of_match_data {
#define STATUS_NOERR 0x00
#define STATUS_WAKE_SUCCESSFUL 0x11
-/* Definitions for eeprom organization */
-#define CONFIGURATION_ZONE 0
-
/* Definitions for Indexes common to all commands */
-#define RSP_DATA_IDX 1 /* buffer index of data in response */
+#define ATMEL_I2C_RSP_DATA_IDX 1 /* buffer index of data in response */
#define DATA_SLOT_2 2 /* used for ECDH private key */
-/* Definitions for the device lock state */
-#define DEVICE_LOCK_ADDR 0x15
-#define LOCK_VALUE_IDX (RSP_DATA_IDX + 2)
-#define LOCK_CONFIG_IDX (RSP_DATA_IDX + 3)
-
/*
* Wake High delay to data communication (microseconds). SDA should be stable
* high for this entire duration.
@@ -195,8 +187,6 @@ void atmel_i2c_flush_queue(void);
int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd);
-void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd,
- const struct atmel_i2c_max_exec_timings *timings);
void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
const struct atmel_i2c_max_exec_timings *timings);
void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
@@ -207,6 +197,8 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
struct device *dev);
+int atmel_i2c_device_sanity_check(struct i2c_client *client);
+
ssize_t atmel_i2c_eeprom_display(struct device *dev,
struct device_attribute *attr,
char *buf,
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 341554b7b7a2..88726f6ef87c 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -64,6 +64,12 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv->data = data;
i2c_priv->caps = 0;
+ ret = atmel_i2c_device_sanity_check(client);
+ if (ret) {
+ dev_err(&client->dev, "failed to read EEPROM, is hardware attached?\n");
+ goto done;
+ }
+
/* add to client list */
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/12] crypto: atmel - check client data in remove callbacks
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (7 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 08/12] crypto: atmel - move device sanity check to core driver Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit Lothar Rubusch
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Check the i2c client private data pointer in the remove callbacks of
the Atmel ECC and SHA204A drivers before accessing driver state.
Move sysfs group removal ahead of the NULL check so cleanup can still
proceed even if client data is unavailable. Also downgrade the
busy-device warning in the ECC remove path from dev_emerg() to
dev_warn().
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 20 ++++++--------------
drivers/crypto/atmel-sha204a.c | 7 +++++--
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index f6d1a9694d63..9ad6d42b6eef 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -380,19 +380,13 @@ static void atmel_ecc_remove(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
- /* Return EBUSY if i2c client already allocated. */
- if (atomic_read(&i2c_priv->tfm_count)) {
- /*
- * After we return here, the memory backing the device is freed.
- * That happens no matter what the return value of this function
- * is because in the Linux device model there is no error
- * handling for unbinding a driver.
- * If there is still some action pending, it probably involves
- * accessing the freed memory.
- */
- dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
+ sysfs_remove_group(&client->dev.kobj, &atmel_ecc508a_groups);
+
+ if (!i2c_priv)
return;
- }
+
+ if (atomic_read(&i2c_priv->tfm_count))
+ dev_warn(&client->dev, "Device is busy, remove it anyhow\n");
atmel_i2c_unregister_client(i2c_priv);
atmel_i2c_flush_queue();
@@ -403,8 +397,6 @@ static void atmel_ecc_remove(struct i2c_client *client)
kfree((void *)i2c_priv->hwrng.priv);
i2c_priv->hwrng.priv = 0;
}
-
- sysfs_remove_group(&client->dev.kobj, &atmel_ecc508a_groups);
}
static const struct atmel_i2c_of_match_data atecc508a_match_data = {
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 88726f6ef87c..6a41024ae40d 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -111,6 +111,11 @@ static void atmel_sha204a_remove(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
+
+ if (!i2c_priv)
+ return;
+
devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
atmel_i2c_flush_queue();
@@ -118,8 +123,6 @@ static void atmel_sha204a_remove(struct i2c_client *client)
kfree((void *)i2c_priv->hwrng.priv);
i2c_priv->hwrng.priv = 0;
}
-
- sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
}
static const struct atmel_i2c_of_match_data atsha204_match_data = {
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (8 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 09/12] crypto: atmel - check client data in remove callbacks Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-06-01 9:03 ` Marco Crivellari
2026-05-12 22:43 ` [PATCH 11/12] crypto: atmel - refactor and localize driver constants Lothar Rubusch
` (2 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Update workqueue initialization to use WQ_MEM_RECLAIM instead of
WQ_PERCPU. WQ_MEM_RECLAIM already provides per-CPU execution
semantics via a bound workqueue while also ensuring forward progress
via a rescue thread.
Add a flush_workqueue() call during module exit to ensure all queued
work is completed before destroying the workqueue.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-i2c.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 50b6bce478d2..0ec2d768a763 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -626,12 +626,13 @@ EXPORT_SYMBOL(atmel_i2c_probe);
static int __init atmel_i2c_init(void)
{
- atmel_wq = alloc_workqueue("atmel_wq", WQ_PERCPU, 0);
+ atmel_wq = alloc_workqueue("atmel_wq", WQ_MEM_RECLAIM, 0);
return atmel_wq ? 0 : -ENOMEM;
}
static void __exit atmel_i2c_exit(void)
{
+ flush_workqueue(atmel_wq);
destroy_workqueue(atmel_wq);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/12] crypto: atmel - refactor and localize driver constants
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (9 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 12/12] crypto: atmel - add SHA256 ahash support Lothar Rubusch
2026-05-14 19:51 ` [PATCH 00/12] crypto: atmel - refactor common i2c support and " Thorsten Blum
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
After refactoring the client drivers to use the shared atmel-i2c core,
many constants and definitions no longer need global visibility.
Move command definitions, timing constants, status codes and related
helpers from the public header into the local compile unit of the core
driver where possible.
As part of this cleanup, rename macros and constants to use consistent
ATMEL_I2C_* naming and align them with common kernel driver conventions.
Also replace remaining hardcoded values with named constants throughout
the driver.
This is a preparatory cleanup and does not change functionality.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 4 +-
drivers/crypto/atmel-i2c.c | 115 +++++++++++++++++++++++++------------
drivers/crypto/atmel-i2c.h | 76 +++++++++---------------
3 files changed, 108 insertions(+), 87 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 9ad6d42b6eef..ed8c0ce5562b 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -137,7 +137,7 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
ctx->do_fallback = false;
- atmel_i2c_init_genkey_cmd(cmd, DATA_SLOT_2, &data->timings);
+ atmel_i2c_init_genkey_cmd(cmd, ATMEL_I2C_ECDH_SLOT_DEFAULT, &data->timings);
ret = atmel_i2c_send_receive(ctx->client, cmd);
if (ret)
@@ -296,7 +296,7 @@ static struct kpp_alg atmel_ecdh_nist_p256 = {
.cra_flags = CRYPTO_ALG_NEED_FALLBACK,
.cra_name = "ecdh-nist-p256",
.cra_driver_name = "atmel-ecdh",
- .cra_priority = ATMEL_ECC_PRIORITY,
+ .cra_priority = ATMEL_I2C_PRIORITY,
.cra_module = THIS_MODULE,
.cra_ctxsize = sizeof(struct atmel_ecdh_ctx),
},
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 0ec2d768a763..53aba2f4bedb 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -21,19 +21,62 @@
#include <linux/workqueue.h>
#include "atmel-i2c.h"
-#define ATMEL_I2C_COMMAND 0x03 /* packet function */
+#define ATMEL_I2C_COMMAND 0x03 /* packet function */
+#define ATMEL_I2C_SLEEP_TOKEN 0x01
/* Definitions for the device lock state */
-#define ATMEL_I2C_DEVICE_LOCK_ADDR 0x15
-#define ATMEL_I2C_LOCK_VALUE_IDX (ATMEL_I2C_RSP_DATA_IDX + 2)
-#define ATMEL_I2C_LOCK_CONFIG_IDX (ATMEL_I2C_RSP_DATA_IDX + 3)
+#define ATMEL_I2C_DEVICE_LOCK_ADDR 0x15
+#define ATMEL_I2C_LOCK_VALUE_IDX (ATMEL_I2C_RSP_DATA_IDX + 2)
+#define ATMEL_I2C_LOCK_CONFIG_IDX (ATMEL_I2C_RSP_DATA_IDX + 3)
+
+/* Definitions for the READ Command */
+#define ATMEL_I2C_READ_COUNT ATMEL_I2C_COUNT_OVERHEAD_SIZE
+#define ATMEL_I2C_READ_RSP_SIZE (4 + ATMEL_I2C_RSP_OVERHEAD_SIZE)
+
+/* Definitions for the RANDOM Command */
+#define ATMEL_I2C_RANDOM_COUNT ATMEL_I2C_COUNT_OVERHEAD_SIZE
+#define ATMEL_I2C_RNG_BLOCK_SIZE 32
+#define ATMEL_I2C_RANDOM_RSP_SIZE (ATMEL_I2C_RNG_BLOCK_SIZE + \
+ ATMEL_I2C_RSP_OVERHEAD_SIZE)
+#define ATMEL_I2C_RANDOM_COUNT ATMEL_I2C_COUNT_OVERHEAD_SIZE
+
+/* Definitions for the GenKey Command */
+#define ATMEL_I2C_GENKEY_COUNT ATMEL_I2C_COUNT_OVERHEAD_SIZE
+#define ATMEL_I2C_GENKEY_MODE_PRIVATE 0x04
+
+/* Definitions for the ECDH Command */
+#define ATMEL_I2C_ECDH_COUNT 71
+#define ATMEL_I2C_ECDH_RSP_SIZE (32 + ATMEL_I2C_RSP_OVERHEAD_SIZE)
+#define ATMEL_I2C_ECDH_PREFIX_MODE 0x00
/* Command opcode */
-#define ATMEL_I2C_OPCODE_ECDH 0x43
-#define ATMEL_I2C_OPCODE_GENKEY 0x40
-#define ATMEL_I2C_OPCODE_READ 0x02
-#define ATMEL_I2C_OPCODE_RANDOM 0x1b
-#define ATMEL_I2C_OPCODE_WRITE 0x12
+#define ATMEL_I2C_OPCODE_ECDH 0x43
+#define ATMEL_I2C_OPCODE_GENKEY 0x40
+#define ATMEL_I2C_OPCODE_READ 0x02
+#define ATMEL_I2C_OPCODE_RANDOM 0x1b
+#define ATMEL_I2C_OPCODE_WRITE 0x12
+
+/*
+ * Wake High delay to data communication (microseconds). SDA should be stable
+ * high for this entire duration.
+ */
+#define ATMEL_I2C_TWHI_MIN 1500
+#define ATMEL_I2C_TWHI_MAX 1550
+
+/* Wake Low duration */
+#define ATMEL_I2C_TWLO_USEC 60
+
+/* Status/Error codes */
+enum atmel_i2c_error_codes {
+ ATMEL_STATUS_OK_NOERR = 0x00, /* success */
+ ATMEL_STATUS_CHECKMAC_OR_VERIFY_MISCOMPARE = 0x01,
+ ATMEL_STATUS_PARSE_ERROR = 0x03,
+ ATMEL_STATUS_ECC_FAULT = 0x05,
+ ATMEL_STATUS_EXECUTION_FAULT = 0x0F,
+ ATMEL_STATUS_OK_WAKE_SUCCESSFULL = 0x11, /* success */
+ ATMEL_STATUS_WATCHDOG_EXPIRE = 0xEE,
+ ATMEL_STATUS_CRC_ERROR = 0xFF,
+};
struct atmel_i2c_client_mgmt atmel_i2c_mgmt = {
.i2c_list_lock = __SPIN_LOCK_UNLOCKED(atmel_i2c_mgmt.i2c_list_lock),
@@ -45,12 +88,12 @@ static const struct {
u8 value;
const char *error_text;
} error_list[] = {
- { 0x01, "CheckMac or Verify miscompare" },
- { 0x03, "Parse Error" },
- { 0x05, "ECC Fault" },
- { 0x0F, "Execution Error" },
- { 0xEE, "Watchdog about to expire" },
- { 0xFF, "CRC or other communication error" },
+ { ATMEL_STATUS_CHECKMAC_OR_VERIFY_MISCOMPARE, "CheckMac or Verify miscompare" },
+ { ATMEL_STATUS_PARSE_ERROR, "Parse Error" },
+ { ATMEL_STATUS_ECC_FAULT, "ECC Fault" },
+ { ATMEL_STATUS_EXECUTION_FAULT, "Execution Error" },
+ { ATMEL_STATUS_WATCHDOG_EXPIRE, "Watchdog about to expire" },
+ { ATMEL_STATUS_CRC_ERROR, "CRC or other communication error" },
};
/**
@@ -65,7 +108,7 @@ static const struct {
static void atmel_i2c_checksum(struct atmel_i2c_cmd *cmd)
{
u8 *data = &cmd->count;
- size_t len = cmd->count - CRC_SIZE;
+ size_t len = cmd->count - ATMEL_I2C_CRC_SIZE;
__le16 *__crc16 = (__le16 *)(data + len);
*__crc16 = cpu_to_le16(bitrev16(crc16(0, data, len)));
@@ -141,12 +184,12 @@ void atmel_i2c_init_random_cmd(struct atmel_i2c_cmd *cmd,
cmd->opcode = ATMEL_I2C_OPCODE_RANDOM;
cmd->param1 = 0;
cmd->param2 = 0;
- cmd->count = RANDOM_COUNT;
+ cmd->count = ATMEL_I2C_RANDOM_COUNT;
atmel_i2c_checksum(cmd);
cmd->msecs = timings->max_exec_time_random;
- cmd->rxsize = RANDOM_RSP_SIZE;
+ cmd->rxsize = ATMEL_I2C_RANDOM_RSP_SIZE;
}
EXPORT_SYMBOL(atmel_i2c_init_random_cmd);
@@ -154,16 +197,16 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
const struct atmel_i2c_max_exec_timings *timings)
{
cmd->word_addr = ATMEL_I2C_COMMAND;
- cmd->count = GENKEY_COUNT;
+ cmd->count = ATMEL_I2C_GENKEY_COUNT;
cmd->opcode = ATMEL_I2C_OPCODE_GENKEY;
- cmd->param1 = GENKEY_MODE_PRIVATE;
+ cmd->param1 = ATMEL_I2C_GENKEY_MODE_PRIVATE;
/* a random private key will be generated and stored in slot keyID */
cmd->param2 = cpu_to_le16(keyid);
atmel_i2c_checksum(cmd);
cmd->msecs = timings->max_exec_time_genkey;
- cmd->rxsize = GENKEY_RSP_SIZE;
+ cmd->rxsize = ATMEL_I2C_GENKEY_RSP_SIZE;
}
EXPORT_SYMBOL(atmel_i2c_init_genkey_cmd);
@@ -174,11 +217,11 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
size_t copied;
cmd->word_addr = ATMEL_I2C_COMMAND;
- cmd->count = ECDH_COUNT;
+ cmd->count = ATMEL_I2C_ECDH_COUNT;
cmd->opcode = ATMEL_I2C_OPCODE_ECDH;
- cmd->param1 = ECDH_PREFIX_MODE;
+ cmd->param1 = ATMEL_I2C_ECDH_PREFIX_MODE;
/* private key slot */
- cmd->param2 = cpu_to_le16(DATA_SLOT_2);
+ cmd->param2 = cpu_to_le16(ATMEL_I2C_ECDH_SLOT_DEFAULT);
/*
* The device only supports NIST P256 ECC keys. The public key size will
@@ -195,7 +238,7 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
atmel_i2c_checksum(cmd);
cmd->msecs = timings->max_exec_time_ecdh;
- cmd->rxsize = ECDH_RSP_SIZE;
+ cmd->rxsize = ATMEL_I2C_ECDH_RSP_SIZE;
return 0;
}
@@ -231,7 +274,7 @@ static int atmel_i2c_rng_read_nonblocking(struct hwrng *rng, void *buf,
if (rng->priv) {
work_data = (struct atmel_i2c_work_data *)rng->priv;
- max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
+ max = min(ATMEL_I2C_RANDOM_RSP_SIZE - ATMEL_I2C_RSP_OVERHEAD_SIZE, max);
memcpy(buf, &work_data->cmd.data[ATMEL_I2C_RSP_DATA_IDX], max);
rng->priv = 0;
} else {
@@ -271,7 +314,7 @@ static int atmel_i2c_rng_read(struct hwrng *rng, void *buf, size_t max,
if (ret)
return ret;
- max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
+ max = min(ATMEL_I2C_RANDOM_RSP_SIZE - ATMEL_I2C_RSP_OVERHEAD_SIZE, max);
memcpy(buf, &cmd.data[ATMEL_I2C_RSP_DATA_IDX], max);
return max;
@@ -323,7 +366,7 @@ static int atmel_i2c_eeprom_read(struct i2c_client *client, u16 addr,
goto err;
}
- memcpy(buf, cmd->data + ATMEL_I2C_RSP_DATA_IDX, 4);
+ memcpy(buf, cmd->data + ATMEL_I2C_RSP_DATA_IDX, ATMEL_I2C_STATUS_RSP_SIZE);
err:
kfree(cmd);
@@ -381,10 +424,10 @@ static int atmel_i2c_status(struct device *dev, u8 *status)
int i;
u8 err_id = status[1];
- if (*status != STATUS_SIZE)
+ if (*status != ATMEL_I2C_STATUS_RSP_SIZE)
return 0;
- if (err_id == STATUS_WAKE_SUCCESSFUL || err_id == STATUS_NOERR)
+ if (err_id == ATMEL_STATUS_OK_WAKE_SUCCESSFULL || err_id == ATMEL_STATUS_OK_NOERR)
return 0;
for (i = 0; i < err_list_len; i++)
@@ -403,7 +446,7 @@ static int atmel_i2c_status(struct device *dev, u8 *status)
static int atmel_i2c_wakeup(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
- u8 status[STATUS_RSP_SIZE];
+ u8 status[ATMEL_I2C_STATUS_RSP_SIZE];
int ret;
/*
@@ -418,9 +461,9 @@ static int atmel_i2c_wakeup(struct i2c_client *client)
* Wait to wake the device. Typical execution times for ecdh and genkey
* are around tens of milliseconds. Delta is chosen to 50 microseconds.
*/
- usleep_range(TWHI_MIN, TWHI_MAX);
+ usleep_range(ATMEL_I2C_TWHI_MIN, ATMEL_I2C_TWHI_MAX);
- ret = i2c_master_recv(client, status, STATUS_SIZE);
+ ret = i2c_master_recv(client, status, ATMEL_I2C_STATUS_RSP_SIZE);
if (ret < 0)
return ret;
@@ -429,7 +472,7 @@ static int atmel_i2c_wakeup(struct i2c_client *client)
static int atmel_i2c_sleep(struct i2c_client *client)
{
- u8 sleep = SLEEP_TOKEN;
+ u8 sleep = ATMEL_I2C_SLEEP_TOKEN;
return i2c_master_send(client, &sleep, 1);
}
@@ -461,7 +504,7 @@ int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
goto err;
/* send the command */
- ret = i2c_master_send(client, (u8 *)cmd, cmd->count + WORD_ADDR_SIZE);
+ ret = i2c_master_send(client, (u8 *)cmd, cmd->count + ATMEL_I2C_ADDR_SIZE);
if (ret < 0)
goto err;
@@ -521,7 +564,7 @@ EXPORT_SYMBOL(atmel_i2c_flush_queue);
static inline size_t atmel_i2c_wake_token_sz(u32 bus_clk_rate)
{
- u32 no_of_bits = DIV_ROUND_UP(TWLO_USEC * bus_clk_rate, USEC_PER_SEC);
+ u32 no_of_bits = DIV_ROUND_UP(ATMEL_I2C_TWLO_USEC * bus_clk_rate, USEC_PER_SEC);
/* return the size of the wake_token in bytes */
return DIV_ROUND_UP(no_of_bits, 8);
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 2f76e107340e..20afe2da4f8d 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -10,28 +10,39 @@
#include <linux/hw_random.h>
#include <linux/types.h>
-#define ATMEL_ECC_PRIORITY 300
+#define ATMEL_I2C_PRIORITY 300
-#define SLEEP_TOKEN 0x01
-#define WAKE_TOKEN_MAX_SIZE 8
+#define ATMEL_I2C_WAKE_TOKEN_MAX_SIZE 8
/* Definitions of Data and Command sizes */
-#define WORD_ADDR_SIZE 1
-#define COUNT_SIZE 1
-#define CRC_SIZE 2
-#define CMD_OVERHEAD_SIZE (COUNT_SIZE + CRC_SIZE)
+#define ATMEL_I2C_ADDR_SIZE 1
+#define ATMEL_I2C_OPCODE_SIZE 1
+#define ATMEL_I2C_COUNT_SIZE 1
+#define ATMEL_I2C_PARAM1_SIZE 1
+#define ATMEL_I2C_PARAM2_SIZE 2
+#define ATMEL_I2C_CRC_SIZE 2
+
+#define ATMEL_I2C_RSP_OVERHEAD_SIZE (ATMEL_I2C_COUNT_SIZE + \
+ ATMEL_I2C_CRC_SIZE)
+#define ATMEL_I2C_COUNT_OVERHEAD_SIZE (ATMEL_I2C_OPCODE_SIZE + \
+ ATMEL_I2C_COUNT_SIZE + \
+ ATMEL_I2C_PARAM1_SIZE + \
+ ATMEL_I2C_PARAM2_SIZE + \
+ ATMEL_I2C_CRC_SIZE)
+
+/* Definitions for the status Command */
+#define ATMEL_I2C_STATUS_RSP_SIZE 4
/* size in bytes of the n prime */
#define ATMEL_ECC_NIST_P256_N_SIZE 32
#define ATMEL_ECC_PUBKEY_SIZE (2 * ATMEL_ECC_NIST_P256_N_SIZE)
+#define ATMEL_I2C_GENKEY_RSP_SIZE (ATMEL_ECC_PUBKEY_SIZE + \
+ ATMEL_I2C_RSP_OVERHEAD_SIZE)
+#define ATMEL_I2C_MAX_RSP_SIZE ATMEL_I2C_GENKEY_RSP_SIZE
-#define STATUS_RSP_SIZE 4
-#define ECDH_RSP_SIZE (32 + CMD_OVERHEAD_SIZE)
-#define GENKEY_RSP_SIZE (ATMEL_ECC_PUBKEY_SIZE + \
- CMD_OVERHEAD_SIZE)
-#define ATMEL_I2C_READ_RSP_SIZE (4 + CMD_OVERHEAD_SIZE)
-#define RANDOM_RSP_SIZE (32 + CMD_OVERHEAD_SIZE)
-#define MAX_RSP_SIZE GENKEY_RSP_SIZE
+/* Definitions for Indexes common to all commands */
+#define ATMEL_I2C_RSP_DATA_IDX 1 /* buffer index of data in response */
+#define ATMEL_I2C_ECDH_SLOT_DEFAULT 2
/**
* atmel_i2c_cmd - structure used for communicating with the device.
@@ -51,7 +62,7 @@ struct atmel_i2c_cmd {
u8 opcode;
u8 param1;
__le16 param2;
- u8 data[MAX_RSP_SIZE];
+ u8 data[ATMEL_I2C_MAX_RSP_SIZE];
u8 msecs;
u16 rxsize;
} __packed;
@@ -77,39 +88,6 @@ struct atmel_i2c_of_match_data {
size_t eeprom_zone_size[3]; /* all atmel devices have three zones */
};
-/* Status/Error codes */
-#define STATUS_SIZE 0x04
-#define STATUS_NOERR 0x00
-#define STATUS_WAKE_SUCCESSFUL 0x11
-
-/* Definitions for Indexes common to all commands */
-#define ATMEL_I2C_RSP_DATA_IDX 1 /* buffer index of data in response */
-#define DATA_SLOT_2 2 /* used for ECDH private key */
-
-/*
- * Wake High delay to data communication (microseconds). SDA should be stable
- * high for this entire duration.
- */
-#define TWHI_MIN 1500
-#define TWHI_MAX 1550
-
-/* Wake Low duration */
-#define TWLO_USEC 60
-
-/* Definitions for the READ Command */
-#define ATMEL_I2C_READ_COUNT 7
-
-/* Definitions for the RANDOM Command */
-#define RANDOM_COUNT 7
-
-/* Definitions for the GenKey Command */
-#define GENKEY_COUNT 7
-#define GENKEY_MODE_PRIVATE 0x04
-
-/* Definitions for the ECDH Command */
-#define ECDH_COUNT 71
-#define ECDH_PREFIX_MODE 0x00
-
/* Used for binding tfm objects to i2c clients. */
enum atmel_i2c_capability {
ATMEL_CAP_ECDH = 0,
@@ -144,7 +122,7 @@ struct atmel_i2c_client_priv {
struct i2c_client *client;
struct list_head i2c_client_list_node;
struct mutex lock;
- u8 wake_token[WAKE_TOKEN_MAX_SIZE];
+ u8 wake_token[ATMEL_I2C_WAKE_TOKEN_MAX_SIZE];
size_t wake_token_sz;
atomic_t tfm_count ____cacheline_aligned;
struct hwrng hwrng;
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/12] crypto: atmel - add SHA256 ahash support
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (10 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 11/12] crypto: atmel - refactor and localize driver constants Lothar Rubusch
@ 2026-05-12 22:43 ` Lothar Rubusch
2026-05-14 19:51 ` [PATCH 00/12] crypto: atmel - refactor common i2c support and " Thorsten Blum
12 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-12 22:43 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
Add SHA256 ahash support for ATSHA204A and ECC based devices using
the hardware SHA engine provided by the Atmel secure element devices.
Implement common SHA256 request handling in the atmel-i2c core driver,
including init, update, final, finup, digest, export and import
operations. Scatterlist input is processed using the crypto hash
walker.
ATSHA204A devices require software-side SHA256 padding according to
FIPS 180-4 before submitting the final data blocks to the device.
Newer ECC devices instead support a dedicated SHA final command which
performs padding internally in hardware. For these devices, the final
block length is passed through the command parameter field.
The SHA engine requires a strict multi-command transaction sequence:
SHA INIT, followed by one or more SHA COMPUTE operations and, on ECC
devices, a terminating SHA FINAL operation. The device SHA context is
lost if the device enters sleep mode or if unrelated commands are
interleaved during the sequence.
To support these hardware requirements, split the existing
send/receive helper into a low-level transfer helper and a higher
level wrapper handling wakeup, sleep and locking. SHA operations keep
the device awake and hold the i2c client lock across the complete hash
transaction until the final digest has been retrieved.
Register the SHA256 ahash algorithm in both atmel-sha204a and
atmel-ecc drivers and add capability based client allocation for SHA
operations.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
---
drivers/crypto/atmel-ecc.c | 50 +++++-
drivers/crypto/atmel-i2c.c | 273 +++++++++++++++++++++++++++++++--
drivers/crypto/atmel-i2c.h | 40 +++++
drivers/crypto/atmel-sha204a.c | 55 ++++++-
4 files changed, 407 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index ed8c0ce5562b..aacf9e8add7a 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -19,10 +19,50 @@
#include <linux/slab.h>
#include <linux/workqueue.h>
#include <crypto/internal/kpp.h>
+#include <crypto/internal/hash.h>
#include <crypto/ecdh.h>
#include <crypto/kpp.h>
+#include <crypto/sha2.h>
#include "atmel-i2c.h"
+static int atmel_ecc_sha_init_tfm(struct crypto_tfm *tfm)
+{
+ struct atmel_i2c_sha_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->client = atmel_i2c_client_alloc(ATMEL_CAP_SHA);
+ if (IS_ERR(ctx->client)) {
+ pr_err("tfm - i2c_client binding failed\n");
+ return PTR_ERR(ctx->client);
+ }
+
+ return 0;
+}
+
+static struct ahash_alg atmel_ecc_sha = {
+ .init = atmel_i2c_sha_init,
+ .update = atmel_i2c_sha_update,
+ .final = atmel_i2c_sha_final,
+ .finup = atmel_i2c_sha_finup,
+ .digest = atmel_i2c_sha_digest,
+ .export = atmel_i2c_sha_export,
+ .import = atmel_i2c_sha_import,
+ .halg = {
+ .digestsize = SHA256_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_i2c_sha_reqctx),
+ .base = {
+ .cra_name = "sha256",
+ .cra_driver_name = "atmel-sha256",
+ .cra_init = atmel_ecc_sha_init_tfm,
+ .cra_priority = ATMEL_I2C_PRIORITY,
+ .cra_flags = CRYPTO_ALG_TYPE_AHASH,
+ .cra_blocksize = SHA256_BLOCK_SIZE,
+ .cra_ctxsize = sizeof(struct atmel_i2c_sha_ctx),
+ .cra_reqsize = sizeof(struct atmel_i2c_sha_reqctx),
+ .cra_module = THIS_MODULE,
+ }
+ }
+};
+
static ssize_t config_show(struct device *dev, struct device_attribute *attr, char *buf)
{
return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_CONFIG_ZONE);
@@ -321,7 +361,7 @@ static int atmel_ecc_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
i2c_priv->data = data;
- i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
+ i2c_priv->caps = BIT(ATMEL_CAP_ECDH) | BIT(ATMEL_CAP_SHA);
ret = atmel_i2c_device_sanity_check(client);
if (ret) {
@@ -364,6 +404,12 @@ static int atmel_ecc_probe(struct i2c_client *client)
dev_info(&client->dev, "atmel ecc algorithms registered in /proc/crypto\n");
}
+ ret = crypto_register_ahash(&atmel_ecc_sha);
+ if (ret) {
+ dev_err(&client->dev, "SHA256 registration failed\n");
+ goto err_list_del;
+ }
+
goto done;
err_list_del:
@@ -392,6 +438,7 @@ static void atmel_ecc_remove(struct i2c_client *client)
atmel_i2c_flush_queue();
crypto_unregister_kpp(&atmel_ecdh_nist_p256);
+ crypto_unregister_ahash(&atmel_ecc_sha);
if (i2c_priv->hwrng.priv) {
kfree((void *)i2c_priv->hwrng.priv);
@@ -405,6 +452,7 @@ static const struct atmel_i2c_of_match_data atecc508a_match_data = {
.max_exec_time_genkey = 115,
.max_exec_time_random = 23,
.max_exec_time_read = 1,
+ .max_exec_time_sha = 9,
.max_exec_time_write = 42,
},
.eeprom_zone_size = {
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 53aba2f4bedb..cbdc8c0e5aca 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -19,6 +19,10 @@
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
+#include <crypto/hash.h>
+#include <crypto/sha2.h>
+#include <crypto/internal/hash.h>
+
#include "atmel-i2c.h"
#define ATMEL_I2C_COMMAND 0x03 /* packet function */
@@ -49,12 +53,17 @@
#define ATMEL_I2C_ECDH_RSP_SIZE (32 + ATMEL_I2C_RSP_OVERHEAD_SIZE)
#define ATMEL_I2C_ECDH_PREFIX_MODE 0x00
+/* Definitions for the SHA Command */
+#define ATMEL_I2C_SHA_RSP_SIZE (ATMEL_I2C_RSP_OVERHEAD_SIZE + \
+ SHA256_DIGEST_SIZE)
+
/* Command opcode */
#define ATMEL_I2C_OPCODE_ECDH 0x43
#define ATMEL_I2C_OPCODE_GENKEY 0x40
#define ATMEL_I2C_OPCODE_READ 0x02
#define ATMEL_I2C_OPCODE_RANDOM 0x1b
#define ATMEL_I2C_OPCODE_WRITE 0x12
+#define ATMEL_I2C_OPCODE_SHA 0x47
/*
* Wake High delay to data communication (microseconds). SDA should be stable
@@ -244,6 +253,43 @@ int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
}
EXPORT_SYMBOL(atmel_i2c_init_ecdh_cmd);
+int atmel_i2c_init_sha_cmd(struct atmel_i2c_cmd *cmd,
+ u8 *challenge, size_t len,
+ enum atmel_i2c_sha_engine_cmd sha_engine_cmd,
+ const struct atmel_i2c_max_exec_timings *timings)
+{
+ cmd->word_addr = ATMEL_I2C_COMMAND;
+ cmd->opcode = ATMEL_I2C_OPCODE_SHA;
+ cmd->param1 = sha_engine_cmd;
+
+ cmd->param2 = cpu_to_le16(0);
+ /*
+ * Starting with the bigger ECCs, the device learned how to do SHA256
+ * padding (FIPS 180-4). Since SHA UPDATE always consumes 64B (SHA256
+ * block size), the only length needed to communicate is the number of
+ * used bytes in the final block. For the Atmel ECC series, this is
+ * passed in the param2.
+ */
+ if (sha_engine_cmd == atmel_sha_ecc_end)
+ cmd->param2 = cpu_to_le16(len);
+
+ cmd->count = ATMEL_I2C_COUNT_OVERHEAD_SIZE;
+ if (sha_engine_cmd == atmel_sha_init) {
+ memset(cmd->data, 0, sizeof(cmd->data));
+ } else {
+ memcpy(cmd->data, challenge, len);
+ cmd->count += len;
+ }
+
+ atmel_i2c_checksum(cmd);
+
+ cmd->msecs = timings->max_exec_time_sha;
+ cmd->rxsize = atmel_i2c_sha_rsp_size[sha_engine_cmd];
+
+ return 0;
+}
+EXPORT_SYMBOL(atmel_i2c_init_sha_cmd);
+
static void atmel_i2c_rng_done(struct atmel_i2c_work_data *work_data,
void *areq, int status)
{
@@ -492,21 +538,15 @@ static int atmel_i2c_sleep(struct i2c_client *client)
* counter other than to put the device into sleep or idle mode and then
* wake it up again.
*/
-int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
+static int _atmel_i2c_send_receive(struct i2c_client *client,
+ struct atmel_i2c_cmd *cmd)
{
- struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
int ret;
- mutex_lock(&i2c_priv->lock);
-
- ret = atmel_i2c_wakeup(client);
- if (ret)
- goto err;
-
/* send the command */
ret = i2c_master_send(client, (u8 *)cmd, cmd->count + ATMEL_I2C_ADDR_SIZE);
if (ret < 0)
- goto err;
+ return ret;
/* delay the appropriate amount of time for command to execute */
msleep(cmd->msecs);
@@ -514,6 +554,24 @@ int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
/* receive the response */
ret = i2c_master_recv(client, cmd->data, cmd->rxsize);
if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
+{
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+ int ret;
+
+ mutex_lock(&i2c_priv->lock);
+
+ ret = atmel_i2c_wakeup(client);
+ if (ret)
+ goto err;
+
+ ret = _atmel_i2c_send_receive(client, cmd);
+ if (ret)
goto err;
/* put the device into low-power mode */
@@ -529,6 +587,203 @@ int atmel_i2c_send_receive(struct i2c_client *client, struct atmel_i2c_cmd *cmd)
}
EXPORT_SYMBOL(atmel_i2c_send_receive);
+int atmel_i2c_sha_init(struct ahash_request *req)
+{
+ struct atmel_i2c_sha_reqctx *rctx = ahash_request_ctx(req);
+ struct atmel_i2c_sha_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(ctx->client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_cmd *cmd;
+ int ret;
+
+ rctx->bufcnt = 0;
+ rctx->total = 0;
+ rctx->ctx = i2c_get_clientdata(ctx->client);
+
+ cmd = kmalloc_obj(*cmd);
+ if (!cmd)
+ return -ENOMEM;
+
+ /* SHA init */
+ ret = atmel_i2c_init_sha_cmd(cmd, NULL, 0, atmel_sha_init, &data->timings);
+ if (ret)
+ goto err_free;
+
+ mutex_lock(&i2c_priv->lock);
+
+ ret = atmel_i2c_wakeup(ctx->client);
+ if (ret)
+ goto err;
+
+ ret = _atmel_i2c_send_receive(ctx->client, cmd);
+ if (ret)
+ goto err;
+
+ /* we keep the lock hold until error out or _sha_final() is called */
+ return 0;
+err:
+ mutex_unlock(&i2c_priv->lock);
+err_free:
+ kfree_sensitive(cmd);
+ return ret;
+}
+EXPORT_SYMBOL(atmel_i2c_sha_init);
+
+int atmel_i2c_sha_update(struct ahash_request *req)
+{
+ struct atmel_i2c_sha_reqctx *rctx = ahash_request_ctx(req);
+ struct atmel_i2c_sha_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(ctx->client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_cmd *cmd;
+ struct crypto_hash_walk walk;
+ int nbytes, take, copied = 0;
+ const u8 *pdata;
+ int ret;
+
+ rctx->total += req->nbytes;
+
+ cmd = kmalloc_obj(*cmd);
+ if (!cmd) {
+ ret = -ENOMEM;
+ goto err_nomem;
+ }
+
+ /*
+ * Note, we are actively holding the i2c_priv->lock while the SHA engine
+ * operates. This covers init, update and final steps.
+ */
+ nbytes = crypto_hash_walk_first(req, &walk);
+ for (; nbytes > 0; nbytes = crypto_hash_walk_done(&walk, copied)) {
+ copied = nbytes;
+ pdata = walk.data;
+ while (copied > 0) {
+ take = min(copied, SHA256_BLOCK_SIZE - rctx->bufcnt);
+
+ memcpy(rctx->buffer + rctx->bufcnt, pdata, take);
+ pdata += take;
+ copied -= take;
+ rctx->bufcnt += take;
+ if (rctx->bufcnt == SHA256_BLOCK_SIZE) {
+ ret = atmel_i2c_init_sha_cmd(cmd, rctx->buffer,
+ SHA256_BLOCK_SIZE,
+ atmel_sha_compute,
+ &data->timings);
+ if (ret)
+ goto err;
+
+ ret = _atmel_i2c_send_receive(ctx->client, cmd);
+ if (ret)
+ goto err;
+
+ rctx->bufcnt = 0;
+ }
+ }
+ }
+
+ kfree_sensitive(cmd);
+ return 0;
+err:
+ kfree_sensitive(cmd);
+err_nomem:
+ mutex_unlock(&i2c_priv->lock);
+ return ret;
+}
+EXPORT_SYMBOL(atmel_i2c_sha_update);
+
+int atmel_i2c_sha_final(struct ahash_request *req)
+{
+ struct atmel_i2c_sha_reqctx *rctx = ahash_request_ctx(req);
+ struct atmel_i2c_sha_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(ctx->client);
+ const struct atmel_i2c_of_match_data *data = i2c_priv->data;
+ struct atmel_i2c_cmd *cmd;
+ u8 final_blocks[2 * SHA256_BLOCK_SIZE];
+ u32 total_pad;
+ __be64 bits;
+ int i, ret = 0;
+
+ cmd = kmalloc_obj(*cmd);
+ if (!cmd) {
+ ret = -ENOMEM;
+ goto err_nomem;
+ }
+
+ if (data->needs_sha_padding) {
+ /*
+ * Determine if padding fits in current block or needs another,
+ * SHA256 needs 8 bytes for length at the end of a 64-byte block.
+ */
+ memset(final_blocks, 0, sizeof(final_blocks));
+ memcpy(final_blocks, rctx->buffer, rctx->bufcnt);
+ final_blocks[rctx->bufcnt] = 0x80; /* pad bit */
+ total_pad = SHA256_BLOCK_SIZE * (rctx->bufcnt < 56 ? 1 : 2);
+ bits = cpu_to_be64((u64)rctx->total << 3); /* needs num of bits */
+ memcpy(final_blocks + total_pad - 8, &bits, 8);
+ for (i = 0; i < total_pad; i += SHA256_BLOCK_SIZE) {
+ ret = atmel_i2c_init_sha_cmd(cmd, final_blocks + i,
+ SHA256_BLOCK_SIZE,
+ atmel_sha_compute, &data->timings);
+ if (ret)
+ goto err_or_done;
+
+ ret = _atmel_i2c_send_receive(ctx->client, cmd);
+ if (ret)
+ goto err_or_done;
+ }
+ } else {
+ ret = atmel_i2c_init_sha_cmd(cmd, rctx->buffer, rctx->bufcnt,
+ atmel_sha_ecc_end, &data->timings);
+ if (ret)
+ goto err_or_done;
+
+ ret = _atmel_i2c_send_receive(ctx->client, cmd);
+ if (ret)
+ goto err_or_done;
+ }
+
+ memcpy(req->result, &cmd->data[ATMEL_I2C_RSP_DATA_IDX],
+ SHA256_DIGEST_SIZE);
+
+ /* Sleep returns a positive int on success, API requires 0 on success */
+ ret = atmel_i2c_sleep(ctx->client);
+ if (ret < 0)
+ goto err_or_done;
+ ret = 0;
+err_or_done:
+ kfree_sensitive(cmd);
+err_nomem:
+ mutex_unlock(&i2c_priv->lock);
+ return ret;
+}
+EXPORT_SYMBOL(atmel_i2c_sha_final);
+
+int atmel_i2c_sha_finup(struct ahash_request *req)
+{
+ return atmel_i2c_sha_update(req) ? : atmel_i2c_sha_final(req);
+}
+EXPORT_SYMBOL(atmel_i2c_sha_finup);
+
+int atmel_i2c_sha_digest(struct ahash_request *req)
+{
+ return atmel_i2c_sha_init(req) ? : atmel_i2c_sha_finup(req);
+}
+EXPORT_SYMBOL(atmel_i2c_sha_digest);
+
+int atmel_i2c_sha_export(struct ahash_request *req, void *out)
+{
+ memcpy(out, ahash_request_ctx(req), sizeof(struct atmel_i2c_sha_reqctx));
+ return 0;
+}
+EXPORT_SYMBOL(atmel_i2c_sha_export);
+
+int atmel_i2c_sha_import(struct ahash_request *req, const void *in)
+{
+ memcpy(ahash_request_ctx(req), in, sizeof(struct atmel_i2c_sha_reqctx));
+ return 0;
+}
+EXPORT_SYMBOL(atmel_i2c_sha_import);
+
static void atmel_i2c_work_handler(struct work_struct *work)
{
struct atmel_i2c_work_data *work_data =
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 20afe2da4f8d..e0021d4ea686 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -7,8 +7,11 @@
#ifndef __ATMEL_I2C_H__
#define __ATMEL_I2C_H__
+#include <linux/device.h>
+#include <crypto/internal/hash.h>
#include <linux/hw_random.h>
#include <linux/types.h>
+#include <crypto/sha2.h>
#define ATMEL_I2C_PRIORITY 300
@@ -79,11 +82,13 @@ struct atmel_i2c_max_exec_timings {
unsigned int max_exec_time_ecdh;
unsigned int max_exec_time_random;
unsigned int max_exec_time_read;
+ unsigned int max_exec_time_sha;
unsigned int max_exec_time_write;
};
struct atmel_i2c_of_match_data {
const unsigned short needs_legacy_hwrng;
+ const unsigned short needs_sha_padding;
struct atmel_i2c_max_exec_timings timings;
size_t eeprom_zone_size[3]; /* all atmel devices have three zones */
};
@@ -91,6 +96,30 @@ struct atmel_i2c_of_match_data {
/* Used for binding tfm objects to i2c clients. */
enum atmel_i2c_capability {
ATMEL_CAP_ECDH = 0,
+ ATMEL_CAP_SHA,
+};
+
+enum atmel_i2c_sha_engine_cmd {
+ atmel_sha_init = 0,
+ atmel_sha_compute,
+ atmel_sha_ecc_end,
+};
+
+size_t atmel_i2c_sha_rsp_size[] = {
+ [atmel_sha_init] = ATMEL_I2C_STATUS_RSP_SIZE,
+ [atmel_sha_compute] = SHA256_DIGEST_SIZE + ATMEL_I2C_RSP_OVERHEAD_SIZE,
+ [atmel_sha_ecc_end] = SHA256_DIGEST_SIZE + ATMEL_I2C_RSP_OVERHEAD_SIZE,
+};
+
+struct atmel_i2c_sha_ctx {
+ struct i2c_client *client;
+};
+
+struct atmel_i2c_sha_reqctx {
+ u8 buffer[SHA256_BLOCK_SIZE];
+ size_t bufcnt;
+ size_t total; /* size of full input, needed for padding */
+ struct atmel_i2c_client_priv *ctx;
};
struct atmel_i2c_client_mgmt {
@@ -172,9 +201,20 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid,
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey,
const struct atmel_i2c_max_exec_timings *timings);
+int atmel_i2c_init_sha_cmd(struct atmel_i2c_cmd *cmd, u8 *challenge, size_t len,
+ enum atmel_i2c_sha_engine_cmd sha_engine_cmd,
+ const struct atmel_i2c_max_exec_timings *timings);
int atmel_i2c_register_rng(struct atmel_i2c_client_priv *i2c_priv,
struct device *dev);
+int atmel_i2c_sha_init(struct ahash_request *req);
+int atmel_i2c_sha_update(struct ahash_request *req);
+int atmel_i2c_sha_final(struct ahash_request *req);
+int atmel_i2c_sha_finup(struct ahash_request *req);
+int atmel_i2c_sha_digest(struct ahash_request *req);
+int atmel_i2c_sha_export(struct ahash_request *req, void *out);
+int atmel_i2c_sha_import(struct ahash_request *req, const void *in);
+
int atmel_i2c_device_sanity_check(struct i2c_client *client);
ssize_t atmel_i2c_eeprom_display(struct device *dev,
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 6a41024ae40d..74535480edeb 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -17,8 +17,48 @@
#include <linux/slab.h>
#include <linux/sysfs.h>
#include <linux/workqueue.h>
+#include <crypto/sha2.h>
+
#include "atmel-i2c.h"
+static int atmel_sha204a_sha_init_tfm(struct crypto_tfm *tfm)
+{
+ struct atmel_i2c_sha_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ ctx->client = atmel_i2c_client_alloc(ATMEL_CAP_SHA);
+ if (IS_ERR(ctx->client)) {
+ pr_err("tfm - i2c_client binding failed\n");
+ return PTR_ERR(ctx->client);
+ }
+
+ return 0;
+}
+
+static struct ahash_alg atmel_sha204a_sha = {
+ .init = atmel_i2c_sha_init,
+ .update = atmel_i2c_sha_update,
+ .final = atmel_i2c_sha_final,
+ .finup = atmel_i2c_sha_finup,
+ .digest = atmel_i2c_sha_digest,
+ .export = atmel_i2c_sha_export,
+ .import = atmel_i2c_sha_import,
+ .halg = {
+ .digestsize = SHA256_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_i2c_sha_reqctx),
+ .base = {
+ .cra_name = "sha256",
+ .cra_driver_name = "atmel-sha256",
+ .cra_init = atmel_sha204a_sha_init_tfm,
+ .cra_priority = ATMEL_I2C_PRIORITY,
+ .cra_flags = CRYPTO_ALG_TYPE_AHASH,
+ .cra_blocksize = SHA256_BLOCK_SIZE,
+ .cra_ctxsize = sizeof(struct atmel_i2c_sha_ctx),
+ .cra_reqsize = sizeof(struct atmel_i2c_sha_reqctx),
+ .cra_module = THIS_MODULE,
+ }
+ }
+};
+
static ssize_t config_show(struct device *dev, struct device_attribute *attr, char *buf)
{
return atmel_i2c_eeprom_display(dev, attr, buf, ATMEL_EEPROM_CONFIG_ZONE);
@@ -62,7 +102,7 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
i2c_priv->data = data;
- i2c_priv->caps = 0;
+ i2c_priv->caps = BIT(ATMEL_CAP_SHA);
ret = atmel_i2c_device_sanity_check(client);
if (ret) {
@@ -95,6 +135,13 @@ static int atmel_sha204a_probe(struct i2c_client *client)
goto err_list_del;
}
+ /* register algorithms */
+ ret = crypto_register_ahash(&atmel_sha204a_sha);
+ if (ret) {
+ dev_err(&client->dev, "SHA256 registration failed\n");
+ goto err_list_del;
+ }
+
goto done;
err_list_del:
@@ -119,6 +166,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
atmel_i2c_flush_queue();
+ crypto_unregister_ahash(&atmel_sha204a_sha);
+
if (i2c_priv->hwrng.priv) {
kfree((void *)i2c_priv->hwrng.priv);
i2c_priv->hwrng.priv = 0;
@@ -130,6 +179,7 @@ static const struct atmel_i2c_of_match_data atsha204_match_data = {
.max_exec_time_genkey = 43,
.max_exec_time_random = 50,
.max_exec_time_read = 4,
+ .max_exec_time_sha = 22,
.max_exec_time_write = 42,
},
.eeprom_zone_size = {
@@ -142,6 +192,7 @@ static const struct atmel_i2c_of_match_data atsha204_match_data = {
* [1] https://www.metzdowd.com/pipermail/cryptography/2014-December/023858.html
*/
.needs_legacy_hwrng = 1,
+ .needs_sha_padding = 1,
};
static const struct atmel_i2c_of_match_data atsha204a_match_data = {
@@ -149,6 +200,7 @@ static const struct atmel_i2c_of_match_data atsha204a_match_data = {
.max_exec_time_genkey = 43,
.max_exec_time_random = 50,
.max_exec_time_read = 4,
+ .max_exec_time_sha = 22,
.max_exec_time_write = 42,
},
.eeprom_zone_size = {
@@ -156,6 +208,7 @@ static const struct atmel_i2c_of_match_data atsha204a_match_data = {
[ATMEL_EEPROM_OTP_ZONE] = 64,
[ATMEL_EEPROM_DATA_ZONE] = 512
},
+ .needs_sha_padding = 1,
};
static const struct of_device_id atmel_sha204a_dt_ids[] __maybe_unused = {
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
` (11 preceding siblings ...)
2026-05-12 22:43 ` [PATCH 12/12] crypto: atmel - add SHA256 ahash support Lothar Rubusch
@ 2026-05-14 19:51 ` Thorsten Blum
2026-05-15 20:29 ` Lothar Rubusch
2026-05-19 20:47 ` Lothar Rubusch
12 siblings, 2 replies; 18+ messages in thread
From: Thorsten Blum @ 2026-05-14 19:51 UTC (permalink / raw)
To: Lothar Rubusch
Cc: herbert, davem, nicolas.ferre, alexandre.belloni, claudiu.beznea,
linux-crypto, linux-arm-kernel, linux-kernel
Hi Lothar,
On Tue, May 12, 2026 at 10:43:37PM +0000, Lothar Rubusch wrote:
> This series restructures the Atmel secure element drivers around a
> shared atmel-i2c core and adds SHA256 ahash support for ATSHA204A and
> ECC based devices.
>
> The existing drivers duplicated substantial parts of the transport,
> RNG, EEPROM and device management logic. This series consolidates the
> common functionality into the shared i2c core and converts the client
> drivers to capability based allocation.
>
> The series also introduces per-device timing configuration through
> match data, moves sanity checks and RNG handling into the core driver,
> updates workqueue handling and cleans up internal constants and helper
> definitions.
>
> The final patch adds SHA256 ahash support using the hardware SHA engine
> provided by the devices.
>
> ATSHA204A devices require software-side SHA256 padding according to
> FIPS 180-4, while newer ECC devices provide a dedicated SHA final
> command and perform padding internally in hardware.
>
> Supporting the SHA engine also requires changes to the command
> transport path. SHA operations must execute as a strict uninterrupted
> sequence consisting of SHA INIT, one or more SHA COMPUTE commands and,
> for ECC devices, a terminating SHA FINAL command. The device loses its
> internal SHA state if it enters sleep mode or if unrelated commands
> are interleaved during the transaction.
>
> To satisfy these hardware requirements, the send/receive path is split
> into a low-level transfer helper and a higher-level wrapper managing
> wakeup, sleep and locking. SHA operations keep the device awake and
> hold the i2c lock for the full duration of the hashing transaction.
>
> The series has been tested on ATSHA204A and ATECC508A devices.
> Tests are ongoing/pending on ATECC608A and ATECC608B.
> ---
> Lothar Rubusch (12):
> crypto: atmel - introduce shared I2C client management
> crypto: atmel - move capability-based client allocation into i2c core
> crypto: atmel - remove obsolete CONFIG_OF guard
> crypto: atmel - add per-device timing and match-data driven
> configuration
> crypto: atmel - move RNG support into common i2c core
> crypto: atmel - move EEPROM access support into common i2c core
> crypto: atmel - expose CONFIG zone through sysfs
> crypto: atmel - move device sanity check to core driver
> crypto: atmel - check client data in remove callbacks
> crypto: atmel - update workqueue flags and add flush on exit
> crypto: atmel - refactor and localize driver constants
> crypto: atmel - add SHA256 ahash support
>
> drivers/crypto/atmel-ecc.c | 252 +++++++-----
> drivers/crypto/atmel-i2c.c | 679 +++++++++++++++++++++++++++++----
> drivers/crypto/atmel-i2c.h | 180 +++++----
> drivers/crypto/atmel-sha204a.c | 284 +++++++-------
> 4 files changed, 1010 insertions(+), 385 deletions(-)
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
Thanks, but I'm not sure reviewing such a large series is sustainable.
I've only skimmed it, but it also mixes several different things that
should probably be submitted separately (e.g., refactorings and new
features).
Sashiko [1] also reviewed the series and found potential regressions
that might be helpful to consider.
Thanks,
Thorsten
[1] https://sashiko.dev/#/patchset/20260512224349.64621-1-l.rubusch%40gmail.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support
2026-05-14 19:51 ` [PATCH 00/12] crypto: atmel - refactor common i2c support and " Thorsten Blum
@ 2026-05-15 20:29 ` Lothar Rubusch
2026-05-19 20:47 ` Lothar Rubusch
1 sibling, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-15 20:29 UTC (permalink / raw)
To: Thorsten Blum
Cc: herbert, davem, nicolas.ferre, alexandre.belloni, claudiu.beznea,
linux-crypto, linux-arm-kernel, linux-kernel
Hi Thorsten & ML,
On Thu, May 14, 2026 at 9:51 PM Thorsten Blum <thorsten.blum@linux•dev> wrote:
>
> Hi Lothar,
>
> On Tue, May 12, 2026 at 10:43:37PM +0000, Lothar Rubusch wrote:
> > This series restructures the Atmel secure element drivers around a
> > shared atmel-i2c core and adds SHA256 ahash support for ATSHA204A and
> > ECC based devices.
> >
> > The existing drivers duplicated substantial parts of the transport,
> > RNG, EEPROM and device management logic. This series consolidates the
> > common functionality into the shared i2c core and converts the client
> > drivers to capability based allocation.
> >
> > The series also introduces per-device timing configuration through
> > match data, moves sanity checks and RNG handling into the core driver,
> > updates workqueue handling and cleans up internal constants and helper
> > definitions.
> >
> > The final patch adds SHA256 ahash support using the hardware SHA engine
> > provided by the devices.
> >
> > ATSHA204A devices require software-side SHA256 padding according to
> > FIPS 180-4, while newer ECC devices provide a dedicated SHA final
> > command and perform padding internally in hardware.
> >
> > Supporting the SHA engine also requires changes to the command
> > transport path. SHA operations must execute as a strict uninterrupted
> > sequence consisting of SHA INIT, one or more SHA COMPUTE commands and,
> > for ECC devices, a terminating SHA FINAL command. The device loses its
> > internal SHA state if it enters sleep mode or if unrelated commands
> > are interleaved during the transaction.
> >
> > To satisfy these hardware requirements, the send/receive path is split
> > into a low-level transfer helper and a higher-level wrapper managing
> > wakeup, sleep and locking. SHA operations keep the device awake and
> > hold the i2c lock for the full duration of the hashing transaction.
> >
> > The series has been tested on ATSHA204A and ATECC508A devices.
> > Tests are ongoing/pending on ATECC608A and ATECC608B.
> > ---
> > Lothar Rubusch (12):
> > crypto: atmel - introduce shared I2C client management
> > crypto: atmel - move capability-based client allocation into i2c core
> > crypto: atmel - remove obsolete CONFIG_OF guard
> > crypto: atmel - add per-device timing and match-data driven
> > configuration
> > crypto: atmel - move RNG support into common i2c core
> > crypto: atmel - move EEPROM access support into common i2c core
> > crypto: atmel - expose CONFIG zone through sysfs
> > crypto: atmel - move device sanity check to core driver
> > crypto: atmel - check client data in remove callbacks
> > crypto: atmel - update workqueue flags and add flush on exit
> > crypto: atmel - refactor and localize driver constants
> > crypto: atmel - add SHA256 ahash support
> >
> > drivers/crypto/atmel-ecc.c | 252 +++++++-----
> > drivers/crypto/atmel-i2c.c | 679 +++++++++++++++++++++++++++++----
> > drivers/crypto/atmel-i2c.h | 180 +++++----
> > drivers/crypto/atmel-sha204a.c | 284 +++++++-------
> > 4 files changed, 1010 insertions(+), 385 deletions(-)
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail•com>
>
> Thanks, but I'm not sure reviewing such a large series is sustainable.
> I've only skimmed it, but it also mixes several different things that
> should probably be submitted separately (e.g., refactorings and new
> features).
>
No problem at all. Pls, understand my series as a proposal.
Usually I'm testing the features and use checkers. So, the series is
supposed to be functional. Anyway, there are quite some changes, which
individually need to be analyzed and well understood to get this into
"maintainable" quality, which is definitely not the case yet. I agree.
Having said that, I'd propose to separate out the first, say, 3 patches.
(AFAIK patch #3 is +/- something, you already presented, too, so I assume
it disappears by rebasing soon). I'll split up these initial patches,
do smaller steps, and come up with this series the next days.
Pls, let me know what you think. Would this be better for a review?
> Sashiko [1] also reviewed the series and found potential regressions
> that might be helpful to consider.
>
> Thanks,
> Thorsten
>
> [1] https://sashiko.dev/#/patchset/20260512224349.64621-1-l.rubusch%40gmail.com
Fascinating! Thank you so much for pointing this out!
I'll try to take sashiko comments into account. Sashiko was new to me.
I'll definitely
have a look into it, either reviews or if there is a chance to set it
up locally.
Best,
L
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support
2026-05-14 19:51 ` [PATCH 00/12] crypto: atmel - refactor common i2c support and " Thorsten Blum
2026-05-15 20:29 ` Lothar Rubusch
@ 2026-05-19 20:47 ` Lothar Rubusch
1 sibling, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-05-19 20:47 UTC (permalink / raw)
To: Thorsten Blum
Cc: herbert, davem, nicolas.ferre, alexandre.belloni, claudiu.beznea,
linux-crypto, linux-arm-kernel, linux-kernel
Hi Thorsten,
(...)
>
> Sashiko [1] also reviewed the series and found potential regressions
> that might be helpful to consider.
>
> Thanks,
> Thorsten
>
> [1] https://sashiko.dev/#/patchset/20260512224349.64621-1-l.rubusch%40gmail.com
FYI
As you asked to consider the Sashiko output, I will paste here the sashiko
review feedback and inlined my answers down below. I may continue with this
still some more iterations against AI/sashiko bot, no problem. How eventually
can I signal you (guys), when I need someone more "human" / maintainer?
What exciting times, we're living in... - let me know, if I use this tool wrong.
[PATCH 01/12] crypto: atmel-ecc - rename driver_data before moving it
into atmel-i2c
> commit 0ed25c0f5044cd5112cafff9ff8ab8248d0e2676
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-ecc - rename driver_data before moving it into atmel-i2c
> This commit renames the local driver_data instance to atmel_i2c_mgmt
> to prepare for moving shared I2C client management infrastructure into
> the atmel-i2c core driver. No functional changes are intended.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 9660f6426a849..c9f798ebf44fe 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -323,16 +323,16 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_add_tail(&i2c_priv->i2c_client_list_node,
> > - &driver_data.i2c_client_list);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + &atmel_i2c_mgmt.i2c_client_list);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> >
> > ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> This isn't a bug introduced by this patch, but does adding i2c_priv to the
It is IMHO not very helpful to flag a(ll) particular patch(es) for regressions
in the existing code aside, but ok. Perhaps I can fix something.
> global list before crypto_register_kpp() create a use-after-free risk?
> If algorithm registration fails, the error path removes the client from the
> list and returns an error, leading the driver core to free the i2c_priv
> memory.
> However, during the brief window it was on the list, a concurrent
> atmel_ecc_i2c_client_alloc() could discover the client and assign it to an
> active crypto request.
> When the probe fails and memory is freed, would the concurrent crypto
> request operate on freed memory?
> Also, does calling crypto_register_kpp() with a single static global
> algorithm structure cause memory corruption if multiple devices are probed?
> The crypto core modifies the algorithm structure before taking locks, which
> would corrupt the state of the active algorithm registered by the first
> device.
> The second probe would also fail, breaking the multiple-device load-balancing
> design. Unregistering a single device also unconditionally unregisters the
> shared algorithm.
As soon as being registered, the struct `atmel_ecdh_nist_p256` will require to
access the `atmel_i2c_mgmt.i2c_client_list`. Being it for the selftests, or a
consumer. Thus I cannot simply change the order. I added a ready/state flag to
address the use-after-free risk.
> > if (ret) {
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> [ ... ]
> > @@ -363,9 +363,9 @@ static void atmel_ecc_remove(struct i2c_client *client)
> >
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> >
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > }
> This isn't a bug introduced by this patch, but does the lockless check of
Sure, let's continue drive-by-reviewing...
> tfm_count earlier in atmel_ecc_remove() introduce a race condition?
> A concurrent atmel_ecc_i2c_client_alloc() could acquire the list lock and
> increment tfm_count immediately after the check, causing the device to be
> freed while a new session is starting.
> Additionally, if tfm_count is detected as greater than zero, the driver
> initially returns early. This leaves the freed i2c_priv on the global list
> which leads to list corruption and use-after-free on subsequent allocations.
> Returning from remove does not prevent the driver core from freeing the
> devres-managed memory. Should this synchronously block until tfm_count
> reaches zero to safely unbind?
> The dangling list pointer issue is fixed later in the series by commit
> "crypto: atmel-ecc - simplify remove path and relax busy handling", but the
> underlying active-user use-after-free remains unfixed.
I added a global lock with refcnting to address this.
[PATCH 02/12] crypto: atmel - rename atmel_ecc_driver_data to
atmel_i2c_client_mgmt
> Result:
> No issues found.
[PATCH 03/12] crypto: atmel - move i2c client management instance into
core driver
> commit 81ed08d3b64ea6591f1cadc2ccdca81d40c20205
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel - move i2c client management instance into core driver
> This commit moves the atmel_i2c client management instance from the ecc
> driver into the atmel-i2c core driver. This prepares for consolidating
> shared i2c client tracking infrastructure in the core.
> [ ... ]
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > index 0e275dbdc8c55..db24f65ae90e8 100644
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> > @@ -21,6 +21,12 @@
> > #include <linux/workqueue.h>
> > #include "atmel-i2c.h"
> >
> > +struct atmel_i2c_client_mgmt atmel_i2c_mgmt = {
> > + .i2c_list_lock = __SPIN_LOCK_UNLOCKED(atmel_i2c_mgmt.i2c_list_lock),
> > + .i2c_client_list = LIST_HEAD_INIT(atmel_i2c_mgmt.i2c_client_list),
> > +};
> > +EXPORT_SYMBOL_GPL(atmel_i2c_mgmt);
> While this regression wasn't introduced by this patch, does exposing this
Sure..
> globally shared list allow for a time-of-check to time-of-use race in
> atmel_ecc_remove() that can result in a use-after-free?
> In atmel_ecc_remove(), the check if (atomic_read(&i2c_priv->tfm_count)) is
> performed without holding the i2c_list_lock.
> If the count is 0, the function proceeds to call crypto_unregister_kpp().
> During this algorithm unregistration window, the client remains accessible on
> the globally shared atmel_i2c_mgmt.i2c_client_list.
> If a concurrent thread calls crypto_alloc_kpp(), it triggers the .init callback
> atmel_ecdh_init_tfm(), which calls atmel_ecc_i2c_client_alloc().
> The allocator acquires the list lock, discovers the dying client still on the
> list, increments tfm_count, and returns it.
> Then, atmel_ecc_remove() acquires the list lock, removes the client from the
> list, and returns.
> Returning from the remove function causes the devres framework to automatically
> free i2c_priv, leaving the concurrent thread with a tfm bound to freed memory.
> Can this sequence lead to memory corruption?
The two additional patches will cover the remove function.
[PATCH 04/12] crypto: atmel-ecc - simplify probe error handling
> commit 3bca954bd7e39050b3fe5381aad35c915da883f3
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-ecc - simplify probe error handling
> This commit refactors the error path in atmel_ecc_probe to use a goto
> statement instead of duplicating the list removal code. It also claims to
> add comments clarifying client list insertion and algorithm registration.
> > crypto: atmel-ecc - simplify probe error handling
> >
> > Replace early return in atmel_ecc_probe() with explicit error handling
> > using a goto-based cleanup path.
> >
> > Add comments to clarify client list insertion and algorithm registration
> > steps.
> This isn't a bug, but the commit message states it replaces an early
> return and adds comments, yet the diff doesn't show any comments being
> added or an early return being removed. Should the commit message be
> updated to reflect the actual changes?
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 76fb1d0cf0753..696ab1d51fc67 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -328,17 +328,20 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> Is it safe to register the static global atmel_ecdh_nist_p256 algorithm
> structure here directly from the probe function? If multiple devices are
> probed, a second call to crypto_register_kpp with the same static struct
> could cause crypto_check_alg to reset the active algorithm's reference
> count, and __crypto_register_alg to re-initialize the cra_users list,
> potentially causing a regression that corrupts the state for active
> crypto users.
> > if (ret) {
> > - spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > - list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > -
> > dev_err(&client->dev, "%s alg registration failed\n",
> > atmel_ecdh_nist_p256.base.cra_driver_name);
> > + goto err_list_del;
> By moving the list removal to a goto block after the dev_err call, does this
> code introduce a regression by widening the window for a race condition?
> Since the client is added to the global atmel_i2c_mgmt.i2c_client_list earlier
> in the probe, if registration fails, a concurrent task might find and use the
> failed client. Executing a potentially slow console log before removing the
> client from the list seems to worsen the window for a use-after-free when
> devres cleans up i2c_priv.
> [ ... ]
Patch replaced and merged
[PATCH 05/12] crypto: atmel - factor out i2c client unregistration helper
> commit 67148be9a0cfbd189723055fbc1cdfa19bd244ad
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel - factor out i2c client unregistration helper
> This commit moves the i2c client removal logic into a dedicated helper
> function in the atmel-i2c core. It also converts the ECC driver remove
> path to use this new helper.
> > Convert ECC driver remove path to use the new helper and
> > ensure queue flushing is performed after removing the device
> > from the shared client list.
> Is this description accurate? Looking at the implementation below, the queue
> flushing is performed before removing the device from the list.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 696ab1d51fc67..e5dd77008b97f 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -364,9 +364,8 @@ static void atmel_ecc_remove(struct i2c_client *client)
> >
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> >
> > - spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > - list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > + atmel_i2c_flush_queue();
> > + atmel_i2c_unregister_client(i2c_priv);
> > }
> While crypto_unregister_kpp() prevents new lookups, concurrent threads that
> already resolved the algorithm can still execute atmel_ecdh_init_tfm().
> Because atmel_i2c_flush_queue() is called first, it will sleep waiting for
> the workqueue to drain while the client is still present in the global
> atmel_i2c_mgmt.i2c_client_list.
> Could a concurrent atmel_ecdh_init_tfm() call atmel_i2c_client_alloc(),
> find the still-linked client, increment its tfm_count, and assign it to
> ctx->client?
> Once the flush completes, atmel_i2c_unregister_client() removes the client,
> the driver returns, and the device core frees the i2c_priv memory.
> Would this result in a use-after-free on subsequent crypto operations since
> ctx->client now points to freed memory?
Replaced by a more comprehensive dealing with the teardown, patch replaced.
[PATCH 06/12] crypto: atmel-sha204a - add i2c hw client list and
improve probe error handling
> commit 14fd691f797c32576c19b57c937cd6cf75be5ef8
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-sha204a - add i2c hw client list and improve probe error handling
> This patch adds registration of the sha204a i2c client into the shared
> atmel_i2c client management list during probe. This allows the driver to
> participate in the common hardware selection infrastructure.
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index 6e6ac4770416e..cdfdcf2e43a7d 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -173,6 +173,13 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > + /* add to client list */
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > + list_add_tail(&i2c_priv->i2c_client_list_node,
> > + &atmel_i2c_mgmt.i2c_client_list);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> Is it possible for another thread to discover this client before the rest of
> the probe completes?
> If another thread like atmel_i2c_client_alloc() grabs a reference to this
> client from the shared list, and then a subsequent initialization step fails,
> the error path will fail the probe. The devres cleanup will free i2c_priv,
> but the concurrent user will be left with a dangling pointer.
> Also, since the shared atmel_i2c_mgmt.i2c_client_list is used by the
> atmel-ecc driver for hardware selection, could adding the sha204a device
> here cause the atmel-ecc driver to incorrectly dispatch ecdh/ecdsa
> operations to this device?
> Since the selection logic picks the client with the lowest active workload,
> it might select this device and the hardware would reject unsupported
> commands. This was addressed later in the series by commit "crypto: atmel -
> add capability-based i2c client selection".
> Additionally, if the atmel-sha204a module is loaded first, could loading the
> atmel-ecc module later completely wipe out this shared list?
> If the atmel-ecc initialization blindly uses INIT_LIST_HEAD() on this list,
> it would orphan the sha204a clients. This was fixed later in the series by
> commit "crypto: atmel-ecc - switch to module_i2c_driver".
> > +
> > + /* register rng */
> > memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
> >
> > i2c_priv->hwrng.name = dev_name(&client->dev);
> > @@ -183,15 +190,24 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> > i2c_priv->hwrng.quality = *quality;
> >
> > ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
> > - if (ret)
> > + if (ret) {
> > dev_warn(&client->dev, "failed to register rng (%d)\n", ret);
> > + goto err_list_del;
> > + }
> >
> > ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
> > if (ret) {
> > dev_err(&client->dev, "failed to register sysfs entry\n");
> > - return ret;
> > + goto err_list_del;
> > }
> Is it possible for userspace to queue an asynchronous non-blocking read
> between devm_hwrng_register() and this sysfs_create_group() call?
> If an async rng read is queued to atmel_wq and work_data is allocated, and
> then sysfs_create_group() fails, devres will unwind allocations and free
> i2c_priv.
> Without calling atmel_i2c_flush_queue() in the error path, the pending work
> would execute after i2c_priv is freed. This would also leak the dynamically
> allocated work_data.
> >
> > + return ret;
> > +
> > +err_list_del:
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > + list_del(&i2c_priv->i2c_client_list_node);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > return ret;
> > }
> Does the atmel_sha204a_remove() function need an update to remove the i2c
> client from the global list?
> Since the client is added during probe, it appears it would be left dangling
> in the list when the device is unbound and devres frees i2c_priv,
> potentially causing a crash if the list is traversed later.
> This was fixed later in the series by commit "crypto: atmel-sha204a -
> switch to module_i2c_driver".
Chronologically this is the wrong order. I moved this patch down to the end.
The issue should be fully covered. In this way, Atmel SHA204a devices are
added to the i2c list, when they can configure not being available for ECDH
operations. This is a preparation for future capabilities
(as in the initial roughly splitted series).
[PATCH 07/12] crypto: atmel-sha204a - switch to module_i2c_driver
> commit e46024ee80d35e3759c2e5365d027bec6fb2ea51
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-sha204a - switch to module_i2c_driver
> Replace custom module init/exit functions with module_i2c_driver() for
> driver registration. Update remove path to unregister the client from the
> shared I2C management list before flushing pending work and cleaning up
> sysfs and hwrng resources.
> > Update remove path to unregister the client from the shared I2C management
> > list before flushing pending work and cleaning up sysfs and hwrng
> > resources.
> >
> > No functional change intended.
> This isn't a bug, but this patch appears to fix a list corruption regression.
> Before this patch, unloading the module would leave a freed i2c_priv structure
> linked into the shared atmel_i2c_mgmt.i2c_client_list. Does adding the
> atmel_i2c_unregister_client() call fix this dangling pointer issue, making
> the "No functional change intended" claim inaccurate?
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index cdfdcf2e43a7d..613ed5e7b3f61 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -216,6 +216,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
> > struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> >
> > devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
> > +
> > + atmel_i2c_unregister_client(i2c_priv);
> The commit message states the intent to unregister the client from the shared
> list before cleaning up hwrng resources. Since devm_hwrng_unregister()
> blocks until active synchronous readers complete, does placing
> atmel_i2c_unregister_client() after it leave the client exposed in the shared
> list while the device is actively being torn down?
> Could another subsystem discover and bind to this partially removed client
> during the blocking window, resulting in memory corruption once the unbind
> completes?
> Should atmel_i2c_unregister_client() be moved to the beginning of the remove
> function to match the design described in the commit message?
> > atmel_i2c_flush_queue();
> >
> > sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
> Additionally, I noticed a potential use-after-free regression in the probe
> error path.
> In atmel_sha204a_probe(), if sysfs_create_group() fails, the function
> returns an error:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_probe() {
> [ ... ]
> ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
> if (ret) {
> dev_warn(&client->dev, "failed to register RNG (%d)\n", ret);
> goto err_list_del;
> }
> ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
> if (ret) {
> dev_err(&client->dev, "failed to register sysfs entry\n");
> goto err_list_del;
> }
> return ret;
> err_list_del:
> spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> list_del(&i2c_priv->i2c_client_list_node);
> spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> return ret;
> }
> At this point, the RNG has already been registered and exposed to user-space
> via devm_hwrng_register(). A concurrent user-space read could have initiated
> an asynchronous read, which queues a work item onto atmel_wq.
> When probe() returns an error, devres will immediately unregister the RNG and
> free the i2c_priv structure. Since the pending work on atmel_wq is never
> flushed or canceled, will it dereference the freed i2c_priv when it
> eventually executes?
> Should atmel_i2c_flush_queue() be explicitly called in the probe error path,
> just as it is in remove()?
> I also noticed a potential kernel heap memory leak to user-space in the
> asynchronous read path.
> In atmel_sha204a_rng_done(), if an asynchronous I2C transaction fails,
> the buffer is unconditionally cached:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_rng_done() {
> [ ... ]
> if (status)
> dev_warn_ratelimited(&i2c_priv->client->dev,
> "i2c transaction failed (%d)\n",
> status);
> rng->priv = (unsigned long)work_data;
> [ ... ]
> }
> If status != 0, the work_data->cmd.data buffer is left uninitialized or
> partially written.
> On the next call to atmel_sha204a_rng_read_nonblocking(), the code blindly
> copies up to 32 bytes of this buffer:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_rng_read_nonblocking() {
> [ ... ]
> if (rng->priv) {
> work_data = (struct atmel_i2c_work_data *)rng->priv;
> max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
> memcpy(data, &work_data->cmd.data[RSP_DATA_IDX], max);
> rng->priv = 0;
> } else {
> work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
> [ ... ]
> }
> Because work_data is allocated without zeroing memory, does this copy
> uninitialized kernel heap memory into the hwrng output data buffer?
> Could this allow an attacker to leak kernel heap memory to user-space via
> /dev/hwrng by inducing I2C read failures?
Chronologically this is the wrong order. I moved this patch down to the end.
Add different error labels in probe to address failing sysfs_create_group().
Fix kernel heap information leak in a separate patch.
[PATCH 08/12] crypto: atmel-ecc - switch to module_i2c_driver
> commit 1156053e97f2cc049a62750ad595b29dc8a4e921
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> Date: Sun May 17 18:06:35 2026 +0000
> crypto: atmel-ecc - switch to module_i2c_driver
>
> Replace custom module init/exit functions with module_i2c_driver() for
> simplified driver registration.
>
> Initialization of the shared I2C client management structure is handled by
> the core driver and no longer performed in the ECC module init path.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index e5dd77008b97f..dcfc09d244973 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -392,21 +392,7 @@ static struct i2c_driver atmel_ecc_driver = {
> > .id_table = atmel_ecc_id,
> > };
> >
> > -static int __init atmel_ecc_init(void)
> > -{
> > - spin_lock_init(&atmel_i2c_mgmt.i2c_list_lock);
> > - INIT_LIST_HEAD(&atmel_i2c_mgmt.i2c_client_list);
> > - return i2c_add_driver(&atmel_ecc_driver);
> > -}
> > -
> > -static void __exit atmel_ecc_exit(void)
> > -{
> > - atmel_i2c_flush_queue();
> > - i2c_del_driver(&atmel_ecc_driver);
> > -}
> > -
> > -module_init(atmel_ecc_init);
> > -module_exit(atmel_ecc_exit);
> > +module_i2c_driver(atmel_ecc_driver);
> While this change simplifies driver registration, does this driver have a
> pre-existing regression involving a use-after-free in its remove path?
again, "pre-existing regression". I already addressed this topic above.
> Looking at the driver's remove path, it can return early if the client is
> busy:
> drivers/crypto/atmel-ecc.c:atmel_ecc_remove() {
> struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> if (atomic_read(&i2c_priv->tfm_count)) {
> dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
> return;
> }
> ...
> }
> When atmel_ecc_remove() returns, the driver core unconditionally frees all
> devres-managed memory, including i2c_priv, which is allocated via
> devm_kmalloc() during probe.
> If there are open crypto transforms holding pointers to this freed memory,
> will subsequently closing a busy transform cause atmel_ecdh_exit_tfm() to
> access the freed i2c_priv->tfm_count and trigger a use-after-free of
> i2c_priv?
> Furthermore, does the early return leave the atmel_ecdh_nist_p256 algorithm
> permanently registered with dangling pointers to module text?
> Additionally, could a time-of-check to time-of-use race occur here?
> If atmel_ecc_remove() sees tfm_count == 0, a concurrent transform allocation
> might acquire the client just before it is removed from the global list,
> leading to the same devres use-after-free.
> Note that later commits like "crypto: atmel-ecc - simplify remove path and
> relax busy handling" remove the early return, but the fundamental
> use-after-free persists because devres still frees i2c_priv while active
> transforms hold references to it.
> Is there also a pre-existing regression in the probe error path that can lead
> to a use-after-free?
> The client is added to the global atmel_i2c_mgmt.i2c_client_list before
> crypto_register_kpp() is called:
> drivers/crypto/atmel-ecc.c:atmel_ecc_probe() {
> ...
> spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> list_add_tail(&i2c_priv->i2c_client_list_node,
> &atmel_i2c_mgmt.i2c_client_list);
> spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> if (ret) {
> ...
> goto err_list_del;
> }
> ...
> }
> If registration fails (for example, because another device already registered
> the algorithm), the error path removes the client from the list and returns
> an error. This causes the driver core to free the devm-allocated i2c_priv.
> While the client was briefly in the global list, could a concurrent transform
> allocation acquire it via atmel_ecc_i2c_client_alloc()?
> This would leave the concurrent transform holding a dangling pointer to the
> freed i2c_priv memory, resulting in a use-after-free when cryptographic
> operations are submitted.
> >
> > MODULE_AUTHOR("Tudor Ambarus");
> > MODULE_DESCRIPTION("Microchip / Atmel ECC (I2C) driver");
All of the issues are supposed to be already addressed here. Let's have a look
into next Sahiko iteration. Patch updated.
[PATCH 09/12] crypto: atmel-ecc - simplify remove path and relax busy handling
> commit e814c428ad784badd861d6adbe0e2dbf99beae12
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-ecc - simplify remove path and relax busy handling
> This commit simplifies the removal path of the atmel-ecc driver by removing
> the early exit logic when active transform users are detected. Instead, it
> issues a warning and continues with the full teardown of the crypto
> registration and device cleanup.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index dcfc09d244973..ce7a2e750ba88 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -346,21 +346,14 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > static void atmel_ecc_remove(struct i2c_client *client)
> > {
> > - struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > + struct atmel_i2c_client_priv *i2c_priv;
> >
> > - /* Return EBUSY if i2c client already allocated. */
> > - if (atomic_read(&i2c_priv->tfm_count)) {
> [ ... ]
> > - dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
> > + i2c_priv = i2c_get_clientdata(client);
> > + if (WARN_ON(!i2c_priv))
> > return;
> > - }
> > +
> > + if (atomic_read(&i2c_priv->tfm_count))
> > + dev_warn(&client->dev, "Device is busy, remove it anyhow\n");
> Could this still lead to a use-after-free on i2c_priv?
> The i2c_priv structure is allocated using devres during probe. When
> atmel_ecc_remove() completes, the Linux device model automatically triggers
> devres cleanup, which unconditionally frees i2c_priv.
> If active crypto transforms (indicated by tfm_count > 0) retain pointers to
> the device context, won't they dereference the freed i2c_priv when enqueuing
> new crypto operations or when they are eventually closed?
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> Does unregistering the statically allocated crypto algorithm while busy
> trigger a warning and corrupt its reference count?
> Because the atmel_ecdh_nist_p256 structure is statically allocated, it has
> no cra_destroy callback. Calling crypto_unregister_alg() while active
> transforms exist (cra_refcnt > 1) deterministically hits the safety check
> WARN_ON(!alg->cra_destroy && refcount_read(&alg->cra_refcnt) != 1) in
> crypto/algapi.c.
> Additionally, if the device is subsequently re-probed, crypto_register_kpp()
> will overwrite the structure's cra_refcnt to 1. When the original, orphaned
> transforms finally close and call crypto_alg_put(), they will decrement the
> newly reset refcount to 0. Future allocations of the algorithm would then
> trigger refcount_inc() on a zeroed refcount, permanently corrupting the
> algorithm's state.
> >
> > - atmel_i2c_flush_queue();
Patch dropped.
[PATCH 10/12] crypto: atmel-sha204a - guard remove path against
missing client data
> commit 940da01227a9a17cd454a0d965228cb8cdc7880a
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel-sha204a - guard remove path against missing client data
> This commit attempts to prevent dereferencing invalid client state during
> driver removal by adding a NULL check for the I2C client private data.
> It also mentions retrieving the data only after sysfs cleanup.
> > Retrieve the I2C client private data in atmel_sha204a_remove() only
> > after sysfs cleanup and add a NULL check before continuing device
> > teardown.
> Is this description accurate? The patch places the i2c_get_clientdata()
> call at the top of the function and leaves sysfs_remove_group() at the
> bottom, which does not appear to reorder these operations.
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -213,7 +213,11 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> >
> > static void atmel_sha204a_remove(struct i2c_client *client)
> > {
> > - struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > + struct atmel_i2c_client_priv *i2c_priv;
> > +
> > + i2c_priv = i2c_get_clientdata(client);
> > + if (WARN_ON(!i2c_priv))
> > + return;
> Since the driver core guarantees atmel_sha204a_remove() is only called if
> atmel_sha204a_probe() completed successfully, can i2c_priv ever actually be
> NULL here?
> If this early return were to trigger, would it skip the rest of the teardown
> sequence?
> Bypassing mandatory cleanup like sysfs_remove_group() and
> atmel_i2c_flush_queue() would leave dangling sysfs files and uncancelled
> work queues.
> Could the crash this patch attempts to mitigate actually be caused by a
> pre-existing issue in the probe() error path?
> If sysfs_create_group() fails after devm_hwrng_register() succeeds in
> atmel_sha204a_probe(), the function returns an error and devres cleans up
> i2c_priv.
> However, if a concurrent non-blocking read queued asynchronous work to
> atmel_wq, this work is not flushed because atmel_i2c_flush_queue() is
> omitted from the probe error path.
> When atmel_i2c_work_handler() executes later, would it dereference the
> already freed i2c_priv structure?
Patch dropped.
[PATCH 11/12] crypto: atmel - move i2c client selection to core driver
> commit ec87e048a52b48d6d9799216ee15e9f3c0e86cbc
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel - move i2c client selection to core driver
> Moves the I2C client selection logic from the ECC-specific driver into the
> shared Atmel I2C core. This allows the load-balancing logic to be reused
> across other Atmel crypto drivers.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index ce7a2e750ba88..f877f236552f7 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > -static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
> > -{
> [ ... ]
> > - return client;
> > -}
> > -
> > static void atmel_ecc_i2c_client_free(struct i2c_client *client)
> > {
> > struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > @@ -249,7 +214,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
> This isn't a bug, but the patch exports atmel_i2c_client_alloc() to share
> allocation logic while leaving the corresponding free function static here.
> Since the goal is for other Atmel crypto drivers to use the new shared
> allocation API, won't they be forced to manually access i2c_priv->tfm_count
> to decrement the counter? Should we also move and export a shared free
> function to create a symmetric API lifecycle?
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > index 861af52d7a88b..4beab68997c4f 100644
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> [ ... ]
> > +struct i2c_client *atmel_i2c_client_alloc(void)
> > +{
> > + struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
> > + struct i2c_client *client = ERR_PTR(-ENODEV);
> > + int min_tfm_cnt = INT_MAX;
> > + int tfm_cnt;
> > +
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > + if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
> > + i2c_client_list_node) {
> > + tfm_cnt = atomic_read(&i2c_priv->tfm_count);
> > + if (tfm_cnt < min_tfm_cnt) {
> > + min_tfm_cnt = tfm_cnt;
> > + min_i2c_priv = i2c_priv;
> > + }
> > + if (!min_tfm_cnt)
> > + break;
> > + }
> > +
> > + if (min_i2c_priv) {
> > + atomic_inc(&min_i2c_priv->tfm_count);
> > + client = min_i2c_priv->client;
> > + }
> > +
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > + return client;
> > +}
> Could this result in a use-after-free if the device is unbound?
> If the underlying I2C device is asynchronously unbound while a crypto
> transformation is active, the driver's remove callback executes. The remove
> callback unregisters the algorithm to prevent new transformations, but it
> does not block waiting for active ones to be freed.
> When remove returns, the I2C subsystem frees the i2c_client structure, and
> devres automatically frees the allocated i2c_priv structure. The active
> transformation would retain the dangling ctx->client pointer. Any subsequent
> crypto operation would enqueue a work item that dereferences the freed client
> and accesses freed memory.
> Should we take a reference on the device via get_device(&client->dev) before
> returning it here to ensure it stays alive?
No. atmel_ecc_remove() was reworked, this issue should be addressed now.
> > +EXPORT_SYMBOL(atmel_i2c_client_alloc);
> This isn't a bug, but this function internally acquires locks and iterates
> over atmel_i2c_mgmt, which is exported via EXPORT_SYMBOL_GPL.
> Does exporting this as a non-GPL symbol allow non-GPL modules to bypass the
> GPL restriction of the underlying shared data structure? Should this be
> exported using EXPORT_SYMBOL_GPL to maintain consistency?
Renamed patch title. I changed to EXPORT_SYMBOL_GPL. The alloc and free
functions are now migrated.
[PATCH 12/12] crypto: atmel - add capability-based I2C client selection
> commit 67dd11917ea1c6943f1088bb31e4aa5da0cb31f4
> Author: Lothar Rubusch <l.rubusch@gmail•com>
> crypto: atmel - add capability-based I2C client selection
> This commit introduces capability-based filtering for the I2C client
> allocator. Devices now advertise supported features during probe, and the
> allocator selects clients that match the requested capabilities. This
> prepares the driver for using the hardware beyond a single algorithm type.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -286,6 +286,8 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > + i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
> > +
> > spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_add_tail(&i2c_priv->i2c_client_list_node,
> > &atmel_i2c_mgmt.i2c_client_list);
> This problem wasn't introduced by this patch, but there appears to be a race
Sure
> condition if a device probe fails.
> In atmel_ecc_probe(), the i2c_priv object is added to the global list before
> all initialization steps are complete:
> atmel_ecc_probe()
> list_add_tail(&i2c_priv->i2c_client_list_node, ...)
> ...
> ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> if (ret) {
> goto err_list_del;
> If multiple Atmel ECC devices are present, the second device will fail to
> register the same static algorithm and return an error.
> During this window, could a concurrent call to atmel_i2c_client_alloc()
> select this incompletely probed device for a new crypto transformation?
> If so, when the probe function hits the error path, removes the device from
> the list, and devres frees i2c_priv, the crypto transformation would be left
> with a dangling pointer, potentially causing a use-after-free on subsequent
> operations. This same pattern also appears to be present in
> atmel_sha204a_probe().
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> [ ... ]
> > @@ -73,6 +73,9 @@ struct i2c_client *atmel_i2c_client_alloc(void)
> >
> > list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
> > i2c_client_list_node) {
> > + if (!(i2c_priv->caps & BIT(cap)))
> > + continue;
> > +
> > tfm_cnt = atomic_read(&i2c_priv->tfm_count);
> > if (tfm_cnt < min_tfm_cnt) {
> This problem wasn't introduced by this patch, but it seems active
Already mentioned...
> cryptographic transformations might not properly secure the lifetime of the
> underlying I2C device.
> The atmel_i2c_client_alloc() function increments a local tfm_count to track
> active references, but does not pin the device structure itself or delay its
> destruction.
> If a device is hot-unplugged or manually unbound via sysfs, the driver's
> remove callback executes. Since i2c_priv is tied to the device's lifecycle
> via devres, it will be automatically freed upon returning from the remove
> function. The unbind process does not block waiting for tfm_count to drop to
> zero.
> Does this mean any concurrently active crypto TFM previously allocated via
> atmel_i2c_client_alloc() will maintain a dangling pointer to these freed
> structures?
> When the TFM is later destroyed or performs an operation, it might
> dereference the freed client and execute atomic_dec() on freed memory.
The time-of-check to time-of-use situation should be addressed, as also the
multiple hardware devices competing for the algo structure.
Best,
L
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit
2026-05-12 22:43 ` [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit Lothar Rubusch
@ 2026-06-01 9:03 ` Marco Crivellari
2026-06-04 7:22 ` Lothar Rubusch
0 siblings, 1 reply; 18+ messages in thread
From: Marco Crivellari @ 2026-06-01 9:03 UTC (permalink / raw)
To: l.rubusch
Cc: alexandre.belloni, claudiu.beznea, davem, herbert,
linux-arm-kernel, linux-crypto, linux-kernel, nicolas.ferre,
thorsten.blum, Marco Crivellari
Hi,
> Update workqueue initialization to use WQ_MEM_RECLAIM instead of
> WQ_PERCPU
Not sure if you're working on this series right now, but this must keep
the WQ_PERCPU flag. WQ_PERCPU has been added to mark explicitly mark
workqueue that are per-CPU (it is the complement of WQ_UNBOUND).
Thanks!
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit
2026-06-01 9:03 ` Marco Crivellari
@ 2026-06-04 7:22 ` Lothar Rubusch
0 siblings, 0 replies; 18+ messages in thread
From: Lothar Rubusch @ 2026-06-04 7:22 UTC (permalink / raw)
To: Marco Crivellari
Cc: alexandre.belloni, claudiu.beznea, davem, herbert,
linux-arm-kernel, linux-crypto, linux-kernel, nicolas.ferre,
thorsten.blum
Hi Marco,
On Mon, Jun 1, 2026 at 11:03 AM Marco Crivellari
<marco.crivellari@suse•com> wrote:
>
> Hi,
>
> > Update workqueue initialization to use WQ_MEM_RECLAIM instead of
> > WQ_PERCPU
>
> Not sure if you're working on this series right now, but this must keep
> the WQ_PERCPU flag. WQ_PERCPU has been added to mark explicitly mark
> workqueue that are per-CPU (it is the complement of WQ_UNBOUND).
>
Yes. I plan on splitting up the material presented in this series.
This particular patch was one of the things I was a unsure, if this
actually could be done.
Therefore it was already separted out. I highly appreciate your
feedback. I will drop this
change and leave WQ_PERCPU until I get there.
>
> Thanks!
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-06-04 7:23 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 22:43 [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support Lothar Rubusch
2026-05-12 22:43 ` [PATCH 01/12] crypto: atmel - introduce shared I2C client management Lothar Rubusch
2026-05-12 22:43 ` [PATCH 02/12] crypto: atmel - move capability-based client allocation into i2c core Lothar Rubusch
2026-05-12 22:43 ` [PATCH 03/12] crypto: atmel - remove obsolete CONFIG_OF guard Lothar Rubusch
2026-05-12 22:43 ` [PATCH 04/12] crypto: atmel - add per-device timing and match-data driven configuration Lothar Rubusch
2026-05-12 22:43 ` [PATCH 05/12] crypto: atmel - move RNG support into common i2c core Lothar Rubusch
2026-05-12 22:43 ` [PATCH 06/12] crypto: atmel - move EEPROM access " Lothar Rubusch
2026-05-12 22:43 ` [PATCH 07/12] crypto: atmel - expose CONFIG zone through sysfs Lothar Rubusch
2026-05-12 22:43 ` [PATCH 08/12] crypto: atmel - move device sanity check to core driver Lothar Rubusch
2026-05-12 22:43 ` [PATCH 09/12] crypto: atmel - check client data in remove callbacks Lothar Rubusch
2026-05-12 22:43 ` [PATCH 10/12] crypto: atmel - update workqueue flags and add flush on exit Lothar Rubusch
2026-06-01 9:03 ` Marco Crivellari
2026-06-04 7:22 ` Lothar Rubusch
2026-05-12 22:43 ` [PATCH 11/12] crypto: atmel - refactor and localize driver constants Lothar Rubusch
2026-05-12 22:43 ` [PATCH 12/12] crypto: atmel - add SHA256 ahash support Lothar Rubusch
2026-05-14 19:51 ` [PATCH 00/12] crypto: atmel - refactor common i2c support and " Thorsten Blum
2026-05-15 20:29 ` Lothar Rubusch
2026-05-19 20:47 ` Lothar Rubusch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox