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

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.

---
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(-)

index 24bded4..5ff7bf5 100644
@@ -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)

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

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

Yes, basically.

Note that there are two time constants after which the device goes to sleep mode:
t_TIMEOUT (45 to 85 ms) - if I2C bus is inactive the device goes to sleep t_WATCHDOG (0.7 to 1.7s) - the device goes to sleep automatically at most
t_WATCHDOG time after wakeup, unless put into IDLE mode

If the RANDOM command was the only command the driver is issuing, I would simply make it so that the device is put to IDLE mode after each command.

But if there are other commands executed, maybe the implementation needs to be smarter. Depends on the commands.