[PATCH 5/9 v2] crypto: atmel-ecc: Create a config word reader

[Linus Walleij]

Rewrite the function atmel_ecc_init_read_cmd() into a more
general atmel_ecc_init_read_config_word() function to read
any word from the configuration zone, and use this
parameterized with what we want to read out.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Rebased
---
 drivers/crypto/atmel-ecc.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 98f9322ca303..2ec570d06a27 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -136,16 +136,13 @@ static void atmel_ecc_checksum(struct atmel_ecc_cmd *cmd)
*crc16 = atmel_ecc_crc16(0, data, len);
 }
 
-static void atmel_ecc_init_read_cmd(struct atmel_ecc_cmd *cmd)
+static void atmel_ecc_init_read_config_word(struct atmel_ecc_cmd *cmd,
+   u16 config_word)
 {
cmd->word_addr = COMMAND;
cmd->opcode = OPCODE_READ;
-   /*
-* Read the word from Configuration zone that contains the lock bytes
-* (UserExtra, Selector, LockValue, LockConfig).
-*/
cmd->param1 = CONFIG_ZONE;
-   cmd->param2 = CONFIG_ZONE_FOOTER;
+   cmd->param2 = config_word;
cmd->count = READ_COUNT;
 
atmel_ecc_checksum(cmd);
@@ -626,7 +623,7 @@ static int device_sanity_check(struct i2c_client *client)
if (!cmd)
return -ENOMEM;
 
-   atmel_ecc_init_read_cmd(cmd);
+   atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_FOOTER);
 
ret = atmel_ecc_send_receive(client, cmd);
if (ret) {
-- 
2.17.0

[PATCH 9/9 v2] crypto: atmel-ecc: Break out lock check helper

[Linus Walleij]

This breaks out a lock status checker to be used with further
refactorings.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Rebased
---
 drivers/crypto/atmel-ecc.c | 38 ++
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 0dfea6eadc56..549ec7190287 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -666,7 +666,9 @@ static int atmel_ecc_get_serial(struct i2c_client *client)
return ret;
 }
 
-static int device_sanity_check(struct i2c_client *client)
+static int atmel_ecc_get_lock_status(struct i2c_client *client,
+bool *config_is_locked,
+bool *otp_data_is_locked)
 {
struct atmel_ecc_cmd *cmd;
int ret;
@@ -680,21 +682,49 @@ static int device_sanity_check(struct i2c_client *client)
ret = atmel_ecc_send_receive(client, cmd);
if (ret) {
dev_err(>dev,
-   "failed to send ECC init command\n");
+   "failed to send config read command\n");
goto free_cmd;
}
 
