Re: [PATCH 7/9] crypto: atmel-ecc: Print out serial number
On Tue, Jun 12, 2018 at 3:13 PM Tudor Ambarus
wrote:
> > +#include
>
> includes should be ordered alphabetically.
OK fixed.
> > +static int atmel_ecc_get_serial(struct i2c_client *client)
> > +{
> > + struct atmel_ecc_cmd *cmd;
> > + u8 serial[9];
>
> int i, ret; before serial to avoid stack padding.
>
> > + int ret;
> > + int i;
Is your point trying to help the compiler to lay things out on
the stack? Isn't that premature optimization? I was under the
impression that we should leave this stuff to the compiler.
But sure, it is a small change so I did it anyways :)
> > +
> > + cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);
> > + if (!cmd)
> > + return -ENOMEM;
> > +
> > + atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_0_3);
> > + ret = atmel_ecc_send_receive(client, cmd);
> > + if (ret) {
>
> free(cmd);
Whoops added a goto contruction to do proper free:ing of
cmd, thanks.
> > + dev_err(>dev,
> > + "failed to read serial byte 0-3\n");
> > + return ret;
> > + }
> > + for (i = 0; i < 4; i++)
> > + serial[0+i] = cmd->data[RSP_DATA_IDX + i];
>
> serial[i]?
OK.
> > + atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_4_7);
>
> can't we read the serial_no once?
Sorry not following. Do you mean we need an accessor to
read several config words in succession? I think I read somewhere
that the device only supports reading one config word at the
time and the serial number is spread out over three config words...
> > + for (i = 0; i < 4; i++)
> > + serial[4+i] = cmd->data[RSP_DATA_IDX + i];
>
> spaces preferred around that '+'
OK.
> > + dev_info(>dev,
> > + "serial number: %02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
> > + serial[0], serial[1], serial[2], serial[3], serial[4],
> > + serial[5], serial[6], serial[7], serial[8]);
>
> Why do you need the serial number printed out?
Cuddly feeling I guess. If there is a problem with the device the
user might want to report the serial number to the
manufacturer?
Yours,
Linus Walleij
Re: [PATCH 7/9] crypto: atmel-ecc: Print out serial number
Hi, Linus,
On 06/05/2018 04:49 PM, Linus Walleij wrote:
This reads out the serial number of the crypto chip and prints it,
also toss this into the entropy pool as it is device-unique data.
Signed-off-by: Linus Walleij
---
drivers/crypto/atmel-ecc.c | 56 ++
1 file changed, 56 insertions(+)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 603e29bdcb97..fd8149313104 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
includes should be ordered alphabetically.
#include
#include
#include
@@ -616,6 +617,57 @@ static inline size_t atmel_ecc_wake_token_sz(u32
bus_clk_rate)
return DIV_ROUND_UP(no_of_bits, 8);
}
+static int atmel_ecc_get_serial(struct i2c_client *client)
+{
+ struct atmel_ecc_cmd *cmd;
+ u8 serial[9];
int i, ret; before serial to avoid stack padding.
+ int ret;
+ int i;
+
+ cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_0_3);
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
free(cmd);
+ dev_err(>dev,
+ "failed to read serial byte 0-3\n");
+ return ret;
+ }
+ for (i = 0; i < 4; i++)
+ serial[0+i] = cmd->data[RSP_DATA_IDX + i];
serial[i]?
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_4_7);
can't we read the serial_no once?
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
+ dev_err(>dev,
+ "failed to read serial byte 4-7\n");
+ return ret;
+ }
+ for (i = 0; i < 4; i++)
+ serial[4+i] = cmd->data[RSP_DATA_IDX + i];
spaces preferred around that '+'
+
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN);
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
+ dev_err(>dev,
+ "failed to read serial byte 8\n");
+ return ret;
+ }
+ serial[8] = cmd->data[RSP_DATA_IDX];
+
+ /* This is device-unique data so it goes into the entropy pool */
+ add_device_randomness(serial, sizeof(serial));
+
+ dev_info(>dev,
+"serial number: %02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
+serial[0], serial[1], serial[2], serial[3], serial[4],
+serial[5], serial[6], serial[7], serial[8]);
Why do you need the serial number printed out?
+ return 0;
+}
+
static int device_sanity_check(struct i2c_client *client)
{
struct atmel_ecc_cmd *cmd;
@@ -700,6 +752,10 @@ static int atmel_ecc_probe(struct i2c_client *client,
if (ret)
return ret;
+ ret = atmel_ecc_get_serial(client);
+ if (ret)
+ return ret;
+
spin_lock(_data.i2c_list_lock);
list_add_tail(_priv->i2c_client_list_node,
_data.i2c_client_list);
[PATCH 7/9] crypto: atmel-ecc: Print out serial number
This reads out the serial number of the crypto chip and prints it,
also toss this into the entropy pool as it is device-unique data.
Signed-off-by: Linus Walleij
---
drivers/crypto/atmel-ecc.c | 56 ++
1 file changed, 56 insertions(+)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 603e29bdcb97..fd8149313104 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -616,6 +617,57 @@ static inline size_t atmel_ecc_wake_token_sz(u32
bus_clk_rate)
return DIV_ROUND_UP(no_of_bits, 8);
}
+static int atmel_ecc_get_serial(struct i2c_client *client)
+{
+ struct atmel_ecc_cmd *cmd;
+ u8 serial[9];
+ int ret;
+ int i;
+
+ cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_0_3);
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
+ dev_err(>dev,
+ "failed to read serial byte 0-3\n");
+ return ret;
+ }
+ for (i = 0; i < 4; i++)
+ serial[0+i] = cmd->data[RSP_DATA_IDX + i];
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_4_7);
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
+ dev_err(>dev,
+ "failed to read serial byte 4-7\n");
+ return ret;
+ }
+ for (i = 0; i < 4; i++)
+ serial[4+i] = cmd->data[RSP_DATA_IDX + i];
+
+
+ atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN);
+ ret = atmel_ecc_send_receive(client, cmd);
+ if (ret) {
+ dev_err(>dev,
+ "failed to read serial byte 8\n");
+ return ret;
+ }
+ serial[8] = cmd->data[RSP_DATA_IDX];
+
+ /* This is device-unique data so it goes into the entropy pool */
+ add_device_randomness(serial, sizeof(serial));
+
+ dev_info(>dev,
+"serial number: %02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
+serial[0], serial[1], serial[2], serial[3], serial[4],
+serial[5], serial[6], serial[7], serial[8]);
+ return 0;
+}
+
static int device_sanity_check(struct i2c_client *client)
{
struct atmel_ecc_cmd *cmd;
@@ -700,6 +752,10 @@ static int atmel_ecc_probe(struct i2c_client *client,
if (ret)
return ret;
+ ret = atmel_ecc_get_serial(client);
+ if (ret)
+ return ret;
+
spin_lock(_data.i2c_list_lock);
list_add_tail(_priv->i2c_client_list_node,
_data.i2c_client_list);
--
2.17.0
