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(&client->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(&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]); > > 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(&client->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(&client->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(&client->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(&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]); 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(&driver_data.i2c_list_lock); list_add_tail(&i2c_priv->i2c_client_list_node, &driver_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(&client->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(&client->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(&client->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(&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]); + 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(&driver_data.i2c_list_lock); list_add_tail(&i2c_priv->i2c_client_list_node, &driver_data.i2c_client_list); -- 2.17.0