+   /* According to datasheet anything else than 0x55 means "locked" */
+   if (cmd->data[RSP_DATA_IDX+3] == 0x55)
+   *config_is_locked = false;
+   else
+   *config_is_locked = true;
+
+   if (cmd->data[RSP_DATA_IDX+2] == 0x55)
+   *otp_data_is_locked = false;
+   else
+   *otp_data_is_locked = true;
+
+free_cmd:
+   kfree(cmd);
+   return ret;
+}
+
+static int device_sanity_check(struct i2c_client *client)
+{
+   struct atmel_ecc_cmd *cmd;
+   bool config_locked;
+   bool otp_data_locked;
+   int ret;
+
/*
 * It is vital that the Configuration, Data and OTP zones be locked
 * prior to release into the field of the system containing the device.
 * Failure to lock these zones may permit modification of any secret
 * keys and may lead to other security problems.
 */
-   if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+   ret = atmel_ecc_get_lock_status(client, _locked,
+   _data_locked);
+   if (ret)
+   return ret;
+
+   if (!config_locked) {
dev_err(>dev, "configuration zone is unlocked\n");
ret = -ENOTSUPP;
}
-   if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+   if (!otp_data_locked) {
dev_err(>dev, "data and OTP zones are unlocked\n");
ret = -ENOTSUPP;
}
-- 
2.17.0

[PATCH 1/9 v2] crypto: atmel-ecc: Make available for other platforms

[Linus Walleij]

This is a pure I2C driver, and this device appears on the
96boards Secure96 mezzanine card, so we want to enable the
driver on other devices. Cut the Kconfig limitations to
Atmel SoC only.

Reviewed-by: Tudor Ambarus 
Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Collect Tudor's review tag.
---
 drivers/crypto/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 43cccf6aff61..b4dd64d2d4ca 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -519,7 +519,6 @@ config CRYPTO_DEV_ATMEL_SHA
 
 config CRYPTO_DEV_ATMEL_ECC
tristate "Support for Microchip / Atmel ECC hw accelerator"
-   depends on ARCH_AT91 || COMPILE_TEST
depends on I2C
select CRYPTO_ECDH
select CRC16
-- 
2.17.0

[PATCH 4/9 v2] crypto: atmel-ecc: Provide config zone defines

[Linus Walleij]

The config zone has 0x16 words of 4 bytes each, so provide
some basic defines so that we can address these individually.
Rename the last word to "footer", this is where we currently
look to see if the configuration is locked.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Diet the defines down to the ones used in this patch
  set.
---
 drivers/crypto/atmel-ecc.c | 2 +-
 drivers/crypto/atmel-ecc.h | 9 +++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index baef0d07164d..98f9322ca303 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -145,7 +145,7 @@ static void atmel_ecc_init_read_cmd(struct atmel_ecc_cmd 
*cmd)
 * (UserExtra, Selector, LockValue, LockConfig).
 */
cmd->param1 = CONFIG_ZONE;
-   cmd->param2 = DEVICE_LOCK_ADDR;
+   cmd->param2 = CONFIG_ZONE_FOOTER;
cmd->count = READ_COUNT;
 
atmel_ecc_checksum(cmd);
diff --git a/drivers/crypto/atmel-ecc.h b/drivers/crypto/atmel-ecc.h
index 25232c8abcc2..d941c4f3d28f 100644
--- a/drivers/crypto/atmel-ecc.h
+++ b/drivers/crypto/atmel-ecc.h
@@ -89,8 +89,13 @@ static const struct {
 #define RSP_DATA_IDX   1 /* buffer index of data in response */
 #define DATA_SLOT_22 /* used for ECDH private key */
 
-/* Definitions for the device lock state */
-#define DEVICE_LOCK_ADDR   0x15
+/* Definitions for the configuration zone words, these are 4 bytes each */
+#define CONFIG_ZONE_SERIAL_0_3 0x00
+#define CONFIG_ZONE_REVISION   0x01
+#define CONFIG_ZONE_SERIAL_4_7 0x02
+#define CONFIG_ZONE_SERIAL_8_I2CEN 0x03
+#define CONFIG_ZONE_I2C_OTP0x04
+#define CONFIG_ZONE_FOOTER 0x15
 #define LOCK_VALUE_IDX (RSP_DATA_IDX + 2)
 #define LOCK_CONFIG_IDX(RSP_DATA_IDX + 3)
 
-- 
2.17.0

[PATCH 2/9 v2] crypto: atmel-ecc: Just warn on missing clock frequency

[Linus Walleij]

The Atmel ECC driver contains a check for the I2C bus clock
frequency, so as to check that the I2C adapter in use
satisfies the device specs.

If the device is connected to a device tree node that does not
contain a clock frequency setting, such as an I2C mux or gate,
this blocks the probe. Make the probe continue with a warning
if no clock frequency can be found, assuming it is safe.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Instead of silently ignoring the missing clock frequency,
  issue a warning and continue.
---
 drivers/crypto/atmel-ecc.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index e66f18a0ddd0..33773920e4bf 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -659,12 +659,9 @@ static int atmel_ecc_probe(struct i2c_client *client,
 
ret = of_property_read_u32(client->adapter->dev.of_node,
   "clock-frequency", _clk_rate);
-   if (ret) {
-   dev_err(dev, "of: failed to read clock-frequency property\n");
-   return ret;
-   }
-
-   if (bus_clk_rate > 100L) {
+   if (ret)
+   dev_warn(dev, "i2c host missing clock frequency information\n");
+   else if (bus_clk_rate > 100L) {
dev_err(dev, "%d exceeds maximum supported clock frequency 
(1MHz)\n",
bus_clk_rate);
return -EINVAL;
-- 
2.17.0

[PATCH 8/9 v2] crypto: atmel-ecc: Detail what is unlocked

[Linus Walleij]

Instead of just providing a broad error message about the
chip being unlocked provide details on what is unlocked,
one line per thing that can be locked: data and OTP and
configuration are locked independently. Loose the
overzealous defines.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Rebased
---
 drivers/crypto/atmel-ecc.c | 8 ++--
 drivers/crypto/atmel-ecc.h | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 659cb3cf37a9..0dfea6eadc56 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -690,8 +690,12 @@ static int device_sanity_check(struct i2c_client *client)
 * Failure to lock these zones may permit modification of any secret
 * keys and may lead to other security problems.
 */
-   if (cmd->data[LOCK_CONFIG_IDX] || cmd->data[LOCK_VALUE_IDX]) {
-   dev_err(>dev, "Configuration or Data and OTP zones are 
unlocked!\n");
+   if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+   dev_err(>dev, "configuration zone is unlocked\n");
+   ret = -ENOTSUPP;
+   }
+   if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+   dev_err(>dev, "data and OTP zones are unlocked\n");
ret = -ENOTSUPP;
}
 
diff --git a/drivers/crypto/atmel-ecc.h b/drivers/crypto/atmel-ecc.h
index 4458585ab588..42b724256f93 100644
--- a/drivers/crypto/atmel-ecc.h
+++ b/drivers/crypto/atmel-ecc.h
@@ -97,8 +97,6 @@ static const struct {
 #define CONFIG_ZONE_SERIAL_8_I2CEN 0x03
 #define CONFIG_ZONE_I2C_OTP0x04
 #define CONFIG_ZONE_FOOTER 0x15
-#define LOCK_VALUE_IDX (RSP_DATA_IDX + 2)
-#define LOCK_CONFIG_IDX(RSP_DATA_IDX + 3)
 
 /*
  * Wake High delay to data communication (microseconds). SDA should be stable
-- 
2.17.0

[PATCH 6/9 v2] crypto: atmel-ecc: Marshal the command while sending

[Linus Walleij]

Instead of casting the struct for the command into (u8 *)
which is problematic in many ways, and instead of
calculating the CRC sum in a separate function, marshal,
checksum and send the command in one single function.

Instead of providing the length of the whole command
in defines, it makes more sense to provide the length
of the data buffer used with the command.

This avoids the hazzle to try to keep the command
structure in the device endianness, we fix up the
endianness when marshalling the command instead.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Rebase on other changes.
---
 drivers/crypto/atmel-ecc.c | 72 ++
 drivers/crypto/atmel-ecc.h | 13 +++
 2 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 2ec570d06a27..f3322fae454e 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -113,29 +113,6 @@ struct atmel_ecc_work_data {
struct atmel_ecc_cmd cmd;
 };
 
-static u16 atmel_ecc_crc16(u16 crc, const u8 *buffer, size_t len)
-{
-   return cpu_to_le16(bitrev16(crc16(crc, buffer, len)));
-}
-
-/**
- * atmel_ecc_checksum() - Generate 16-bit CRC as required by ATMEL ECC.
- * CRC16 verification of the count, opcode, param1, param2 and data bytes.
- * The checksum is saved in little-endian format in the least significant
- * two bytes of the command. CRC polynomial is 0x8005 and the initial register
- * value should be zero.
- *
- * @cmd : structure used for communicating with the device.
- */
-static void atmel_ecc_checksum(struct atmel_ecc_cmd *cmd)
-{
-   u8 *data = >count;
-   size_t len = cmd->count - CRC_SIZE;
-   u16 *crc16 = (u16 *)(data + len);
-
-   *crc16 = atmel_ecc_crc16(0, data, len);
-}
-
 static void atmel_ecc_init_read_config_word(struct atmel_ecc_cmd *cmd,
u16 config_word)
 {
@@ -143,10 +120,7 @@ static void atmel_ecc_init_read_config_word(struct 
atmel_ecc_cmd *cmd,
cmd->opcode = OPCODE_READ;
cmd->param1 = CONFIG_ZONE;
cmd->param2 = config_word;
-   cmd->count = READ_COUNT;
-
-   atmel_ecc_checksum(cmd);
-
+   cmd->datasz = READ_DATASZ;
cmd->msecs = MAX_EXEC_TIME_READ;
cmd->rxsize = READ_RSP_SIZE;
 }
@@ -154,14 +128,11 @@ static void atmel_ecc_init_read_config_word(struct 
atmel_ecc_cmd *cmd,
 static void atmel_ecc_init_genkey_cmd(struct atmel_ecc_cmd *cmd, u16 keyid)
 {
cmd->word_addr = COMMAND;
-   cmd->count = GENKEY_COUNT;
+   cmd->datasz = GENKEY_DATASZ;
cmd->opcode = OPCODE_GENKEY;
cmd->param1 = GENKEY_MODE_PRIVATE;
/* a random private key will be generated and stored in slot keyID */
-   cmd->param2 = cpu_to_le16(keyid);
-
-   atmel_ecc_checksum(cmd);
-
+   cmd->param2 = keyid;
cmd->msecs = MAX_EXEC_TIME_GENKEY;
cmd->rxsize = GENKEY_RSP_SIZE;
 }
@@ -172,11 +143,11 @@ static int atmel_ecc_init_ecdh_cmd(struct atmel_ecc_cmd 
*cmd,
size_t copied;
 
cmd->word_addr = COMMAND;
-   cmd->count = ECDH_COUNT;
+   cmd->datasz = ECDH_DATASZ;
cmd->opcode = OPCODE_ECDH;
cmd->param1 = ECDH_PREFIX_MODE;
/* private key slot */
-   cmd->param2 = cpu_to_le16(DATA_SLOT_2);
+   cmd->param2 = DATA_SLOT_2;
 
/*
 * The device only supports NIST P256 ECC keys. The public key size will
@@ -186,9 +157,6 @@ static int atmel_ecc_init_ecdh_cmd(struct atmel_ecc_cmd 
*cmd,
copied = sg_copy_to_buffer(pubkey, 1, cmd->data, ATMEL_ECC_PUBKEY_SIZE);
if (copied != ATMEL_ECC_PUBKEY_SIZE)
return -EINVAL;
-
-   atmel_ecc_checksum(cmd);
-
cmd->msecs = MAX_EXEC_TIME_ECDH;
cmd->rxsize = ECDH_RSP_SIZE;
 
@@ -302,7 +270,11 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
  struct atmel_ecc_cmd *cmd)
 {
struct atmel_ecc_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+   u8 buf[MAX_CMD_SIZE];
+   u16 cmdcrc;
+   u8 cmdlen;
int ret;
+   int i;
 
mutex_lock(_priv->lock);
 
@@ -312,7 +284,31 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
goto err;
}
 
-   ret = i2c_master_send(client, (u8 *)cmd, cmd->count + WORD_ADDR_SIZE);
+   /* Marshal the command */
+   cmdlen = 6 + cmd->datasz + CRC_SIZE;
+   buf[0] = cmd->word_addr;
+   /* This excludes the word address, includes CRC */
+   buf[1] = cmdlen - 1;
+   buf[2] = cmd->opcode;
+   buf[3] = cmd->param1;
+   /* Enforce little-endian byte order */
+   buf[4] = cmd->param2 & 0xff;
+   buf[5] = (cmd->param2 >> 8);
+   /* Copy over the data array */
+   for (i = 0; i < cmd->datasz; i++)
+   buf[6+i] = 

[PATCH 7/9 v2] crypto: atmel-ecc: Print out serial number

[Linus Walleij]

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 
---
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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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(>dev,
+   "failed to read serial byte 0-3\n");
+   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(>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];
+
+
+   atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN);
+   ret = atmel_ecc_send_receive(client, cmd);
+   if (ret) {
+   kfree(cmd);
+   dev_err(>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(>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(_data.i2c_list_lock);
list_add_tail(_priv->i2c_client_list_node,
  _data.i2c_client_list);
-- 
2.17.0

[PATCH 3/9 v2] crypto: atmel-ecc: More helpful error messages

[Linus Walleij]

Report errors once when they happen on the I2C bus so we
get good information in cases such as when the wrong I2C
address is used.

Signed-off-by: Linus Walleij 
---
ChangeLog v1->v2:
- Strip some comments that are now obvious from the context
  with the error messages.
- Do not print the excess ECC error message,  atmel_ecc_status()
  already prints error messages.
---
 drivers/crypto/atmel-ecc.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 33773920e4bf..baef0d07164d 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -310,26 +310,31 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
mutex_lock(_priv->lock);
 
ret = atmel_ecc_wakeup(client);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "wakeup failed\n");
goto err;
+   }
 
-   /* send the command */
ret = i2c_master_send(client, (u8 *)cmd, cmd->count + WORD_ADDR_SIZE);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "command send failed\n");
goto err;
+   }
 
/* delay the appropriate amount of time for command to execute */
msleep(cmd->msecs);
 
-   /* receive the response */
ret = i2c_master_recv(client, cmd->data, cmd->rxsize);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "getting response failed\n");
goto err;
+   }
 
-   /* put the device into low-power mode */
ret = atmel_ecc_sleep(client);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "putting to sleep failed\n");
goto err;
+   }
 
mutex_unlock(_priv->lock);
return atmel_ecc_status(>dev, cmd->data);
@@ -624,8 +629,11 @@ static int device_sanity_check(struct i2c_client *client)
atmel_ecc_init_read_cmd(cmd);
 
ret = atmel_ecc_send_receive(client, cmd);
-   if (ret)
+   if (ret) {
+   dev_err(>dev,
+   "failed to send ECC init command\n");
goto free_cmd;
+   }
 
/*
 * It is vital that the Configuration, Data and OTP zones be locked
-- 
2.17.0

Re: [PATCH 4/9] crypto: atmel-ecc: Provide config zone defines

[Linus Walleij]

On Tue, Jun 12, 2018 at 2:59 PM Tudor Ambarus
 wrote:
> On 06/05/2018 04:49 PM, Linus Walleij wrote:
> > The config zone has 0x16 words of 4 bytes each, so provide
> > some basic defines so that we can address these individually.
>
> Are you going to use all these defines? I would add just the defines
> that are needed, when they are needed, but I guess it's a matter of
> preference.

I can cut it down to the ones I actually use in the subsequent
patches, but I'd like to keep it in a separate patch so as not
to disturb the order of the patch series. I can do like you say and
add it when adding users in future patches.

Yours,
Linus Walleij

Re: [PATCH 8/9] crypto: atmel-ecc: Detail what is unlocked

[Linus Walleij]

On Tue, Jun 12, 2018 at 3:25 PM Tudor Ambarus
 wrote:
> On 06/05/2018 04:49 PM, Linus Walleij wrote:

> > Instead of just providing a broad error message about the
> > chip being unlocked provide details on what is unlocked,
> > one line per thing that can be locked: data and OTP and
> > configuration are locked independently. Loose the
>
> Failure to lock these zones may permit modification of secret keys, so
> we don't permit using the device if any of these zones is unlocked.
> Why do you think it's relevant to indicate which zone is unlocked?

It is always nice for the user to know exactly what the problem
is. Instead of "there is some problem with locking" they now
know what locking the problem is about: the first, the second
or both.

Yours,
Linus Walleij

Re: [PATCH 7/9] crypto: atmel-ecc: Print out serial number

[Linus Walleij]

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 6/9] crypto: atmel-ecc: Marshal the command while sending

[Linus Walleij]

On Tue, Jun 12, 2018 at 12:19 PM Tudor Ambarus
 wrote:

> The struct atmel_ecc_cmd (__packed) is composed of u8 members with only
> 2 exceptions, u16 param2 and u16 crc that were written in little endian,
> as the device expects. The (u8 *) cast will point to the first member,
> which is u8 as well, so we're safe. I2C transfers are done at a per-byte
> level, so no problems here either.

This is not helpful for the developer, who has to start thinking
like the computer before they understand what is going on.
It makes the code hard to read IMHO.

The i2c transfers are done at a byte level, but the code goes
to call cpu_to_le16() to lay out the bytes in the struct in
the right way which is confusing if you don't immediately
know that the whole struct is going to be serialized.

> You are allocating a temporary buffer and duplicate the initialization,
> without an obvious benefit. Can you please explain what do want to fix
> or what are the potential problems?

The buffer is allocated on the stack, which is fine, this is
definately not on any hotpath as we're dealing with i2c traffic
in the end, but if you are worried about the buffer being
reallocated every time we enter the function I can certainly
mark it static.

It is fundamentally about readability and code centralization,
avoiding code duplication I would say.

It is pretty hard to see how endianness and
marshalling across to the chip happens unless the code dealing with
that is collected in one spot.

As it is now, that procedure is spread out, and it is hard to figure
out how endianness is dealt with.

First problem: we have several calls to
atmel_ecc_checksum(cmd); spread out all over the place instead
of doing the intuitive thing, which is to call that right before
we send the command. This is code duplication.

Second problem: we have two instances of inlined
cmd->param2 = cpu_to_le16(keyid); dealing with endianness
instead of doing this as part of marshalling the command.
This is also code duplication.

Yours,
Linus Walleij

Re: [PATCH 3/9] crypto: atmel-ecc: More helpful error messages

[Linus Walleij]

On Tue, Jun 12, 2018 at 2:36 PM Tudor Ambarus
 wrote:
> On 06/05/2018 04:49 PM, Linus Walleij wrote:

> >   /* send the command */
>
> I guess that this comment will become superfluous if you're going to add
> an error message.

OK stripped obvious comments.

> > - return atmel_ecc_status(>dev, cmd->data);
> > + ret = atmel_ecc_status(>dev, cmd->data);
>
> atmel_ecc_status already prints errors when needed.

OK skipped this change.


> >   ret = atmel_ecc_send_receive(client, cmd);
> > - if (ret)
> > + if (ret) {
> > + dev_err(>dev,
> > + "failed to send ECC init command\n");
>
> Here we will have two errors reported, the first being from the
> atmel_ecc_send_receive(). I would go with just an error reported. Do we
> really care what happened with the i2c transfer, or it's enough to
> report that the init failed?

The more help the better.

I think it is relevant to have both: you will read the log
and say "OK init failed because this I2C transfer is not
getting across as it should", that is helpful.

Yours,
Linus Walleij

Re: [PATCH 2/9] crypto: atmel-ecc: Silently ignore missing clock frequency

[Linus Walleij]

On Mon, Jun 11, 2018 at 11:46 AM Tudor Ambarus
 wrote:
> On 06/05/2018 04:49 PM, Linus Walleij wrote:
> > The Atmel ECC driver contains a check for the I2C bus clock
> > frequency, so as to check that the I2C adapter in use
> > satisfies the device specs.
> >
> > If the device is connected to a device tree node that does not
> > contain a clock frequency setting, such as an I2C mux or gate,
> > this blocks the probe. Make the probe continue silently if
> > no clock frequency can be found, assuming all is safe.
>
> I don't think it's safe. We use bus_clk_rate to compute the ecc508's
> wake token. If you can't wake the device, it will ignore all your
> commands.

I see. I wonder why it works so well for me then?
Could we just print a warning and continue?
The general advice for the kernel is not to bail out if
hardware is not optimally configured, but continue with
a warning that maybe not everything is all right.

> My proposal was to introduce a bus_freq_hz member in the i2c_adapter
> structure (https://patchwork.kernel.org/patch/10307637/), but I haven't
> received feedback yet, Wolfram?
>
> I would say first to check the bus_freq_hz from adapter (if it will be
> accepted) else to look into dt and, in the last case, if nowhere
> provided, to block the probe.

So blocking the probe because we are not 100% sure the hardware
will work is against established practice: the established practice
is to be liberal with letting probe() continue, but emit a warning.

In my case that is good because it makes the hardware probe
and work without any visible problems.

I *can* try to traverse the device tree upwards from the mux node
to find the parent I2C controller and its "clock-frequency"
property. I felt that was hackish, but if you prefer the hack I
can try to use that.

Yours,
Linus Walleij

[PATCH 7/9] crypto: atmel-ecc: Print out serial number

[Linus Walleij]

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

[PATCH 5/9] crypto: atmel-ecc: Create a config word reader

[Linus Walleij]

Rewrite the function atmel_ecc_init_read_cmd() into a more
general atmel_ecc_init_read_config_word() function to read
any word from the configuration zone, and use this
parameterized with what we want to read out.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index f1f422385a91..d89b69d228ac 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -136,16 +136,13 @@ static void atmel_ecc_checksum(struct atmel_ecc_cmd *cmd)
*crc16 = atmel_ecc_crc16(0, data, len);
 }
 
-static void atmel_ecc_init_read_cmd(struct atmel_ecc_cmd *cmd)
+static void atmel_ecc_init_read_config_word(struct atmel_ecc_cmd *cmd,
+   u16 config_word)
 {
cmd->word_addr = COMMAND;
cmd->opcode = OPCODE_READ;
-   /*
-* Read the word from Configuration zone that contains the lock bytes
-* (UserExtra, Selector, LockValue, LockConfig).
-*/
cmd->param1 = CONFIG_ZONE;
-   cmd->param2 = CONFIG_ZONE_FOOTER;
+   cmd->param2 = config_word;
cmd->count = READ_COUNT;
 
atmel_ecc_checksum(cmd);
@@ -633,7 +630,7 @@ static int device_sanity_check(struct i2c_client *client)
if (!cmd)
return -ENOMEM;
 
-   atmel_ecc_init_read_cmd(cmd);
+   atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_FOOTER);
 
ret = atmel_ecc_send_receive(client, cmd);
if (ret) {
-- 
2.17.0

[PATCH 9/9] crypto: atmel-ecc: Break out lock check helper

[Linus Walleij]

This breaks out a lock status checker to be used with further
refactorings.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 38 ++
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 2b6c8bb7bd1c..d15cc77ab739 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -668,7 +668,9 @@ static int atmel_ecc_get_serial(struct i2c_client *client)
return 0;
 }
 
-static int device_sanity_check(struct i2c_client *client)
+static int atmel_ecc_get_lock_status(struct i2c_client *client,
+bool *config_is_locked,
+bool *otp_data_is_locked)
 {
struct atmel_ecc_cmd *cmd;
int ret;
@@ -682,21 +684,49 @@ static int device_sanity_check(struct i2c_client *client)
ret = atmel_ecc_send_receive(client, cmd);
if (ret) {
dev_err(>dev,
-   "failed to send ECC init command\n");
+   "failed to send config read command\n");
goto free_cmd;
}
 
+   /* According to datasheet anything else than 0x55 means "locked" */
+   if (cmd->data[RSP_DATA_IDX+3] == 0x55)
+   *config_is_locked = false;
+   else
+   *config_is_locked = true;
+
+   if (cmd->data[RSP_DATA_IDX+2] == 0x55)
+   *otp_data_is_locked = false;
+   else
+   *otp_data_is_locked = true;
+
+free_cmd:
+   kfree(cmd);
+   return ret;
+}
+
+static int device_sanity_check(struct i2c_client *client)
+{
+   struct atmel_ecc_cmd *cmd;
+   bool config_locked;
+   bool otp_data_locked;
+   int ret;
+
/*
 * It is vital that the Configuration, Data and OTP zones be locked
 * prior to release into the field of the system containing the device.
 * Failure to lock these zones may permit modification of any secret
 * keys and may lead to other security problems.
 */
-   if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+   ret = atmel_ecc_get_lock_status(client, _locked,
+   _data_locked);
+   if (ret)
+   return ret;
+
+   if (!config_locked) {
dev_err(>dev, "configuration zone is unlocked\n");
ret = -ENOTSUPP;
}
-   if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+   if (!otp_data_locked) {
dev_err(>dev, "data and OTP zones are unlocked\n");
ret = -ENOTSUPP;
}
-- 
2.17.0

[PATCH 4/9] crypto: atmel-ecc: Provide config zone defines

[Linus Walleij]

The config zone has 0x16 words of 4 bytes each, so provide
some basic defines so that we can address these individually.
Rename the last word to "footer", this is where we currently
look to see if the configuration is locked.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c |  2 +-
 drivers/crypto/atmel-ecc.h | 25 +++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 214b0572bf8b..f1f422385a91 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -145,7 +145,7 @@ static void atmel_ecc_init_read_cmd(struct atmel_ecc_cmd 
*cmd)
 * (UserExtra, Selector, LockValue, LockConfig).
 */
cmd->param1 = CONFIG_ZONE;
-   cmd->param2 = DEVICE_LOCK_ADDR;
+   cmd->param2 = CONFIG_ZONE_FOOTER;
cmd->count = READ_COUNT;
 
atmel_ecc_checksum(cmd);
diff --git a/drivers/crypto/atmel-ecc.h b/drivers/crypto/atmel-ecc.h
index 25232c8abcc2..6d586a3e443d 100644
--- a/drivers/crypto/atmel-ecc.h
+++ b/drivers/crypto/atmel-ecc.h
@@ -89,8 +89,29 @@ static const struct {
 #define RSP_DATA_IDX   1 /* buffer index of data in response */
 #define DATA_SLOT_22 /* used for ECDH private key */
 
-/* Definitions for the device lock state */
-#define DEVICE_LOCK_ADDR   0x15
+/* Definitions for the configuration zone words, these are 4 bytes each */
+#define CONFIG_ZONE_SERIAL_0_3 0x00
+#define CONFIG_ZONE_REVISION   0x01
+#define CONFIG_ZONE_SERIAL_4_7 0x02
+#define CONFIG_ZONE_SERIAL_8_I2CEN 0x03
+#define CONFIG_ZONE_I2C_OTP0x04
+#define CONFIG_ZONE_SLOT_0_1   0x05
+#define CONFIG_ZONE_SLOT_2_3   0x06
+#define CONFIG_ZONE_SLOT_4_5   0x07
+#define CONFIG_ZONE_SLOT_6_7   0x08
+#define CONFIG_ZONE_SLOT_8_9   0x09
+#define CONFIG_ZONE_SLOT_10_11 0x0a
+#define CONFIG_ZONE_SLOT_12_13 0x0b
+#define CONFIG_ZONE_SLOT_14_15 0x0c
+#define CONFIG_ZONE_FLAG_0_1   0x0d
+#define CONFIG_ZONE_FLAG_2_3   0x0e
+#define CONFIG_ZONE_FLAG_4_5   0x0f
+#define CONFIG_ZONE_FLAG_6_7   0x10
+#define CONFIG_ZONE_LKU_0_10x11
+#define CONFIG_ZONE_LKU_2_30x12
+#define CONFIG_ZONE_LKU_4_50x13
+#define CONFIG_ZONE_LKU_6_70x14
+#define CONFIG_ZONE_FOOTER 0x15
 #define LOCK_VALUE_IDX (RSP_DATA_IDX + 2)
 #define LOCK_CONFIG_IDX(RSP_DATA_IDX + 3)
 
-- 
2.17.0

[PATCH 8/9] crypto: atmel-ecc: Detail what is unlocked

[Linus Walleij]

Instead of just providing a broad error message about the
chip being unlocked provide details on what is unlocked,
one line per thing that can be locked: data and OTP and
configuration are locked independently. Loose the
overzealous defines.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 8 ++--
 drivers/crypto/atmel-ecc.h | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index fd8149313104..2b6c8bb7bd1c 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -692,8 +692,12 @@ static int device_sanity_check(struct i2c_client *client)
 * Failure to lock these zones may permit modification of any secret
 * keys and may lead to other security problems.
 */
-   if (cmd->data[LOCK_CONFIG_IDX] || cmd->data[LOCK_VALUE_IDX]) {
-   dev_err(>dev, "Configuration or Data and OTP zones are 
unlocked!\n");
+   if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+   dev_err(>dev, "configuration zone is unlocked\n");
+   ret = -ENOTSUPP;
+   }
+   if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+   dev_err(>dev, "data and OTP zones are unlocked\n");
ret = -ENOTSUPP;
}
 
diff --git a/drivers/crypto/atmel-ecc.h b/drivers/crypto/atmel-ecc.h
index 2a378bccc213..988e46507619 100644
--- a/drivers/crypto/atmel-ecc.h
+++ b/drivers/crypto/atmel-ecc.h
@@ -113,8 +113,6 @@ static const struct {
 #define CONFIG_ZONE_LKU_4_50x13
 #define CONFIG_ZONE_LKU_6_70x14
 #define CONFIG_ZONE_FOOTER 0x15
-#define LOCK_VALUE_IDX (RSP_DATA_IDX + 2)
-#define LOCK_CONFIG_IDX(RSP_DATA_IDX + 3)
 
 /*
  * Wake High delay to data communication (microseconds). SDA should be stable
-- 
2.17.0

[PATCH 6/9] crypto: atmel-ecc: Marshal the command while sending

[Linus Walleij]

Instead of casting the struct for the command into (u8 *)
which is problematic in many ways, and instead of
calculating the CRC sum in a separate function, marshal,
checksum and send the command in one single function.

Instead of providing the length of the whole command
in defines, it makes more sense to provide the length
of the data buffer used with the command.

This avoids the hazzle to try to keep the command
structure in the device endianness, we fix up the
endianness when marshalling the command instead.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 71 ++
 drivers/crypto/atmel-ecc.h | 13 +++
 2 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index d89b69d228ac..603e29bdcb97 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -113,29 +113,6 @@ struct atmel_ecc_work_data {
struct atmel_ecc_cmd cmd;
 };
 
-static u16 atmel_ecc_crc16(u16 crc, const u8 *buffer, size_t len)
-{
-   return cpu_to_le16(bitrev16(crc16(crc, buffer, len)));
-}
-
-/**
- * atmel_ecc_checksum() - Generate 16-bit CRC as required by ATMEL ECC.
- * CRC16 verification of the count, opcode, param1, param2 and data bytes.
- * The checksum is saved in little-endian format in the least significant
- * two bytes of the command. CRC polynomial is 0x8005 and the initial register
- * value should be zero.
- *
- * @cmd : structure used for communicating with the device.
- */
-static void atmel_ecc_checksum(struct atmel_ecc_cmd *cmd)
-{
-   u8 *data = >count;
-   size_t len = cmd->count - CRC_SIZE;
-   u16 *crc16 = (u16 *)(data + len);
-
-   *crc16 = atmel_ecc_crc16(0, data, len);
-}
-
 static void atmel_ecc_init_read_config_word(struct atmel_ecc_cmd *cmd,
u16 config_word)
 {
@@ -143,10 +120,7 @@ static void atmel_ecc_init_read_config_word(struct 
atmel_ecc_cmd *cmd,
cmd->opcode = OPCODE_READ;
cmd->param1 = CONFIG_ZONE;
cmd->param2 = config_word;
-   cmd->count = READ_COUNT;
-
-   atmel_ecc_checksum(cmd);
-
+   cmd->datasz = READ_DATASZ;
cmd->msecs = MAX_EXEC_TIME_READ;
cmd->rxsize = READ_RSP_SIZE;
 }
@@ -154,14 +128,11 @@ static void atmel_ecc_init_read_config_word(struct 
atmel_ecc_cmd *cmd,
 static void atmel_ecc_init_genkey_cmd(struct atmel_ecc_cmd *cmd, u16 keyid)
 {
cmd->word_addr = COMMAND;
-   cmd->count = GENKEY_COUNT;
+   cmd->datasz = GENKEY_DATASZ;
cmd->opcode = OPCODE_GENKEY;
cmd->param1 = GENKEY_MODE_PRIVATE;
/* a random private key will be generated and stored in slot keyID */
-   cmd->param2 = cpu_to_le16(keyid);
-
-   atmel_ecc_checksum(cmd);
-
+   cmd->param2 = keyid;
cmd->msecs = MAX_EXEC_TIME_GENKEY;
cmd->rxsize = GENKEY_RSP_SIZE;
 }
@@ -172,11 +143,11 @@ static int atmel_ecc_init_ecdh_cmd(struct atmel_ecc_cmd 
*cmd,
size_t copied;
 
cmd->word_addr = COMMAND;
-   cmd->count = ECDH_COUNT;
+   cmd->datasz = ECDH_DATASZ;
cmd->opcode = OPCODE_ECDH;
cmd->param1 = ECDH_PREFIX_MODE;
/* private key slot */
-   cmd->param2 = cpu_to_le16(DATA_SLOT_2);
+   cmd->param2 = DATA_SLOT_2;
 
/*
 * The device only supports NIST P256 ECC keys. The public key size will
@@ -186,9 +157,6 @@ static int atmel_ecc_init_ecdh_cmd(struct atmel_ecc_cmd 
*cmd,
copied = sg_copy_to_buffer(pubkey, 1, cmd->data, ATMEL_ECC_PUBKEY_SIZE);
if (copied != ATMEL_ECC_PUBKEY_SIZE)
return -EINVAL;
-
-   atmel_ecc_checksum(cmd);
-
cmd->msecs = MAX_EXEC_TIME_ECDH;
cmd->rxsize = ECDH_RSP_SIZE;
 
@@ -302,7 +270,11 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
  struct atmel_ecc_cmd *cmd)
 {
struct atmel_ecc_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+   u8 buf[MAX_CMD_SIZE];
+   u16 cmdcrc;
+   u8 cmdlen;
int ret;
+   int i;
 
mutex_lock(_priv->lock);
 
@@ -312,8 +284,31 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
goto err;
}
 
+   /* Marshal the command */
+   cmdlen = 6 + cmd->datasz + CRC_SIZE;
+   buf[0] = cmd->word_addr;
+   /* This excludes the word address, includes CRC */
+   buf[1] = cmdlen - 1;
+   buf[2] = cmd->opcode;
+   buf[3] = cmd->param1;
+   /* Enforce little-endian byte order */
+   buf[4] = cmd->param2 & 0xff;
+   buf[5] = (cmd->param2 >> 8);
+   /* Copy over the data array */
+   for (i = 0; i < cmd->datasz; i++)
+   buf[6+i] = cmd->data[i];
+   /*
+* CRC sum the command, do not include word addr or CRC. The
+* bit order in the CRC16 

[PATCH 2/9] crypto: atmel-ecc: Silently ignore missing clock frequency

[Linus Walleij]

The Atmel ECC driver contains a check for the I2C bus clock
frequency, so as to check that the I2C adapter in use
satisfies the device specs.

If the device is connected to a device tree node that does not
contain a clock frequency setting, such as an I2C mux or gate,
this blocks the probe. Make the probe continue silently if
no clock frequency can be found, assuming all is safe.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index e66f18a0ddd0..145ab3a39a56 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -657,14 +657,13 @@ static int atmel_ecc_probe(struct i2c_client *client,
return -ENODEV;
}
 
+   /*
+* Silently assume all is fine if there is no
+* "clock-frequency" property.
+*/
ret = of_property_read_u32(client->adapter->dev.of_node,
   "clock-frequency", _clk_rate);
-   if (ret) {
-   dev_err(dev, "of: failed to read clock-frequency property\n");
-   return ret;
-   }
-
-   if (bus_clk_rate > 100L) {
+   if (!ret && (bus_clk_rate > 100L)) {
dev_err(dev, "%d exceeds maximum supported clock frequency 
(1MHz)\n",
bus_clk_rate);
return -EINVAL;
-- 
2.17.0

[PATCH 3/9] crypto: atmel-ecc: More helpful error messages

[Linus Walleij]

Report errors once when they happen on the I2C bus so we
get good information in cases such as when the wrong I2C
address is used.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/atmel-ecc.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 145ab3a39a56..214b0572bf8b 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -310,29 +310,41 @@ static int atmel_ecc_send_receive(struct i2c_client 
*client,
mutex_lock(_priv->lock);
 
ret = atmel_ecc_wakeup(client);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "wakeup failed\n");
goto err;
+   }
 
/* send the command */
ret = i2c_master_send(client, (u8 *)cmd, cmd->count + WORD_ADDR_SIZE);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "command send failed\n");
goto err;
+   }
 
/* delay the appropriate amount of time for command to execute */
msleep(cmd->msecs);
 
/* receive the response */
ret = i2c_master_recv(client, cmd->data, cmd->rxsize);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "getting response failed\n");
goto err;
+   }
 
/* put the device into low-power mode */
ret = atmel_ecc_sleep(client);
-   if (ret < 0)
+   if (ret < 0) {
+   dev_err(>dev, "putting to sleep failed\n");
goto err;
+   }
 
mutex_unlock(_priv->lock);
-   return atmel_ecc_status(>dev, cmd->data);
+   ret = atmel_ecc_status(>dev, cmd->data);
+   if (ret < 0)
+   dev_err(>dev, "ECC status parse failed\n");
+
+   return ret;
 err:
mutex_unlock(_priv->lock);
return ret;
@@ -624,8 +636,11 @@ static int device_sanity_check(struct i2c_client *client)
atmel_ecc_init_read_cmd(cmd);
 
ret = atmel_ecc_send_receive(client, cmd);
-   if (ret)
+   if (ret) {
+   dev_err(>dev,
+   "failed to send ECC init command\n");
goto free_cmd;
+   }
 
/*
 * It is vital that the Configuration, Data and OTP zones be locked
-- 
2.17.0

[PATCH 1/9] crypto: atmel-ecc: Make available for other platforms

[Linus Walleij]

This is a pure I2C driver, and this device appears on the
96boards Secure96 mezzanine card, so we want to enable the
driver on other devices. Cut the Kconfig limitations to
Atmel SoC only.

Signed-off-by: Linus Walleij 
---
 drivers/crypto/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index d1ea1a07cecb..9e33f2de8dae 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -522,7 +522,6 @@ config CRYPTO_DEV_ATMEL_SHA
 
 config CRYPTO_DEV_ATMEL_ECC
tristate "Support for Microchip / Atmel ECC hw accelerator"
-   depends on ARCH_AT91 || COMPILE_TEST
depends on I2C
select CRYPTO_ECDH
select CRC16
-- 
2.17.0

Re: [PATCH] crypto: ux500/hash: Add empty export and import

[Linus Walleij]

On Tue, Jan 16, 2018 at 5:32 PM, Kamil Konieczny
<k.koniec...@partner.samsung.com> wrote:

> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>

Acked-by: Linus Walleij <linus.wall...@linaro.org>

But why isn't the framework code just checking the vtable for NULL?

if (foo->fp)
foo->fp(bar);

Yours,
Linus Walleij

[PATCH] crypto: ux500: memmove the right size

[Linus Walleij]

The hash buffer is really HASH_BLOCK_SIZE bytes, someone
must have thought that memmove takes n*u32 words by mistake.
Tests work as good/bad as before after this patch.

Cc: Joakim Bech <joakim.b...@linaro.org>
Reported-by: David Binderman <linuxdev.baldr...@gmail.com>
Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
---
 drivers/crypto/ux500/hash/hash_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ux500/hash/hash_core.c 
b/drivers/crypto/ux500/hash/hash_core.c
index 574e87c7f2b8..9acccad26928 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -781,7 +781,7 @@ static int hash_process_data(struct hash_device_data 
*device_data,
_data->state);
memmove(req_ctx->state.buffer,
device_data->state.buffer,
-   HASH_BLOCK_SIZE / sizeof(u32));
+   HASH_BLOCK_SIZE);
if (ret) {
dev_err(device_data->dev,
"%s: hash_resume_state() 
failed!\n",
@@ -832,7 +832,7 @@ static int hash_process_data(struct hash_device_data 
*device_data,
 
memmove(device_data->state.buffer,
req_ctx->state.buffer,
-   HASH_BLOCK_SIZE / sizeof(u32));
+   HASH_BLOCK_SIZE);
if (ret) {
dev_err(device_data->dev, "%s: 
hash_save_state() failed!\n",
__func__);
-- 
2.4.11

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-4.6/drivers/crypto/ux500/hash/hash_core.c: 2 * possible bad size ?

[Linus Walleij]

On Wed, May 18, 2016 at 9:46 AM, Herbert Xu <herb...@gondor.apana.org.au> wrote:
> On Mon, May 16, 2016 at 07:13:12PM +0100, David Binderman wrote:
>> Hello there,
>>
>> 1.
>>
>> linux-4.6/drivers/crypto/ux500/hash/hash_core.c:784]: (warning) Division by
>> result of sizeof(). memmove() expects a size in bytes, did you intend to
>> multiply instead?
>>
>> Source code is
>>
>> memmove(req_ctx->state.buffer,
>> device_data->state.buffer,
>> HASH_BLOCK_SIZE / sizeof(u32));
>>
>> Maybe better code
>>
>> memmove(req_ctx->state.buffer,
>> device_data->state.buffer,
>> HASH_BLOCK_SIZE);

Yeah obviously the latter as in hash_alg.h:

struct hash_state {
(...)
u32buffer[HASH_BLOCK_SIZE / sizeof(u32)];

That could just as well be an u8 of HASH_BLOCK_SIZE.

Sending a patch for this.

>> linux-4.6/drivers/crypto/ux500/hash/hash_core.c:835]: (warning) Division by
>> result of sizeof(). memmove() expects a size in bytes, did you intend to
>> multiply instead?
>>
>> Duplicate.
>
> Thanks for noticing these bugs.  This driver hasn't been maintained
> since 2012, so unless someone steps up I'm going to just delete it.

I'm trying to take a look at it because I'm using this platform
for tests and it's nice to have all features enabled.

And it has some problems (I added prints to also print successful tests):

[3.864746] alg: hash: Test 1 SUCCEEDED for sha1-ux500
[3.870147] alg: hash: Test 2 SUCCEEDED for sha1-ux500
[3.875610] alg: hash: Test 3 SUCCEEDED for sha1-ux500
[3.881408] alg: hash: Test 4 SUCCEEDED for sha1-ux500
[3.886596] alg: hash: Chunking test 1 SUCCEEDED for sha1-ux500
[3.892639] alg: hash: Chunking test 2 FAILED for sha1-ux500
[3.898284] result:
[3.900421] : 76 b4 ed 2f d7 11 1d c8 64 4c 38 b0 f8 27 19 89
[3.906860] 0010: 58 1e bb 3a
[3.915588] expected:
[3.917846] : 97 01 11 c4 e7 7b cc 88 cc 20 45 9c 02 b6 9b 4a
[3.928314] 0010: a8 f5 82 17
[3.937255] alg: hash: Test 1 SUCCEEDED for sha256-ux500
[3.948089] alg: hash: Test 2 SUCCEEDED for sha256-ux500
[3.961944] alg: hash: Test 3 SUCCEEDED for sha256-ux500
[3.967590] alg: hash: Test 4 SUCCEEDED for sha256-ux500
[3.973083] alg: hash: Chunking test 1 SUCCEEDED for sha256-ux500
[3.979248] alt: hash: Failed to export() for sha256-ux500
[3.984802] hash: partial update failed on test 1 for sha256-ux500: ret=38
[3.992004] alg: hash: Test 1 SUCCEEDED for hmac-sha1-ux500
[3.997650] alg: hash: Test 2 SUCCEEDED for hmac-sha1-ux500
[4.003356] alg: hash: Test 3 SUCCEEDED for hmac-sha1-ux500
[4.009002] alg: hash: Test 4 SUCCEEDED for hmac-sha1-ux500
[4.014678] alg: hash: Test 5 SUCCEEDED for hmac-sha1-ux500
[4.020385] alg: hash: Test 6 SUCCEEDED for hmac-sha1-ux500
[4.026062] alg: hash: Chunking test 1 SUCCEEDED for hmac-sha1-ux500
[4.032470] alt: hash: Failed to export() for hmac-sha1-ux500
[4.038208] hash: partial update failed on test 1 for hmac-sha1-ux500: ret=38
[4.045623] alg: hash: Test 1 SUCCEEDED for hmac-sha256-ux500
[4.051483] alg: hash: Test 2 SUCCEEDED for hmac-sha256-ux500
[4.057342] alg: hash: Test 3 SUCCEEDED for hmac-sha256-ux500
[4.063201] alg: hash: Test 4 SUCCEEDED for hmac-sha256-ux500
[4.069030] alg: hash: Test 5 SUCCEEDED for hmac-sha256-ux500
[4.074890] alg: hash: Test 6 SUCCEEDED for hmac-sha256-ux500
[4.080780] alg: hash: Test 7 SUCCEEDED for hmac-sha256-ux500
[4.086608] alg: hash: Test 8 SUCCEEDED for hmac-sha256-ux500
[4.092468] alg: hash: Test 9 SUCCEEDED for hmac-sha256-ux500
[4.098297] alg: hash: Chunking test 1 SUCCEEDED for hmac-sha256-ux500
[4.104888] alt: hash: Failed to export() for hmac-sha256-ux500
[4.110809] hash: partial update failed on test 1 for
hmac-sha256-ux500: ret=38
[4.118164] hash1 hash1: successfully registered
[4.123687] alg: No test for aes (aes-ux500)
[4.132354] alg: No test for des (des-ux500)
[4.136749] alg: No test for des3_ede (des3_ede-ux500)
[4.151306] alg: skcipher: Test 1 failed (invalid result) on
encryption for cbc-des-ux500
[4.159484] : 03 91 6b cc 4a f6 3a 53 9c 4d 2e 2b 91 83 44 f6
[4.165954] 0010: aa 6a 15 6a dc b5 e0 3d
[4.170501] cryp1 cryp1: successfully registered

The simple tests always work, it's those stressful ones that create
problems.

Joakim: did you have a memory of this code working? Should
I check the vendor tree for fixes?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] hwrng: stm32 - add support for STM32 HW RNG

[Linus Walleij]

On Mon, Oct 12, 2015 at 10:21 AM, Daniel Thompson
<daniel.thomp...@linaro.org> wrote:

> Add support for STMicroelectronics STM32 random number generator.
>
> The config value defaults to N, reflecting the fact that STM32 is a
> very low resource microcontroller platform and unlikely to be targeted
> by any "grown up" defconfigs.
>
> Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org>

This is a fine driver, love the performance boost you reported
and stand corrected on the likeness of the other Nomadik driver.
Reviewed-by: Linus Walleij <linus.wall...@linaro.org>

> +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool 
> wait)

Now this read() function is so nice that I feel obliged to
go in and tidy up the Nomadik driver to be as nice.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] hwrng: stm32 - add support for STM32 HW RNG

[Linus Walleij]

On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson
<daniel.thomp...@linaro.org> wrote:

> Add support for STMicroelectronics STM32 random number generator.
>
> The config value defaults to N, reflecting the fact that STM32 is a
> very low resource microcontroller platform and unlikely to be targeted
> by any "grown up" defconfigs.
>
> Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org>

Incidentally both you and Lee Jones submitted similar RNG drivers from
the same company (ST) recent weeks:

Lee's ST thing:
http://marc.info/?l=linux-crypto-vger=144249760524971=2

Daniel's ST thing:
http://marc.info/?l=linux-crypto-vger=144390463810134=2

And we also have from old ST:
drivers/char/hw_random/nomadik-rng.c

1. Are these similar IPs that could be unified in a driver, just different
offsets in memory?

2. Could you at least cross-review each others drivers because both
tight loops to read bytes seem to contain similar semantics.

3. I took out the datasheet for Nomadik STn8820 and it seems that
the hardware is very similar to what this driver is trying to drive.
CR, SR and DR are in the same place, can you check if you also even
have the PrimeCell magic in the words at offset 0xfe0 thru 0xffc?

In any case it seems likely that this driver should supercede and replace
drivers/char/hw_random/nomadik-rng.c
still using the PrimeCell bus, and if it doesn't have the PrimeCell
IDs in hardware, this can be specified in the device tree using
arm,primecell-periphid = <0x000805e1>; in the node if need be.
The in-tree driver is dangerously simple and assume too much IMO.

Now the rest:

> +   /* enable random number generation */
> +   clk_enable(priv->clk);
> +   cr = readl(priv->base + RNG_CR);
> +   writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);

The Nomadik datasheet says this works the inverse way: setting
bit 2 to 1 *disables* the RNG. Can you double-check this?

The Nomadik version also has a TSTCLK bit 3, that should
always be set and loop-back checked in SR bit 1 to see that the block
is properly clocked. (Read data will hang on an AHB stall if it's
unclocked) Is this missing from STM32?

> + sr = readl(priv->base + RNG_SR);

I strongly recommend that you use readl_relaxed() for this and
all other reads on the hotpath, the Nomadik uses __raw_readl()
but that is just for the same reasons that readl_relaxed() should
be used: we just wanna read, not clean buffers etc.

> + /* Has h/ware error dection been triggered? */
> + if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS)))
> + break;
> +
> + /* No data ready... */
> + if (!sr)
> + break;

This assumes that only bits 6,5 and 0 are ever used in this hardware.
Are you sure? On Nomadik DRDY bit 0 is the same, bit 1
is the clock test bit mentioned above, bit 2 is FAULT set if an invalid
bit sequence is detected and should definately be checked if
the HW supports it. Please mask explicitly for DRDY at least.

The bit layout gives at hand that this is indeed a derived IP block,
I wonder what bits 3 & 4 may be used for in some or all
revisions?

Then this construct:

> +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool 
> wait)
> +{
(...)
> + /* enable random number generation */
> + clk_enable(priv->clk);
> + cr = readl(priv->base + RNG_CR);
> + writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
(...)
> + /* disable the generator */
> + writel(cr, priv->base + RNG_CR);
> + clk_disable(priv->clk);

This is in the hotpath where megabytes of data may be read out
by consecutive reads. I think it's wise to move the clock enable/disable
and also the write to cr to enable/disable the block to runtime PM
hooks, so that you can e.g. set some hysteresis around the disablement
with pm_runtime_set_autosuspend_delay(>dev, 50);
pm_runtime_use_autosuspend(>dev);pm_runtime_put_autosuspend().
For a primecell check the usage in drivers/mmc/host/mmci.c
for a complex usecase or e.g. drivers/hwtracing/coresight/coresight-tpiu.c
for a simpler usecase.

For the performance hints I guess you can even test whether
what I'm saying makes sense or not by reading some megabytes
of random and timing it.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: tegra: use kernel entropy instead of ad-hoc

[Linus Walleij]

The way I read the Tegra AES RNG is that it has a homebrew
algorithm for initializing the 128bit RNG using timespec and
the unique chip ID. This looks like reinventing the (square)
wheel, instead just grab 128bits from the kernel entropy pool
where the time and (after another patch) chip unique ID is
already mixed in.

Incidentally this also gets rid of a rather ugly
cross-dependence on the machine using an extern declaration.

Cc: Stephen Warren swar...@wwwdotorg.org
Cc: Varun Wadekar vwade...@nvidia.com
Cc: Neil Horman nhor...@tuxdriver.com
Cc: linux-te...@vger.kernel.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
Only compile-tested as I don't have this platform.
---
 drivers/crypto/tegra-aes.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/tegra-aes.c b/drivers/crypto/tegra-aes.c
index 2d58da9..7f42bfe 100644
--- a/drivers/crypto/tegra-aes.c
+++ b/drivers/crypto/tegra-aes.c
@@ -199,8 +199,6 @@ static void aes_workqueue_handler(struct work_struct *work);
 static DECLARE_WORK(aes_work, aes_workqueue_handler);
 static struct workqueue_struct *aes_wq;
 
-extern unsigned long long tegra_chip_uid(void);
-
 static inline u32 aes_readl(struct tegra_aes_dev *dd, u32 offset)
 {
return readl(dd-io_base + offset);
@@ -713,9 +711,8 @@ static int tegra_aes_rng_reset(struct crypto_rng *tfm, u8 
*seed,
struct tegra_aes_dev *dd = aes_dev;
struct tegra_aes_ctx *ctx = rng_ctx;
struct tegra_aes_slot *key_slot;
-   struct timespec ts;
int ret = 0;
-   u64 nsec, tmp[2];
+   u8 tmp[16]; /* 16 bytes = 128 bits of entropy */
u8 *dt;
 
if (!ctx || !dd) {
@@ -778,14 +775,8 @@ static int tegra_aes_rng_reset(struct crypto_rng *tfm, u8 
*seed,
if (dd-ivlen = (2 * DEFAULT_RNG_BLK_SZ + AES_KEYSIZE_128)) {
dt = dd-iv + DEFAULT_RNG_BLK_SZ + AES_KEYSIZE_128;
} else {
-   getnstimeofday(ts);
-   nsec = timespec_to_ns(ts);
-   do_div(nsec, 1000);
-   nsec ^= dd-ctr  56;
-   dd-ctr++;
-   tmp[0] = nsec;
-   tmp[1] = tegra_chip_uid();
-   dt = (u8 *)tmp;
+   get_random_bytes(tmp, sizeof(tmp));
+   dt = tmp;
}
memcpy(dd-dt, dt, DEFAULT_RNG_BLK_SZ);
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] Various ux500 crypto updates

[Linus Walleij]

On Tue, Jun 25, 2013 at 10:54 AM, Fabio Baltieri
fabio.balti...@linaro.org wrote:

 these are some fixes to issue pointed out by sparse and dmaengine API
 udpates on the recently enabled ux500 hw crypto drivers.

The series:
Acked-by: Linus Walleij linus.wall...@linaro.org

 These are based on recent linux-next, and can probably be applied on an
 architecture specific branch with your ack, or just skip to the next
 merge window.

I think these depend on the DMA40 fixes that reside in the ARM SoC
tree. If I get Herbert's ACK I can queue them.

They will have to go through that tree or wait for the next merge
window, I think they are not regressions so no hurry.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] Various ux500 crypto updates

[Linus Walleij]

On Tue, Jun 25, 2013 at 1:17 PM, Herbert Xu herb...@gondor.apana.org.au wrote:
 On Tue, Jun 25, 2013 at 11:20:50AM +0200, Linus Walleij wrote:
 On Tue, Jun 25, 2013 at 10:54 AM, Fabio Baltieri
 fabio.balti...@linaro.org wrote:

  these are some fixes to issue pointed out by sparse and dmaengine API
  udpates on the recently enabled ux500 hw crypto drivers.

 The series:
 Acked-by: Linus Walleij linus.wall...@linaro.org

  These are based on recent linux-next, and can probably be applied on an
  architecture specific branch with your ack, or just skip to the next
  merge window.

 I think these depend on the DMA40 fixes that reside in the ARM SoC
 tree. If I get Herbert's ACK I can queue them.

 Sure,

 Acked-by: Herbert Xu herb...@gondor.apana.org.au

OK I'll attempt to queue this up and send it to ARM SoC.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/39] ARM: ux500: Stop passing MMC's platform data for Device Tree boots

[Linus Walleij]

On Mon, Jun 10, 2013 at 11:15 AM, Lee Jones lee.jo...@linaro.org wrote:
 On Wed, 15 May 2013, Linus Walleij wrote:

 On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

  It was required to pass DMA channel configuration information to the
  MMC driver before the new DMA API was in place. Now that it is, and
  is fully compatible with Device Tree we can stop doing that.
 
  Reviewed-by: Linus Walleij linus.wall...@linaro.org
  Signed-off-by: Lee Jones lee.jo...@linaro.org

 So since the use of dma_request_slave_channel() is not upstream,
 I guess this will break DMA use (i.e slow down transfers!) on all
 device tree boots?

 I'd be happy to apply it once the MMCI patch is in linux-next
 indicating there may just be a window in the merge period
 where it falls back to IRQ mode, but I don't want to disable
 DMA on DT boots for an entire kernel cycle just like that.

 Not applied as of yet.

 I believe it's now okay to apply this.

Yep, I've rebased and applied it to the ux500-devicetree
branch.

I have some stuff on this branch which is queued up but may
miss v3.11, because I need the 5 outstanding pull requests
to land in ARM SoC so I get a merge base there before I
can send any more stuff.

It's mainly because this stuff isn't any orthogonal, everything
just conflicts in the AUXDATA all the time.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/39] ARM: ux500: Move SDI (MMC) and UART devices under more descriptive heading

[Linus Walleij]

On Mon, Jun 10, 2013 at 11:17 AM, Lee Jones lee.jo...@linaro.org wrote:
 On Wed, 15 May 2013, Linus Walleij wrote:

 On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

  Now DMA DT bindings exist and are in use by he MMC and UART drivers, it
  should be possible to remove them from the auxdata structure. However,
  after doing so the drivers fail. Common clk is still reliant on the
  dev_name() call to do device name matching, which will fail due to the
  fact that Device Tree naming differs somewhat do the more traditional
  conventions.
 
  Reviewed-by: Linus Walleij linus.wall...@linaro.org
  Signed-off-by: Lee Jones lee.jo...@linaro.org

 Cannot be applied due to dependency on 5/39.

 I think this can be applied now (if it hasn't already).

I really need a clean mergebase for this to merge ...
This patch requires both the dma40 and devicetree
branches to land in a common place before it can be
applied.

I want the DMA40 branch to be closed down now as I
have sent all pull requests on it, so pls ping me again on
this when we have something in the ARM SoC tree we
can work on.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 For all ux500 based platforms the maximum number of end-points are used.
 Move this knowledge into the driver so we can relinquish the burden from
 platform data. This also removes quite a bit of complexity from the driver
 and will aid us when we come to enable the driver for Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my dma40 branch with Felipe's ACK.

Thanks!
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Wed, May 29, 2013 at 7:57 PM, Felipe Balbi ba...@ti.com wrote:
 On Wed, May 15, 2013 at 10:51:43AM +0100, Lee Jones wrote:
 For all ux500 based platforms the maximum number of end-points are used.
 Move this knowledge into the driver so we can relinquish the burden from
 platform data. This also removes quite a bit of complexity from the driver
 and will aid us when we come to enable the driver for Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

 for drivers/usb/musb

 Acked-by: Felipe Balbi ba...@ti.com

Is that only for this patch 20/39 or also 21, 22  23?

Poke us if we should re-send them...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 21/39] usb: musb: ux500: move the MUSB HDRC configuration into the driver

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The MUSB HDRC configuration never changes between each of the ux500
 supported platforms, so there's little point passing it though platform
 data. If we set it in the driver instead, we can make good use of it
 when booting with either ATAGs or Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied with Felipe's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/39] usb: musb: ux500: take the dma_mask from coherent_dma_mask

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The dma_mask will always be the same as the coherent_dma_mask, so let's
 cut down on the platform_data burden and set it as such in the driver.
 This also saves us from supporting it separately when we come to enable
 this driver for Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied with Felipe's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 23/39] usb: musb: ux500: harden checks for platform data

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 In its current state, the ux500-musb driver uses platform data pointers
 blindly with no prior checking. If no platform data pointer is passed
 this will Oops the kernel. In this patch we ensure platform data and
 board data are present prior to using them.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied with Felipe's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/39] usb: musb: ux500: attempt to find channels by name before using pdata

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 If we can ever get to a state where we can solely search for DMA channels
 by name, this will almost completely alleviate the requirement to pass
 copious amounts of information though platform data. Here we take the
 first step towards this. The next step will be to enable Device Tree
 complete with name-event_line mapping.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied with Felipe's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 25/39] usb: musb: ux500: add device tree probing support

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 This patch will allow ux500-musb to be probed and configured solely from
 configuration found in Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: linux-...@vger.kernel.org
 Cc: devicetree-disc...@lists.ozlabs.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied with Felipe's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 26/39] ARM: ux500: Add an auxdata entry for MUSB for clock-name look-up

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The recently DT:ed MUSB driver will require clock-name by device-name
 look-up capability, until common clk has is properly supported by the
 ux500 platform.

 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Applied to my ux500-devicetree branch.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 27/39] ARM: ux500: Remove ux500-musb platform registation when booting with DT

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Now the ux500-musb driver has been enabled for Device Tree, there is no
 requirement to register it from platform code.

 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my ux500-dma40 branch on top of the musb
stuff.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 28/39] ARM: ux500: Remove empty function u8500_of_init_devices()

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 As promised, now all devices which resided in u8500_of_init_devices()
 have been enabled for Device Tree, we can completely remove it.

 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Applied on my ux500-dma40 branch.

And this is looking real good now!

Thanks!
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 29/39] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The aim is to make the code that little more readable.

 Acked-by: Vinod Koul vnod.k...@intel.com
 Acked-by: Arnd Bergmann a...@arndb.de
 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my ux500-dma40 branch.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 30/39] ARM: ux500: Replace ST-E's home-brew DMA direction definition with the generic one

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 STEDMA40_*_TO_* direction definitions are identical in all but name to
 the pre-defined generic DMA_*_TO_* ones. Let's make things easy by not
 duplicating such things.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Vinod, I'm lacking your ACK on this patch, but have tentatively
