From: Gao Pan <[email protected]>
When i2c peripheral clk rate is 0 directly return -EINVAL.
Signed-off-by: Gao Pan <[email protected]>
Signed-off-by: Carlos Song <[email protected]>
---
Changes for V2:
- adjust the subject from "debug message" to "error message"
Changes for V3:
- remove output error log when i2c peripheral clk doesn't work
- adjust commit log and subject
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index c3287c887c6f..150d923ca7f1 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -209,6 +209,9 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
lpi2c_imx_set_mode(lpi2c_imx);
clk_rate = clk_get_rate(lpi2c_imx->clks[0].clk);
+ if (!clk_rate)
+ return -EINVAL;
+
if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
filt = 0;
else
--
2.34.1
Hi Carlos,
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -209,6 +209,9 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
> lpi2c_imx_set_mode(lpi2c_imx);
>
> clk_rate = clk_get_rate(lpi2c_imx->clks[0].clk);
> + if (!clk_rate)
> + return -EINVAL;
> +
this is a very unlikely to happen and generally not really
appreciated.
If you got so far it's basically impossible that clk_rate is '0'.
Uwe asked you in v2 if you actually had such case.
I don't have a strong opinion, thoug... I would drop this patch
unless Dong is OK with it and I can accept it with his ack.
Andi
> if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> filt = 0;
> else
> --
> 2.34.1
>
Hi, Andi
According to Aisheng, I find that if the i2c clock rate is not assigned on MX8,
due to chip architecture, the default speed will be 0, it cause our i2c not work.
The judgment will be triggered easily. This is a special case for imx.
The log like this:
[ 2.401402] imx-lpi2c 5a800000.i2c: use pio mode
[ 2.419788] i2c i2c-4: clk_per rate is 0
[ 2.423724] i2c i2c-4: clk_per rate is 0
[ 2.444071] i2c i2c-4: clk_per rate is 0
[ 2.448011] fxos8700_i2c 4-001e: Error reading chip id
[ 2.453172] fxos8700_i2c: probe of 4-001e failed with error -22
[ 2.459271] i2c i2c-4: supply vdd not found, using dummy regulator
[ 2.465522] i2c i2c-4: supply vddio not found, using dummy regulator
[ 2.471913] i2c i2c-4: clk_per rate is 0
[ 2.475867] fxas21002c_i2c: probe of 4-0020 failed with error -22
[ 2.482066] i2c i2c-4: clk_per rate is 0
[ 2.495716] i2c i2c-4: clk_per rate is 0
[ 2.505464] i2c i2c-4: clk_per rate is 0
[ 2.514786] i2c i2c-4: LPI2C adapter registered
So the patch can not be dropped.
> -----Original Message-----
> From: Andi Shyti <[email protected]>
> Sent: Wednesday, July 26, 2023 7:41 AM
> To: Carlos Song <[email protected]>
> Cc: [email protected]; Aisheng Dong <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; Clark Wang <[email protected]>; Bough Chen
> <[email protected]>; dl-linux-imx <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: [EXT] Re: [PATCH v4] i2c: imx-lpi2c: return -EINVAL when i2c peripheral
> clk doesn't work
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
>
>
> Hi Carlos,
>
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -209,6 +209,9 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct
> *lpi2c_imx)
> > lpi2c_imx_set_mode(lpi2c_imx);
> >
> > clk_rate = clk_get_rate(lpi2c_imx->clks[0].clk);
> > + if (!clk_rate)
> > + return -EINVAL;
> > +
>
> this is a very unlikely to happen and generally not really appreciated.
>
> If you got so far it's basically impossible that clk_rate is '0'.
> Uwe asked you in v2 if you actually had such case.
>
> I don't have a strong opinion, thoug... I would drop this patch unless Dong is OK
> with it and I can accept it with his ack.
>
> Andi
>
> > if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> > filt = 0;
> > else
> > --
> > 2.34.1
> >
> From: Andi Shyti <[email protected]>
> Sent: 2023??7??26?? 7:41
> Hi Carlos,
>
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -209,6 +209,9 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct
> *lpi2c_imx)
> > lpi2c_imx_set_mode(lpi2c_imx);
> >
> > clk_rate = clk_get_rate(lpi2c_imx->clks[0].clk);
> > + if (!clk_rate)
> > + return -EINVAL;
> > +
>
> this is a very unlikely to happen and generally not really appreciated.
>
> If you got so far it's basically impossible that clk_rate is '0'.
> Uwe asked you in v2 if you actually had such case.
>
> I don't have a strong opinion, thoug... I would drop this patch unless Dong is
> OK with it and I can accept it with his ack.
On MX8X platforms, the default clock rate is 0 if without explicit clock setting in
dts nodes. So I wonder it may be worth adding a check here.
If you're also ok, feel free to add my tag.
Acked-by: Dong Aisheng <[email protected]>
BTW, please see another reply from Carlos with the test log.
Regards
Aisheng
>
> Andi
>
> > if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
> > filt = 0;
> > else
> > --
> > 2.34.1
> >