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

Message ID 20180628130729.17589-2-linus.walleij@linaro.org
State New
Headers show
Series
  • [1/9,v2] crypto: atmel-ecc: Make available for other platforms
Related show

Commit Message

Linus Walleij June 28, 2018, 1:07 p.m.
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 <linus.walleij@linaro.org>

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

-- 
2.17.0

Comments

Tudor Ambarus July 2, 2018, 9:32 a.m. | #1
Hi, Linus,

On 06/28/2018 04:07 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 with a warning

> if no clock frequency can be found, assuming it is safe.

> 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

> 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", &bus_clk_rate);

> -	if (ret) {

> -		dev_err(dev, "of: failed to read clock-frequency property\n");

> -		return ret;

> -	}

> -> -	if (bus_clk_rate > 1000000L) {

> +	if (ret)

> +		dev_warn(dev, "i2c host missing clock frequency information\n");


Not ok, bus_clk_rate is used to compute the wake_token. If you can't find the
clock-frequency then you will use the stack value of bus_clk_rate. If you don't
want to break the probe, then I propose to do the same assumption that is done
in i2c buses when clock-frequency is not found: assume it is 100kHZ. Although,
personally, I don't like this assumption and that's why I proposed that patch to
Wolfram. Nevertheless, let's do what we can in this conditions: make the same
assumption.

Thanks,
ta

> +	else if (bus_clk_rate > 1000000L) {

>  		dev_err(dev, "%d exceeds maximum supported clock frequency (1MHz)\n",

>  			bus_clk_rate);

>  		return -EINVAL;

>

Patch

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", &bus_clk_rate);
-	if (ret) {
-		dev_err(dev, "of: failed to read clock-frequency property\n");
-		return ret;
-	}
-
-	if (bus_clk_rate > 1000000L) {
+	if (ret)
+		dev_warn(dev, "i2c host missing clock frequency information\n");
+	else if (bus_clk_rate > 1000000L) {
 		dev_err(dev, "%d exceeds maximum supported clock frequency (1MHz)\n",
 			bus_clk_rate);
 		return -EINVAL;