[PATCH] crypto: atmel-i2c - find the bus speed behind an I2C mux

[PATCH] crypto: atmel-i2c - find the bus speed behind an I2C mux

[Josef Schlehofer]

On the Turris Omnia, the ATSHA204A sits behind a PCA9547 I2C mux. The
driver reads "clock-frequency" from the mux channel, which has no such
property, so the probe fails with -EINVAL:

  atmel-sha204a 6-0064: failed to read clock-frequency property

A device tree patch in 2024 added the property to the mux channel. The
review rejected it, because the I2C core ignores the property there. The
driver would then compute the wake token for a wrong bus speed. The
review asked for a fix in the driver, but none was sent.

Keep reading the property of the client's adapter first, as before. If
it has none, look at the parent adapters. If no adapter has it, use
100 kHz, the default of most I2C controller drivers.

Fixes: 11105693fa05 ("crypto: atmel-ecc - introduce Microchip / Atmel ECC driver")
Cc: stable@vger.kernel.org # v5.19+
Reported-by: Marius Dinu <m95d@psihoexpert.ro>
Closes: https://lore.kernel.org/all/46ea55652f8bcdef51d5c552902fd0cc@psihoexpert.ro/
Link: https://lore.kernel.org/all/20240515104149.2ce3566e@dellmb/
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
---
Tested on a Turris Omnia with 7.2.13 and 6.18.59: the driver probes, and
/dev/hwrng reads random data from the ATSHA204A. The mv64xxx controller
stops at the address NAK, so this test does not exercise the token size.

 drivers/crypto/atmel-i2c.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 24bded4..5ff7bf5 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -347,11 +347,28 @@ free_cmd:
 	return ret;
 }
 
+/*
+ * A device behind an I2C mux sits on a channel adapter. Try the adapter of
+ * the client first, as before, then its parents. Without the property,
+ * assume 100 kHz, the default of most I2C controller drivers.
+ */
+static u32 atmel_i2c_bus_speed(struct i2c_adapter *adap)
+{
+	u32 bus_clk_rate;
+
+	for (; adap; adap = i2c_parent_is_i2c_adapter(adap)) {
+		if (!device_property_read_u32(&adap->dev, "clock-frequency",
+					      &bus_clk_rate) && bus_clk_rate)
+			return bus_clk_rate;
+	}
+
+	return I2C_MAX_STANDARD_MODE_FREQ;
+}
+
 int atmel_i2c_probe(struct i2c_client *client)
 {
 	struct atmel_i2c_client_priv *i2c_priv;
 	struct device *dev = &client->dev;
-	int ret;
 	u32 bus_clk_rate;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
@@ -360,14 +377,8 @@ int atmel_i2c_probe(struct i2c_client *client)
 	}
 
 	bus_clk_rate = i2c_acpi_find_bus_speed(&client->adapter->dev);
-	if (!bus_clk_rate) {
-		ret = device_property_read_u32(&client->adapter->dev,
-					       "clock-frequency", &bus_clk_rate);
-		if (ret) {
-			dev_err(dev, "failed to read clock-frequency property\n");
-			return ret;
-		}
-	}
+	if (!bus_clk_rate)
+		bus_clk_rate = atmel_i2c_bus_speed(client->adapter);
 
 	if (bus_clk_rate > I2C_MAX_FAST_MODE_PLUS_FREQ) {
 		dev_err(dev, "%u exceeds maximum supported clock frequency (1MHz)\n",
-- 
2.54.0 (Apple Git-157)

Re: [PATCH] crypto: atmel-i2c - find the bus speed behind an I2C mux

[Marek Behún]

Acked-by: Marek Behún <kabel@kernel.org>

But I would suggest not using this driver on Omnia for TRNG.

If you need TRNG, just use the one provided by the turris-omnia-mcu driver.
Upgrade your MCU firmware with the omnia-mcutool utility.

Why?

The Atmel SHA204A TRNG needs to update a part of its random seed in EEPROM
everytime it goes to sleep. Unless proper power management is implemented in
the driver, it will go to sleep 1.3 seconds after each wakeup.

The EEPROM is rated for ~100 000 cycles before it wears out.

Since proper power management is not implemented in the driver now, if you
enable this driver and read the TRNG continuosly, you can wear the EEPROM in
several days.

Marek

Re: [PATCH] crypto: atmel-i2c - find the bus speed behind an I2C mux

[Josef Schlehofer]

Hi Marek,

thanks for the ack and the warning.

The hwrng core starts a fill thread for the current RNG, whatever its
quality. It reads 32 bytes every 60 seconds, even when idle.
atmel_i2c_send_receive() puts the ATSHA204A to sleep after each
command. That means about 1440 seed updates per day, and 100k cycles
in about 70 days.

The ATSHA204A becomes the current RNG when the MCU firmware has no
TRNG. It also stays current when it registers first, because both RNGs
have quality 1024. Today the probe fails on the Omnia, so it is safe.
This patch could wear out its EEPROM within months. It is also marked
for stable.

The patch also lets the ATSHA204 on the Turris 1.x probe, as its I2C
controller has no clock-frequency either. There the talitos hwrng wins,
because the ATSHA204 has quality 1. Without talitos, the ATSHA204 would
wear out the same way.

Should we fix the wear before this patch goes in? Does "proper power
management" mean the idle command instead of sleep? I can look into
it.

Josef