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.wall...@linaro.org>
> ---
> 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);
>