queued it anyway. Maybe you missed it?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 31/39] dmaengine: ste_dma40: Replace ST-E's home-brew DMA direction defs with generic ones

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 STEDMA40_*_TO_* direction definitions are identical in all but name to
 the pre-defined generic DMA_*_TO_* ones. Let's make things easy by not
 duplicating such things.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my dma40 branch with Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 32/39] ARM: ux500: Remove recently unused stedma40_xfer_dir enums

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 We're now using the transfer direction definitions provided by the DMA
 sub-system, so the home-brew ones have become obsolete.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied, also missing Vinod's ACK on this, but it seems
it was implicitly ACKed in the discussion on patch 31.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 33/39] dmaengine: ste_dma40_ll: Use the BIT macro to replace ugly '(1 x)'s

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The aim is to make the code that little more readable.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 34/39] dmaengine: ste_dma40: Convert data_width from register bit format to value

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 When a DMA client requests and configures a DMA channel, it requests
 data_width in Bytes. The DMA40 driver then swiftly converts it over to
 the necessary register bit value. Unfortunately, for any subsequent
 calculations we have to shift '1' by the bit pattern (1  data_width)
 times to make any sense of it.

 This patch flips the semantics on its head and only converts the value
 to its respective register bit pattern when writing to registers. This
 way we can use the true data_width (in Bytes) value.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with what I interpret as Vinod's ACK
