[PATCH] i2c: efm32: Prevent potential division by zero

[PATCH] i2c: efm32: Prevent potential division by zero

[Axel Lin]

Make sure we don't OOPS in case clock-frequency is set to 0 in a DT. The
variable set here is later used as a divisor.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/i2c/busses/i2c-efm32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-efm32.c b/drivers/i2c/busses/i2c-efm32.c
index 8eff627..394c695 100644
--- a/drivers/i2c/busses/i2c-efm32.c
+++ b/drivers/i2c/busses/i2c-efm32.c
@@ -389,7 +389,7 @@ static int efm32_i2c_probe(struct platform_device *pdev)
 	ddata->location = location;
 
 	ret = of_property_read_u32(np, "clock-frequency", &frequency);
-	if (!ret) {
+	if (!ret && frequency != 0) {
 		dev_dbg(&pdev->dev, "using frequency %u\n", frequency);
 	} else {
 		frequency = 100000;
-- 
2.5.0

Re: [PATCH] i2c: efm32: Prevent potential division by zero

[Uwe Kleine-König]

Hello Axel,

On Thu, Apr 14, 2016 at 03:42:54PM +0800, Axel Lin wrote:
> Make sure we don't OOPS in case clock-frequency is set to 0 in a DT. The
> variable set here is later used as a divisor.

This is correct in principle. I thought it's ok to misbehave if the DT
is broken? In this case the current code is just fine.

Best regards
Uwe

> diff --git a/drivers/i2c/busses/i2c-efm32.c b/drivers/i2c/busses/i2c-efm32.c
> index 8eff627..394c695 100644
> --- a/drivers/i2c/busses/i2c-efm32.c
> +++ b/drivers/i2c/busses/i2c-efm32.c
> @@ -389,7 +389,7 @@ static int efm32_i2c_probe(struct platform_device *pdev)
>  	ddata->location = location;
>  
>  	ret = of_property_read_u32(np, "clock-frequency", &frequency);
> -	if (!ret) {
> +	if (!ret && frequency != 0) {
>  		dev_dbg(&pdev->dev, "using frequency %u\n", frequency);
>  	} else {
>  		frequency = 100000;

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] i2c: efm32: Prevent potential division by zero

[Axel Lin]

2016-04-14 16:24 GMT+08:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello Axel,
>
> On Thu, Apr 14, 2016 at 03:42:54PM +0800, Axel Lin wrote:
>> Make sure we don't OOPS in case clock-frequency is set to 0 in a DT. The
>> variable set here is later used as a divisor.
>
> This is correct in principle. I thought it's ok to misbehave if the DT
> is broken? In this case the current code is just fine.

Current code hit division by zero if clock-frequency is 0.
clkdiv = DIV_ROUND_UP(rate, 8 * ddata->frequency) - 1;
Is it ok?

Re: [PATCH] i2c: efm32: Prevent potential division by zero

[Uwe Kleine-König]

Hello Axel,

On Thu, Apr 14, 2016 at 04:29:42PM +0800, Axel Lin wrote:
> 2016-04-14 16:24 GMT+08:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > On Thu, Apr 14, 2016 at 03:42:54PM +0800, Axel Lin wrote:
> >> Make sure we don't OOPS in case clock-frequency is set to 0 in a DT. The
> >> variable set here is later used as a divisor.
> >
> > This is correct in principle. I thought it's ok to misbehave if the DT
> > is broken? In this case the current code is just fine.
> 
> Current code hit division by zero if clock-frequency is 0.
> clkdiv = DIV_ROUND_UP(rate, 8 * ddata->frequency) - 1;
> Is it ok?

IMHO yes. If you give the kernel a broken dt it's ok that the kernel is
broken then.

So if you want, take my:

	Registered-but-considered-unimportant-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

:-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |