| Message ID | 1460619774.4294.1.camel@ingics.com |
|---|---|
| State | Superseded |
| Headers | show |
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;
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
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
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;
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(-)