(okay... statement on last reply.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 35/39] dmaengine: ste_dma40_ll: Replace meaningless register set with comment

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Unsure of the author's intentions, rather than just removing the nop,
 we're replacing it with a comment containing the possible intention
 of the statement OR:ing with 0.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied. Missing Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 36/39] dmaengine: ste_dma40: Allow memcpy channels to be configured from DT

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 At this moment in time the memcpy channels which can be used by the D40
 are fixed, as each supported platform in Mainline uses the same ones.
 However, platforms do exist which don't follow this convention, so
 these will need to be tailored. Fortunately, these platforms will be DT
 only, so this change has very little impact on platform data.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 37/39] ARM: ux500: Stop passing DMA platform data though AUXDATA

[Linus Walleij]

On Wed, May 15, 2013 at 11:52 AM, Lee Jones lee.jo...@linaro.org wrote:

 The DMA platform data is now empty due to some recent refactoring,
 so there is no longer a requirement to pass it though.

 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Applied to dma40 branch, hm now I may get a conflict with all
the AUXDATA changes in the devicetree branch. Oh well, I'll
figure it out...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 38/39] dmaengine: ste_dma40: Fetch the number of physical channels from DT

[Linus Walleij]

On Wed, May 15, 2013 at 11:52 AM, Lee Jones lee.jo...@linaro.org wrote:

 Some platforms insist on obscure physical channel availability. This
 information is currently passed though platform data in internal BSP
 kernels. Once those platforms land, they'll need to configure them
 appropriately, so we may as well add the infrastructure.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 39/39] dmaengine: ste_dma40: Fetch disabled channels from DT

