Hi, Linus, On 06/28/2018 04:07 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 <linus.walleij@xxxxxxxxxx> > --- > ChangeLog v1->v2: > - kfree(cmd) was missed. Fix it with a goto construction. > - Coding style fixes. > --- > drivers/crypto/atmel-ecc.c | 60 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c > index f3322fae454e..659cb3cf37a9 100644 > --- a/drivers/crypto/atmel-ecc.c > +++ b/drivers/crypto/atmel-ecc.c > @@ -26,6 +26,7 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/random.h> > #include <linux/scatterlist.h> > #include <linux/slab.h> > #include <linux/workqueue.h> > @@ -610,6 +611,61 @@ 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; > + int ret; > + int i; > + u8 serial[9]; > + > + 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(&client->dev, > + "failed to read serial byte 0-3\n"); the message can fit in the previous line. > + goto out_free_cmd; > + } > + for (i = 0; i < 4; i++) > + serial[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(&client->dev, > + "failed to read serial byte 4-7\n"); > + goto out_free_cmd; > + } > + for (i = 0; i < 4; i++) > + serial[4 + i] = cmd->data[RSP_DATA_IDX + i]; > + unnecessary blank line. Some of these coding style issues can be catch with checkpatch.pl --strict. There are few coding style checks in the next 2 patches too. > + > + atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN); > + ret = atmel_ecc_send_receive(client, cmd); > + if (ret) { > + kfree(cmd); you will double free cmd, here and in out_free_cmd. Otherwise looks good. The device supports 4-byte or 32-byte reads. You can also do just one read of 32 bytes instead of three reads of four bytes each, but I can't tell which one is better. Thanks, Linus, ta > + dev_err(&client->dev, > + "failed to read serial byte 8\n"); > + goto out_free_cmd; > + } > + 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(&client->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]); > + > +out_free_cmd: > + kfree(cmd); > + return ret; > +} > + > static int device_sanity_check(struct i2c_client *client) > { > struct atmel_ecc_cmd *cmd; > @@ -692,6 +748,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(&driver_data.i2c_list_lock); > list_add_tail(&i2c_priv->i2c_client_list_node, > &driver_data.i2c_client_list); >