* Re: [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads
2026-02-15 20:51 [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads Thorsten Blum
@ 2026-02-17 11:01 ` Lothar Rubusch
2026-02-22 17:03 ` Lothar Rubusch
2026-02-28 8:48 ` Herbert Xu
2 siblings, 0 replies; 6+ messages in thread
From: Lothar Rubusch @ 2026-02-17 11:01 UTC (permalink / raw)
To: Thorsten Blum
Cc: Herbert Xu, David S. Miller, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, stable, linux-crypto, linux-arm-kernel,
linux-kernel
Hi, the change works (doesn't break behavior at least) verified on
hardware, LGTM.
I remember that time we had a small discussion on what is the right
approach with the return
handling, and at least me was unsure about it. If this puts it
straight I'll take it for me as take
away. Thank you Thorsten, and sorry for the fuzz.
Reviewed-by: Lothar Rubusch <l.rubusch@gmail.com>
Best,
L
On Sun, Feb 15, 2026 at 9:52 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Return -EINVAL from atmel_i2c_init_read_otp_cmd() on invalid addresses
> instead of -1. Since the OTP zone is accessed in 4-byte blocks, valid
> addresses range from 0 to OTP_ZONE_SIZE / 4 - 1. Fix the bounds check
> accordingly.
>
> In atmel_sha204a_otp_read(), propagate the actual error code from
> atmel_i2c_init_read_otp_cmd() instead of -1. Also, return -EIO instead
> of -EINVAL when the device is not ready.
>
> Cc: stable@vger.kernel.org
> Fixes: e05ce444e9e5 ("crypto: atmel-sha204a - add reading from otp zone")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
> ---
> drivers/crypto/atmel-i2c.c | 4 ++--
> drivers/crypto/atmel-sha204a.c | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> index 9688d116d07e..ba9d3f593601 100644
> --- a/drivers/crypto/atmel-i2c.c
> +++ b/drivers/crypto/atmel-i2c.c
> @@ -72,8 +72,8 @@ EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
>
> int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr)
> {
> - if (addr < 0 || addr > OTP_ZONE_SIZE)
> - return -1;
> + if (addr >= OTP_ZONE_SIZE / 4)
> + return -EINVAL;
>
> cmd->word_addr = COMMAND;
> cmd->opcode = OPCODE_READ;
> diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> index 0fcf4a39de27..6b4e2764523e 100644
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -94,9 +94,10 @@ static int atmel_sha204a_rng_read(struct hwrng *rng, void *data, size_t max,
> static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
> {
> struct atmel_i2c_cmd cmd;
> - int ret = -1;
> + int ret;
>
> - if (atmel_i2c_init_read_otp_cmd(&cmd, addr) < 0) {
> + ret = atmel_i2c_init_read_otp_cmd(&cmd, addr);
> + if (ret < 0) {
> dev_err(&client->dev, "failed, invalid otp address %04X\n",
> addr);
> return ret;
> @@ -106,7 +107,7 @@ static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
>
> if (cmd.data[0] == 0xff) {
> dev_err(&client->dev, "failed, device not ready\n");
> - return -EINVAL;
> + return -EIO;
> }
>
> memcpy(otp, cmd.data+1, 4);
> --
> Thorsten Blum <thorsten.blum@linux.dev>
> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads
2026-02-15 20:51 [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads Thorsten Blum
2026-02-17 11:01 ` Lothar Rubusch
@ 2026-02-22 17:03 ` Lothar Rubusch
2026-02-22 20:10 ` Thorsten Blum
2026-02-28 8:48 ` Herbert Xu
2 siblings, 1 reply; 6+ messages in thread
From: Lothar Rubusch @ 2026-02-22 17:03 UTC (permalink / raw)
To: Thorsten Blum
Cc: Herbert Xu, David S. Miller, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, stable, linux-crypto, linux-arm-kernel,
linux-kernel
Hi, find some comments below inlined.
On Sun, Feb 15, 2026 at 9:52 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Return -EINVAL from atmel_i2c_init_read_otp_cmd() on invalid addresses
> instead of -1. Since the OTP zone is accessed in 4-byte blocks, valid
> addresses range from 0 to OTP_ZONE_SIZE / 4 - 1. Fix the bounds check
> accordingly.
>
> In atmel_sha204a_otp_read(), propagate the actual error code from
> atmel_i2c_init_read_otp_cmd() instead of -1. Also, return -EIO instead
> of -EINVAL when the device is not ready.
>
> Cc: stable@vger.kernel.org
> Fixes: e05ce444e9e5 ("crypto: atmel-sha204a - add reading from otp zone")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
> ---
> drivers/crypto/atmel-i2c.c | 4 ++--
> drivers/crypto/atmel-sha204a.c | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> index 9688d116d07e..ba9d3f593601 100644
> --- a/drivers/crypto/atmel-i2c.c
> +++ b/drivers/crypto/atmel-i2c.c
> @@ -72,8 +72,8 @@ EXPORT_SYMBOL(atmel_i2c_init_read_config_cmd);
>
> int atmel_i2c_init_read_otp_cmd(struct atmel_i2c_cmd *cmd, u16 addr)
> {
> - if (addr < 0 || addr > OTP_ZONE_SIZE)
> - return -1;
> + if (addr >= OTP_ZONE_SIZE / 4)
> + return -EINVAL;
>
> cmd->word_addr = COMMAND;
> cmd->opcode = OPCODE_READ;
> diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> index 0fcf4a39de27..6b4e2764523e 100644
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -94,9 +94,10 @@ static int atmel_sha204a_rng_read(struct hwrng *rng, void *data, size_t max,
> static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
> {
> struct atmel_i2c_cmd cmd;
> - int ret = -1;
> + int ret;
>
> - if (atmel_i2c_init_read_otp_cmd(&cmd, addr) < 0) {
> + ret = atmel_i2c_init_read_otp_cmd(&cmd, addr);
> + if (ret < 0) {
> dev_err(&client->dev, "failed, invalid otp address %04X\n",
> addr);
> return ret;
Since I2C bus errors are caught here.
> @@ -106,7 +107,7 @@ static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
>
> if (cmd.data[0] == 0xff) {
> dev_err(&client->dev, "failed, device not ready\n");
> - return -EINVAL;
> + return -EIO;
The cmd.data holding 0xff here is not a bus error. AFAIR it can have
to do with the locking state, pre-initialization,
typically the atmel watchdog kicked in / timeout, etc - so the
response is invalid, although hardware connection (I2C) is
supposed to work. Currently the caller of this function does not
distinguish anyway.
But why is EIO preferable here, over EINVAL?
> }
>
> memcpy(otp, cmd.data+1, 4);
> --
> Thorsten Blum <thorsten.blum@linux.dev>
> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads
2026-02-22 17:03 ` Lothar Rubusch
@ 2026-02-22 20:10 ` Thorsten Blum
0 siblings, 0 replies; 6+ messages in thread
From: Thorsten Blum @ 2026-02-22 20:10 UTC (permalink / raw)
To: Lothar Rubusch
Cc: Herbert Xu, David S. Miller, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, stable, linux-crypto, linux-arm-kernel,
linux-kernel
On 22. Feb 2026, at 18:03, Lothar Rubusch wrote:
> On Sun, Feb 15, 2026 at 9:52 PM Thorsten Blum wrote:
>> Return -EINVAL from atmel_i2c_init_read_otp_cmd() on invalid addresses
>> instead of -1. Since the OTP zone is accessed in 4-byte blocks, valid
>> addresses range from 0 to OTP_ZONE_SIZE / 4 - 1. Fix the bounds check
>> accordingly.
>>
>> In atmel_sha204a_otp_read(), propagate the actual error code from
>> atmel_i2c_init_read_otp_cmd() instead of -1. Also, return -EIO instead
>> of -EINVAL when the device is not ready.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: e05ce444e9e5 ("crypto: atmel-sha204a - add reading from otp zone")
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> [...]
>>
>> @@ -106,7 +107,7 @@ static int atmel_sha204a_otp_read(struct i2c_client *client, u16 addr, u8 *otp)
>>
>> if (cmd.data[0] == 0xff) {
>> dev_err(&client->dev, "failed, device not ready\n");
>> - return -EINVAL;
>> + return -EIO;
> The cmd.data holding 0xff here is not a bus error. AFAIR it can have
> to do with the locking state, pre-initialization,
> typically the atmel watchdog kicked in / timeout, etc - so the
> response is invalid, although hardware connection (I2C) is
> supposed to work. Currently the caller of this function does not
> distinguish anyway.
>
> But why is EIO preferable here, over EINVAL?
AFAIK, -EINVAL is used for invalid arguments or bad input passed by the
caller, which is why the address range check returns -EINVAL.
-EIO signals an I/O error or communication failure, e.g., the caller
passed a valid address, but the device isn't ready yet, for whatever
reason. Maybe -EAGAIN or -EBUSY instead? -EIO seemed like the most
reasonable choice to me.
Since the error code will be propagated by my other patch [1], now would
probably be a good time to adjust it.
Thanks,
Thorsten
[1] https://lore.kernel.org/lkml/20260216074552.656814-1-thorsten.blum@linux.dev/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads
2026-02-15 20:51 [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads Thorsten Blum
2026-02-17 11:01 ` Lothar Rubusch
2026-02-22 17:03 ` Lothar Rubusch
@ 2026-02-28 8:48 ` Herbert Xu
2026-02-28 9:57 ` Thorsten Blum
2 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2026-02-28 8:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: David S. Miller, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Lothar Rubusch, stable, linux-crypto, linux-arm-kernel,
linux-kernel
On Sun, Feb 15, 2026 at 09:51:53PM +0100, Thorsten Blum wrote:
> Return -EINVAL from atmel_i2c_init_read_otp_cmd() on invalid addresses
> instead of -1. Since the OTP zone is accessed in 4-byte blocks, valid
> addresses range from 0 to OTP_ZONE_SIZE / 4 - 1. Fix the bounds check
> accordingly.
>
> In atmel_sha204a_otp_read(), propagate the actual error code from
> atmel_i2c_init_read_otp_cmd() instead of -1. Also, return -EIO instead
> of -EINVAL when the device is not ready.
>
> Cc: stable@vger.kernel.org
> Fixes: e05ce444e9e5 ("crypto: atmel-sha204a - add reading from otp zone")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Compile-tested only.
> ---
> drivers/crypto/atmel-i2c.c | 4 ++--
> drivers/crypto/atmel-sha204a.c | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: atmel-sha204a - Fix error codes in OTP reads
2026-02-28 8:48 ` Herbert Xu
@ 2026-02-28 9:57 ` Thorsten Blum
0 siblings, 0 replies; 6+ messages in thread
From: Thorsten Blum @ 2026-02-28 9:57 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Lothar Rubusch, stable, linux-crypto, linux-arm-kernel,
linux-kernel
On 28. Feb 2026, at 09:48, Herbert Xu wrote:
> On Sun, Feb 15, 2026 at 09:51:53PM +0100, Thorsten Blum wrote:
>> Return -EINVAL from atmel_i2c_init_read_otp_cmd() on invalid addresses
>> instead of -1. Since the OTP zone is accessed in 4-byte blocks, valid
>> addresses range from 0 to OTP_ZONE_SIZE / 4 - 1. Fix the bounds check
>> accordingly.
>>
>> In atmel_sha204a_otp_read(), propagate the actual error code from
>> atmel_i2c_init_read_otp_cmd() instead of -1. Also, return -EIO instead
>> of -EINVAL when the device is not ready.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: e05ce444e9e5 ("crypto: atmel-sha204a - add reading from otp zone")
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> Compile-tested only.
>> ---
>> drivers/crypto/atmel-i2c.c | 4 ++--
>> drivers/crypto/atmel-sha204a.c | 7 ++++---
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> Patch applied. Thanks.
Hi Herbert,
I also submitted [1], which combines this patch here and patch [2] after
Lothar suggested to squash them. Feel free to apply them separately or
together. Just FYI.
Thanks,
Thorsten
[1] https://lore.kernel.org/lkml/20260224225547.683713-2-thorsten.blum@linux.dev/
[2] https://lore.kernel.org/lkml/20260220133135.1122081-2-thorsten.blum@linux.dev/
^ permalink raw reply [flat|nested] 6+ messages in thread