[Linus Walleij]

On Wed, May 15, 2013 at 11:52 AM, Lee Jones lee.jo...@linaro.org wrote:

 Some platforms have channels which are not available for normal use.
 This information is currently passed though platform data in internal
 BSP kernels. Once those platforms land, they'll need to configure them
 appropriately, so we may as well add the infrastructure.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Tue, May 28, 2013 at 6:27 PM, Felipe Balbi ba...@ti.com wrote:
 On Wed, May 15, 2013 at 07:18:01PM +0200, Linus Walleij wrote:

 I can't merge any of this without Felipes ACKs in any
 case.

 Do you want to take this yourself ? I haven't fully read the series yet,
 but seems like this depends on the rest of the series. If you want to
 take it, let me know I can ack the patches as soon as I'm done
 reviewing.

Yes please. They are dependent on a stash of patches that that
has recently landed in the ARM SoC tree, so currently they need
to follow those through the same tree, which is where I funnel
all the ux500 stuff.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] crypto: ux500/cryp - Enable DT probing of the driver

[Linus Walleij]

On Thu, May 16, 2013 at 1:27 PM, Lee Jones lee.jo...@linaro.org wrote:

 By providing an OF match table with a suitable compatible string, we
 can ensure the ux500-crypt driver is probed by supplying an associated
 DT node in a given platform's Device Tree.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: linux-crypto@vger.kernel.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Herbert, can I have your ACK on patch 5  6 in this series to take it
