i2c: efm32: Prevent potential division by zero
diff mbox

Message ID 1460619774.4294.1.camel@ingics.com
State New
Headers show

Commit Message

Axel Lin April 14, 2016, 7:42 a.m. UTC
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(-)

Comments

Uwe Kleine-König April 14, 2016, 8:24 a.m. UTC | #1
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;
Axel Lin April 14, 2016, 8:29 a.m. UTC | #2
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Uwe Kleine-König April 14, 2016, 9:06 a.m. UTC | #3
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

Patch
diff mbox

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;