through the ARM SoC tree?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] crypto: ux500/cryp - Enable DT probing of the driver

[Linus Walleij]

On Thu, May 16, 2013 at 1:27 PM, Lee Jones lee.jo...@linaro.org wrote:

 By providing an OF match table with a suitable compatible string, we
 can ensure the ux500-crypt driver is probed by supplying an associated
 DT node in a given platform's Device Tree.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: linux-crypto@vger.kernel.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/crypto/ux500/cryp/cryp_core.c |6 ++

W00t! No binding document?

OK I guess it's just reg, irq ... but still it's compulsory,
right?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] crypto: ux500/cryp - Enable DT probing of the driver

[Linus Walleij]

On Fri, May 24, 2013 at 10:32 AM, Lee Jones lee.jo...@linaro.org wrote:
 On Fri, 24 May 2013, Linus Walleij wrote:
 On Thu, May 16, 2013 at 1:27 PM, Lee Jones lee.jo...@linaro.org wrote:

  By providing an OF match table with a suitable compatible string, we
  can ensure the ux500-crypt driver is probed by supplying an associated
  DT node in a given platform's Device Tree.
 
  Cc: Herbert Xu herb...@gondor.apana.org.au
  Cc: linux-crypto@vger.kernel.org
  Signed-off-by: Lee Jones lee.jo...@linaro.org

 Herbert, can I have your ACK on patch 5  6 in this series to take it
 through the ARM SoC tree?

 Right, there are no bindings, so they don't need a document.

Surely the requirement that the node must have reg,
irqs and regulator for the driver to even probe is a
binding?

The fact that the drivers/of/* core on Linux will auto-populate
platform devices with those properties is a pure Linux
pecularity, and the bindings are there for OS independence.

Whether we need to document it for such standard things
is another thing, hopefully Rob or someone can answer that?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/39] dmaengine: ste_dma40: Remove unnecessary call to d40_phy_cfg()

[Linus Walleij]

On Thu, May 16, 2013 at 12:59 PM, Lee Jones lee.jo...@linaro.org wrote:
 On Thu, 16 May 2013, Vinod Koul wrote:

 On Thu, May 16, 2013 at 08:25:57AM +0100, Lee Jones wrote:
  On Thu, 16 May 2013, Vinod Koul wrote:
 
   On Wed, May 15, 2013 at 10:51:25AM +0100, Lee Jones wrote:
All configuration left in d40_phy_cfg() is runtime configurable and
there is already a call into it from d40_runtime_config(), so let's
rely on that.
   
Acked-by: Vinod Koul vnod.k...@intel.com
   That needs up update!
 
  Ah, where did I get that from that?
 
  Was that my mistake, or was this in the MAINTAINERS file?
 Certainly not in MAINTAINERS file :)

 My bad then, sorry.

 Linus,

 Would you be kind enough to fix it please, as it's in your tree now.

I've fixed it up!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/39] crypto: ux500/hash - Prepare clock before enabling it

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 If we fail to prepare the ux500-hash clock before enabling it the
 platform will fail to boot. Here we insure this happens.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Ulf Hansson ulf.hans...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my dma40 branch with Herbert's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/39] crypto: ux500/hash - Set DMA configuration though dma_slave_config()

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The DMA controller currently takes configuration information from
 information passed though dma_channel_request(), but it shouldn't.
 Using the API, the DMA channel should only be configured during
 a dma_slave_config() call.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied to my dma40 branch with Herbert's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/39] ARM: ux500: Stop passing Hash DMA channel config information though pdata

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 DMA channel configuration information should be setup in the driver.
 The Ux500 Hash driver now does this, so there's no need to send it
 though here too.

 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied now that the deps are in!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/39] crypto: ux500/cryp - Prepare clock before enabling it

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 If we fail to prepare the ux500-cryp clock before enabling it the
 platform will fail to boot. Here we insure this happens.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Ulf Hansson ulf.hans...@linaro.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Herbert's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/39] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The DMA controller currently takes configuration information from
 information passed though dma_channel_request(), but it shouldn't.
 Using the API, the DMA channel should only be configured during
 a dma_slave_config() call.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Herbert's ACK.

Thanks,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/39] crypto: ux500/[cryp|hash] - Show successful start-up in the bootlog

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 The Cryp driver is currently silent and the Hash driver prints the
 name of its probe function unnecessarily. Let's just put a nice
 descriptive one-liner there instead.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied with Herbert's ACK.

Thanks,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 19/39] ARM: ux500: Register Cyrp and Hash platform drivers on Snowball

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 These drivers are now operational and even use the latest common clk
 and DMA APIs. There's no reason why we shouldn't start them up now.

 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied now that the deps are in place.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 For all ux500 based platforms the maximum number of end-points are used.
 Move this knowledge into the driver so we can relinquish the burden from
 platform data. This also removes quite a bit of complexity from the driver
 and will aid us when we come to enable the driver for Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

I have now gone over and collected ACKs etc for the patches up until here.

Now you need Felipe's consent to proceed with the MUSB changes
through my tree.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Wed, May 15, 2013 at 10:14 PM, Fabio Baltieri
fabio.balti...@linaro.org wrote:
 On Wed, May 15, 2013 at 07:18:01PM +0200, Linus Walleij wrote:
 On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

  For all ux500 based platforms the maximum number of end-points are used.
  Move this knowledge into the driver so we can relinquish the burden from
  platform data. This also removes quite a bit of complexity from the driver
  and will aid us when we come to enable the driver for Device Tree.
 
  Cc: Felipe Balbi ba...@ti.com
  Cc: linux-...@vger.kernel.org
  Acked-by: Linus Walleij linus.wall...@linaro.org
  Acked-by: Fabio Baltieri fabio.balti...@linaro.org
  Signed-off-by: Lee Jones lee.jo...@linaro.org

 I guess this stuff is dependent on the stuff Fabio has recently sent to
 Felipe for the ux500 musb DMA so these musb patches should
 primarily go through his tree.

 Not really, I only sent some coding style fix for the dma driver, as
 almost all of my work was in the usb phy files.  I also checked my
 latest series against Lee's tree and the two seems to merge without any
 conflict, so as far as I'm concerned it's really up to you and Felipe.

OK so Felipe can merge these smallish patches to musb, fine...

The other (DMA) stuff needs to be merged together with the rest
of the DMA40 series, so here basically Felipe needs an indication
if he can ACK it so I merge it through ARM SoC without colissions
and it sounds like we can make the assumption that this will work.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/39] dmaengine: ste_dma40: Separate Logical Global Interrupt Mask (GIM) unmasking

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 During the initial setup of a logical channel, it is necessary to unmask
 the GIM in order to receive generated terminal count and error interrupts.
 We're separating out this required code so it will be possible to move
 the remaining code in d40_phy_cfg(), which is mostly runtime configuration
 into the runtime_config() routine.

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Tentatively applied to my ux500-dma40 branch.

This lacks an ACK from Vinod...

I cannot get any of this stack of patches up to ARM SoC
before I have Vinod's ACK on all hitting drivers/dma/*

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/39] dmaengine: ste_dma40: Don't configure runtime configurable setup during allocate

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Using the dmaengine API, allocating and configuring a channel are two
 separate actions. Here we're removing logical channel configuration from
 the channel allocating routines.

This commit message is also incorrect, but I amended it.

Besides, when I amend commit messages I make sure I really
understand what is going on so no big deal...

 Cc: Vinod Koul vinod.k...@intel.com
 Cc: Dan Williams d...@fb.com
 Cc: Per Forlin per.for...@stericsson.com
 Cc: Rabin Vincent ra...@rab.in
 Signed-off-by: Lee Jones lee.jo...@linaro.org

I still need Vinod's ACK on this before I send it anywhere.

Patch applied!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/39] ARM: ux500: Stop passing UART's platform data for Device Tree boots

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 It was required to pass DMA channel configuration information to the
 UART driver before the new DMA API was in place. Now that it is, and
 is fully compatible with Device Tree we can stop doing that.

 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  arch/arm/mach-ux500/cpu-db8500.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/arch/arm/mach-ux500/cpu-db8500.c 
 b/arch/arm/mach-ux500/cpu-db8500.c
 index 38ebf2b..9b26fe2 100644
 --- a/arch/arm/mach-ux500/cpu-db8500.c
 +++ b/arch/arm/mach-ux500/cpu-db8500.c
 @@ -230,9 +230,9 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] 
 __initdata = {
 /* Requires call-back bindings. */
 OF_DEV_AUXDATA(arm,cortex-a9-pmu, 0, arm-pmu, 
 db8500_pmu_platdata),
 /* Requires DMA bindings. */
 -   OF_DEV_AUXDATA(arm,pl011, 0x8012, uart0, uart0_plat),
 -   OF_DEV_AUXDATA(arm,pl011, 0x80121000, uart1, uart1_plat),
 -   OF_DEV_AUXDATA(arm,pl011, 0x80007000, uart2, uart2_plat),
 +   OF_DEV_AUXDATA(arm,pl011, 0x8012, uart0, NULL),
 +   OF_DEV_AUXDATA(arm,pl011, 0x80121000, uart1, NULL),
 +   OF_DEV_AUXDATA(arm,pl011, 0x80007000, uart2, NULL),
 OF_DEV_AUXDATA(arm,pl022, 0x80002000, ssp0,  ssp0_plat),
 OF_DEV_AUXDATA(arm,pl18x, 0x80126000, sdi0,  mop500_sdi0_data),
 OF_DEV_AUXDATA(arm,pl18x, 0x80118000, sdi1,  mop500_sdi1_data),

OK dma_request_slave_channel() is upstream in the PL011 driver,
so applied!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/39] ARM: ux500: Stop passing MMC's platform data for Device Tree boots

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 It was required to pass DMA channel configuration information to the
 MMC driver before the new DMA API was in place. Now that it is, and
 is fully compatible with Device Tree we can stop doing that.

 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

So since the use of dma_request_slave_channel() is not upstream,
I guess this will break DMA use (i.e slow down transfers!) on all
device tree boots?

I'd be happy to apply it once the MMCI patch is in linux-next
indicating there may just be a window in the merge period
where it falls back to IRQ mode, but I don't want to disable
DMA on DT boots for an entire kernel cycle just like that.

Not applied as of yet.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/39] dmaengine: ste_dma40: Only use addresses passed as configuration information

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Addresses are passed in from the client's driver via the invocation of
 dmaengine_slave_config(), so there's no need to fetch them from platform
 data too, hardwired or otherwise. This is a great step forward, as it
 elevates a large burden from platform data in the way of a look-up
 table.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

This should be the case even without patch 5  6 so patch tentatively
applied.

Waiting for Vinod's ACK on this though.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/39] dmaengine: ste_dma40: Remove redundant address fetching function

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Addresses are now stored in local data structures and are easy to
 obtain, thus a specialist function used to fetch them is now surplus
 to requirement.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch tentatively applied, waiting for Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/39] ARM: ux500: Move SDI (MMC) and UART devices under more descriptive heading

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 Now DMA DT bindings exist and are in use by he MMC and UART drivers, it
 should be possible to remove them from the auxdata structure. However,
 after doing so the drivers fail. Common clk is still reliant on the
 dev_name() call to do device name matching, which will fail due to the
 fact that Device Tree naming differs somewhat do the more traditional
 conventions.

 Reviewed-by: Linus Walleij linus.wall...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Cannot be applied due to dependency on 5/39.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/39] ARM: ux500: Remove DMA address look-up table

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 DMA addresses are now passed as part of the dmaengine API by invoking
 dmaengine_slave_config(). So there's no requirement for the DMA40
 driver to look them up in a table provided by platform data. This
 method does not fit in well using Device Tree either.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Good riddance. Patch tentatively applied.
Awaiting Vinod's ACK for include/platform_data/*.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/39] dmaengine: ste_dma40: Correct copy/paste error

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 'struct stedma40_half_channel_info's header comment says that it's
 called 'struct stedma40_chan_cfg'. Let's straighten that out.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch tentatively applied waiting for Vinod's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/39] crypto: ux500/hash - Prepare clock before enabling it

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 If we fail to prepare the ux500-hash clock before enabling it the
 platform will fail to boot. Here we insure this happens.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Acked-by: Ulf Hansson ulf.hans...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

This patch seems to be totally independent of the other stuff
in the patch series, and it even seems like an -rc fix to me
(I don't think the crypto device works without this).

This should go into the crypto tree?

Else please convince Herbert to give his ACK on this
before I apply it.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/39] ARM: ux500: Remove unnecessary attributes from DMA channel request pdata

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 DMA data width and packet size information is only required at channel
 configuration time. Any information passed from platform data is passed
 directly to the DMA40 driver to use during channel allocation, but these
 pieces of information are subsequently ignored by the driver, so we may
 as well remove them.

 Signed-off-by: Lee Jones lee.jo...@linaro.org

Patch applied.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/39] usb: musb: ux500: move channel number knowledge into the driver

[Linus Walleij]

On Wed, May 15, 2013 at 11:51 AM, Lee Jones lee.jo...@linaro.org wrote:

 For all ux500 based platforms the maximum number of end-points are used.
 Move this knowledge into the driver so we can relinquish the burden from
 platform data. This also removes quite a bit of complexity from the driver
 and will aid us when we come to enable the driver for Device Tree.

 Cc: Felipe Balbi ba...@ti.com
 Cc: linux-...@vger.kernel.org
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Fabio Baltieri fabio.balti...@linaro.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

I guess this stuff is dependent on the stuff Fabio has recently sent to
Felipe for the ux500 musb DMA so these musb patches should
primarily go through his tree.

It seems like the later changes to the platform code
(arch/arm/mach-ux500) may be sufficiently orthogonal
so it can be done out-of-order?

I can't merge any of this without Felipes ACKs in any
case.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Linus Walleij]

On Thu, Apr 25, 2013 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote:

 The dma engine driver must know the address in its dma space, while the
 slave driver has it available in physical space. These two are often the
 same, but there is no generic way to convert between the two, especially
 if the dma engine resides behind an IOMMU.

 The best assumption we can make is that the dma engine driver knows
 how to convert between the two. Interestingly the documentation for
 dma_slave_config talks about physical address, while the structure
 itself uses a dma_addr_t. Linus Walleij introduced the structure in
 c156d0a5b0 DMAENGINE: generic slave channel control v3, so I assume
 he can shed some light on what he was thinking. I assume the documentation
 is right but the structure is not and should be converted to use
 phys_add_t or resource_size_t.

OK I could cook a patch for that, but I think I need some input from
Vinod and/or Russell on this.

It does make sense to me that if anything knows how to map a physical
address into the DMA address space it'll be the DMA engine.

However this rings a bell that there may be a possible relation to
DMA-API, since that API syncs memory buffers to the DMA
address space if there is some MMU inbetween the DMA and the
(ordinary, non-device) memory.

So if we think one step ahead, assuming the DMAC is actually behind
an MMU making it see the device in some other address than the
physical (bus) space, where would the address be resolved?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Linus Walleij]

On Fri, Apr 26, 2013 at 10:16 AM, Vinod Koul vinod.k...@intel.com wrote:

 OK I could cook a patch for that, but I think I need some input from
 Vinod and/or Russell on this.

 the dma_slave_config is physical address that should be passed directly to the
 controller. Obviosuly it should phys_addr_t :)

OK! Sent a patch for this, check it out.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9 v2] crypto: ux500/hash - Prepare clock before enabling it

[Linus Walleij]

On Fri, Apr 19, 2013 at 2:24 PM, Lee Jones lee.jo...@linaro.org wrote:
 Slight change of plan for v2.

 Now we're doing a seperate clk_prepare(), as the clk_enable() in the
 previous patch turned out to be called inside a spin_lock().

 Arnd, can you confirm your Ack please?

Do you really want letters to Arnd to be part of the commit log?


 ---

Note: stuff following the three dashes (---) will be *omitted*
from the change log. This seems to be turned upside-down.


 crypto: ux500/hash - Prepare clock before enabling it

 If we fail to prepare the ux500-hash clock before enabling it the
 platform will fail to boot. Here we insure this happens.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Pls include Ulf Hansson ulf.hans...@linaro.org on this patch.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] crypto: ux500/hash - Set DMA configuration though dma_slave_config()

[Linus Walleij]

Pls include magnus.p.pers...@stericsson.com on all these crypto/hash
postings.

On Thu, Apr 18, 2013 at 12:26 PM, Lee Jones lee.jo...@linaro.org wrote:

 The DMA controller currently takes configuration information from
 information passed though dma_channel_request(), but it shouldn't.
 Using the API, the DMA channel should only be configured during
 a dma_slave_config() call.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

(...)
  #define HASH_BLOCK_SIZE64
 +#define HASH_DMA_FIFO  4

This is an address so write 0x0004 here please. Some hex notation atleast.

  /**
   * struct hash_device_data - structure for a hash device.
 - * @base:  Pointer to the hardware base address.
 + * @base:  Pointer to virtual base address of the hash device.
 + * @phybase:   Pointer to physical memory location of the hash 
 device.
   * @list_node: For inclusion in klist.
   * @dev:   Pointer to the device dev structure.
   * @ctx_lock:  Spinlock for current_ctx.
 @@ -361,6 +363,7 @@ struct hash_req_ctx {
   */
  struct hash_device_data {
 struct hash_register __iomem*base;
 +   phys_addr_t phybase;

What you pass to the config function is a dma_addr_t actually.

This is the same thing on the platform, but generally:

phys_addr_t = in the address space as the memory controller sees it.
dma_addr_t = in the address space as the DMA controller sees it.

Often the same. Not always. So use dma_addr_t.

Apart from this a real nice patch!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9 v2] crypto: ux500/cryp - Prepare clock before enabling it

[Linus Walleij]

On Fri, Apr 19, 2013 at 2:22 PM, Lee Jones lee.jo...@linaro.org wrote:

 Slight change of plan for v2.

 Now we're doing a seperate clk_prepare(), as the clk_enable() in the
 previous patch turned out to be called inside a spin_lock().

 Arnd, can you confirm your Ack please?

 

 crypto: ux500/cryp - Prepare clock before enabling it

 If we fail to prepare the ux500-cryp clock before enabling it the
 platform will fail to boot. Here we insure this happens.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Acked-by: Arnd Bergmann a...@arndb.de
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Same formatting problems as the other prepare/enable
patch. Include Ulf Hansson.

(Apart from this it looks OK.)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] crypto: ux500/cryp - Fix compile error

[Linus Walleij]

On Thu, Apr 18, 2013 at 12:27 PM, Lee Jones lee.jo...@linaro.org wrote:

 Clearly this driver hasn't been tested, or even enabled in a while.

 drivers/crypto/ux500/cryp/cryp_core.c:1771:3:
 error: request for member ‘pm’ in something not a structure or union

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

NAK. I already fixed this. It is in v3.9-rc7.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] crypto: ux500/[cryp|hash] - Show successful start-up in the bootlog

[Linus Walleij]

On Thu, Apr 18, 2013 at 12:27 PM, Lee Jones lee.jo...@linaro.org wrote:

 The Cryp driver is currently silent and the Hash driver prints the
 name of its probe function unnecessarily. Let's just put a nice
 descriptive one-liner there instead.

 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: Andreas Westin andreas.wes...@stericsson.com
 Cc: linux-crypto@vger.kernel.org
 Signed-off-by: Lee Jones lee.jo...@linaro.org

Reviewed-by: Linus Walleij linus.wall...@linaro.org

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] crypto: ux500/cryp - Set DMA configuration though dma_slave_config()

[Linus Walleij]

On Thu, Apr 25, 2013 at 3:44 PM, Lee Jones lee.jo...@linaro.org wrote:
 On Thu, 25 Apr 2013, Linus Walleij wrote:
 On Thu, Apr 18, 2013 at 12:27 PM, Lee Jones lee.jo...@linaro.org wrote:

  The DMA controller currently takes configuration information from
  information passed though dma_channel_request(), but it shouldn't.
  Using the API, the DMA channel should only be configured during
  a dma_slave_config() call.
 
  Cc: Herbert Xu herb...@gondor.apana.org.au
  Cc: David S. Miller da...@davemloft.net
  Cc: Andreas Westin andreas.wes...@stericsson.com
  Cc: linux-crypto@vger.kernel.org
  Signed-off-by: Lee Jones lee.jo...@linaro.org

 (...)
   /* Cryp DMA interface */
  +#define HASH_DMA_TX_FIFO   0x08
  +#define HASH_DMA_RX_FIFO   0x10

 Yes, this is nice address notation :-)

   /**
* struct cryp_device_data - structure for a cryp device.
  - * @base: Pointer to the hardware base address.
  + * @base: Pointer to virtual base address of the cryp device.
  + * @phybase: Pointer to physical memory location of the cryp device.
* @dev: Pointer to the devices dev structure.
* @clk: Pointer to the device's clock control.
* @pwr_regulator: Pointer to the device's power control.
  @@ -232,6 +236,7 @@ struct cryp_dma {
*/
   struct cryp_device_data {
  struct cryp_register __iomem *base;
  +   phys_addr_t phybase;

 Use dma_addr_t. Maybe phybase is misleading,
 dmabase is probably better. (Also applies to the
 cryp patch).

 Accept it's not the dmabase.

 It's the phybase (U8500_CRYP1_BASE) i.e. the physical base address of
 the device's regs.

So when you assign the src_addr or dst_addr in struct
dmaengine_slave_config in this code,
you notice that this looks like so:

struct dma_slave_config {
enum dma_transfer_direction direction;
dma_addr_t src_addr;
dma_addr_t dst_addr;
enum dma_slave_buswidth src_addr_width;
enum dma_slave_buswidth dst_addr_width;
u32 src_maxburst;
u32 dst_maxburst;
bool device_fc;
};

So when you do this:

+   struct dma_slave_config mem2cryp = {
+   .direction = DMA_MEM_TO_DEV,
+   .dst_addr = device_data-phybase + HASH_DMA_TX_FIFO,
+   .dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES,
+   .dst_maxburst = 4,
+};

on .dst_addr you are effectively casting a phys_addr_t
into a dma_addr_t.

But  we must do this at some point, and now I think
that doing it here may be just as good (because we might
just add a physical-to-dma memory translation if need
be).

So allright.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: ux500: add missing comma

[Linus Walleij]

From: Linus Walleij linus.wall...@linaro.org

Commit 4f31f5b19eb0418a847b989abc9ac22af1991fe2
PM / crypto / ux500: Use struct dev_pm_ops for power management
add a new line to the driver struct but missed to add a
trailing comma, causing build errors when crypto is
selected. This adds the missing comma.

This was not noticed until now because the crypto block
is not in the ux500 defconfig. A separate patch will
be submitted to fix this.

Cc: sta...@vger.kernel.org # 3.8.x
Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Magnus Myrstedt magnus.p.pers...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
 drivers/crypto/ux500/cryp/cryp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/ux500/cryp/cryp_core.c 
b/drivers/crypto/ux500/cryp/cryp_core.c
index 8bc5fef..22c9063 100644
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -1750,7 +1750,7 @@ static struct platform_driver cryp_driver = {
.shutdown = ux500_cryp_shutdown,
.driver = {
.owner = THIS_MODULE,
-   .name  = cryp1
+   .name  = cryp1,
.pm= ux500_cryp_pm,
}
 };
-- 
1.7.11.3

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: ux500: hash: remove unneeded return at ux500_hash_mod_fini

[Linus Walleij]

On Fri, Aug 24, 2012 at 10:33 AM, Devendra Naga
develkernel412...@gmail.com wrote:

 Signed-off-by: Devendra Naga develkernel412...@gmail.com

Acked-by: Linus Walleij linus.wall...@linaro.org

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Crypto Update for 3.5

[Linus Walleij]

On Thu, May 24, 2012 at 1:06 AM, Linus Torvalds
torva...@linux-foundation.org wrote:

 There's a declaration for dbx500_add_platform_device_noirq() that does
 not exist and is not used anywhere. Why? It was added in commit
 585d188f8072, and I see no rhyme or reason to it.

 I only noticed because I happened to get a conflict due to the
 location it was added. I removed it. WTF is going on?

AFAICT this was retrofitted to get a booting kernel on
the cryptodev base which was based on something old like
v3.2 just some weeks back. (Now it's upgraded to mainline, great!)

Greg was pushing for us to allocate all devices dynamically at
one point, but there was no real infrastructure for it and some local
implementations to meet that requirement, that's why these
functions pop up and down. We're working on it with device tree
etc, mea culpa...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mach-ux500: Crypto: core support for CRYP/HASH module.

[Linus Walleij]

On Tue, May 8, 2012 at 9:37 PM, Arnd Bergmann a...@arndb.de wrote:

 On Tuesday 08 May 2012, Andreas Westin wrote:
 This adds the required platform data and calls to enable
 the CRYP/HASH driver.

 index 15a0f63..a69db46 100644
 --- a/arch/arm/mach-ux500/id.c
 +++ b/arch/arm/mach-ux500/id.c
 @@ -17,7 +17,10 @@
  #include mach/hardware.h
  #include mach/setup.h

 +#include linux/module.h
 +
  struct dbx500_asic_id dbx500_id;
 +EXPORT_SYMBOL(dbx500_id);

  static unsigned int ux500_read_asicid(phys_addr_t addr)
  {

 This needs an explanation! Why do you export an internal data structure
 to non-GPL modules?

This does not look like it's needed at all, none of the other two patches use
it. Andreas can you just drop this hunk of the patch?

BTW if the asic variant is needed by some driver use cpu_is_*() from
mach/id.h or pass a flag in platform data or something.

 On a more general topic, I wonder if it's really necessary to add new
 devices to the legacy probing path in mach-ux500. Why not make new drivers
 for this platform dt-only, so we don't need to add code now that we will
 have to remove again in one or two kernel releases?

This is a simple platform_device which means that its IRQ/PIO mode is
already supported with standard bindings if you instantiate the device
from drivers/of/platform.c using the generic bindings for platform devices
and their resources.

The only thing that is passed in platform data, as you can see is the
DMA channels and configuration, which is debated elsewhere I
think. The day we know how to pass DMA channels to any platform/amba
device this device should be trivial to fully support with DT.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mach-ux500: Crypto: core support for CRYP/HASH module.

[Linus Walleij]

On Wed, May 9, 2012 at 2:17 PM, Arnd Bergmann a...@arndb.de wrote:
 On Wednesday 09 May 2012, Andreas WESTIN wrote:
 On 2012-05-09 10:36, Linus Walleij wrote:
  This needs an explanation! Why do you export an internal data structure
  to non-GPL modules?
 
  This does not look like it's needed at all, none of the other two patches 
  use
  it. Andreas can you just drop this hunk of the patch?

 It is needed if you compile the drivers as modules, but it should be a
 GPL export. I will change it.

 How about using distinct identification strings for each version of the
 crypto hardware? The driver should really only care about what kind
 of device it is talking to, not which SoC it is built into.

Do you mean like this (from a recent pinctrl driver):

static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
{
const struct platform_device_id *platid = platform_get_device_id(pdev);

   (Here we use that ID to control runtime codepath)
}

static const struct platform_device_id nmk_pinctrl_id[] = {
{ pinctrl-stn8815, PINCTRL_NMK_STN8815 },
{ pinctrl-db8500, PINCTRL_NMK_DB8500 },
};

static struct platform_driver nmk_pinctrl_driver = {
.driver = {
.owner = THIS_MODULE,
.name = pinctrl-nomadik,
},
.probe = nmk_pinctrl_probe,
.id_table = nmk_pinctrl_id,
};

Here one version of ASIC registers the pinctrl-db8500 device.
And so on.

So instead of registering cryp1 and hash1, register
db8500-cryp-v1, db8500-cryp-v2 etc for the versions,
then use the ID to control code path.

Is that what you were thinking of?

That might be a good idea, however it requires some changes to patches
earlier in the series (but can be done as an incremental add-on of
course).

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/3] crypto: ux500 crypto and hash driver

[Linus Walleij]

On Fri, May 4, 2012 at 11:56 AM, Herbert Xu herb...@gondor.apana.org.au wrote:
 On Mon, Apr 30, 2012 at 10:11:16AM +0200, Andreas Westin wrote:
 Resending, something seems to have gone wrong the first time.

 Changed config to depend on correct ARCH, also removed an
 unnecessary if statement check in the hash driver.

 This adds a driver for the ST-Ericsson ux500 crypto hardware
 module. It supports AES, DES and 3DES, the driver implements
 support for AES-ECB,CBC and CTR.

 Patches are also available at: http://www.df.lth.se/~triad/ux500-crypto/

 All applied to cryptodev.  Thanks a lot!

The last patch [3/3] will probably have conflicts since we have a patch
deleting U5500 support pending in the ARM SoC tree.

Stephen usually fixes that up, but you could also drop the hunks
touching devices-db5500.h, board-u5500.c and ste-dma40-db5500.h
from that patch.

Apart from that, THANKS Herbert!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] RFC: New /dev/crypto user-space interface

[Linus Walleij]

Hi Miloslav,

first, thanks a lot for working on the userspace API, we have long missed this
API and I've asked some times in the past about the status and proposals have
been stalled some times, so it is really fun to see that something is happening!

We recognize that since Redhat is providing hardware for governments, this
work will likely continue until the desired mechanism is available in a form
that can be integrated and shipped, which gives great confidence that it is
going to happen this time.

At ST-Ericsson we have considered to provide a user API to the kernel crypto
facilities as well, we have some rough initial ideas on how to go about this.
It has some similarities to what you/Redhat has presented, but we have some
further goals:

1. We'd like the API have to be general enough to not change with new
   algorithms. Having to recompile user space programs becasuse of a minor
   change is messing things up for us. This may be solved by a
   functionality which presents the available crypto resources in the
   user API. (As opposed to using enums for the algorithms.)

   c.f how the ALSA mixer presents a lot of things to userspace without
   using any enums at all in /dev/snd/controlC0 for card 0. For example
   in include/linux/soundcard.h you find the different control knobs
   enumerated with strings so as to avoid explicit enums.

   We'd try to avoid as many enums as possible, and really only define
   the necessary information nodes so that userspace can look for
   strings like aes-plain instead, which is the same that dmcrypt
   is using, and there are already descriptions of available algorithms
   in /proc/crypto (from crypto/proc.c) using this format.

2. To avoid security hazards the API would benefit from being programmed
   with at least some secure programming concepts in mid. Input argument
   checking separate from algorithm separate from output argument checking,
   and erasing of information from stacks and buffers. More or less
   the advice you will find in places like:
   http://www.dwheeler.com/secure-programs/

   (Evidently we and others will help out reviewing and patching up
   proposed code in this aspect, also since you're working on
   government business I believe security audits will be mandatory?)

   For internal keys, a function for compare of HMAC function results
   could improve security considerably.

3. A general interface for stream ciphers would be nice. The only differences
   are that they do not operate on blocks, but bits, and that they
always require
   an IV. Arguably this can be added later if the aspect if just considered when
   devising the interface. The recent discussion in this thread
regarding netlink
   points in a direction where streams are a natural part of the concept I
   believe.

There are some submission-related comments as well:

- The API description in the commit log of patch 1 should be a file in
  Documentation/crypto/usespace-api.txt or so instead, this is of general
  interest.

- The big patch 0002 to crypto/userspace is including the Makefile
  changes for patch 0004 making it non-bisectable. Also it is very large,
  is it possible to break it down a little? Many files are prefixed ncr-*
  and I don't see why - can this prefix simply be removed?

  I hope the patch 0004 with software fallbacks is not strictly required
  by the userspace API so these two can be considered separately?
  Else can you describe how the dependecies go.. surely  it must be
  possible to patch in the software fallbacks in libtom* separately?

- The big patch containing the entire libtomcrypt should be destined to
  drivers/staging or similar at this point (OK it is not a driver, should have
  been lib/staging if such a thing existed). The sheer size of it makes it very
  hard to review, and all attempts at breaking it in smaller pieces would
  be much appreciated. For example is it possible to have one patch per
  algorithm?

  Also: isn't this creating a fork of the library? Not that it matters much as
  Linux is finding good use of it.

  Since it is a fork, it should be adopted to the Linux source hiearchy, and
  since here every algorithm is directly in crypto/ please remove all the
  libtomcrypt subdir and put all directly into crypto/ for simplicity (subdirs
  below like hashes, headers etc are nice and should be preserved though).

  libtomath seems to be duplicating a lot of in-kernel stuff already found
  inside the kernel, and needs to be merged with care. Arguably parts of
  this can be cleaned-up later but it'd be nice to make some initial attempts
  at unifying the math infrastructure.

Overall, thanks for working on this.

Yours,
Linus Walleij (et al)
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Userspace API /dev/crypto

[Linus Walleij]

Hi,

IMO the design discussion of a user space API in:
http://marc.info/?l=linux-crypto-vgerm=121088311223921w=2
Userspace API proposal

And the consequent attempt at a patch for OpenSSL in:
http://marc.info/?l=linux-crypto-vgerm=121822090729047w=2
OpenSSL patch to support Linux CryptoAPI

Was a first nice draft of a desirable userspace API for various crypto/hash
acceleration.

Has there been some/any progress on the userspace API since, or
is this up for grabs?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html