ARM i.MX: Fix a kernel panic in i2c_imx_clk_notifier_call().

Message ID m3o9a2hni5.fsf@t19.piap.pl
State New
Headers show
Series
  • ARM i.MX: Fix a kernel panic in i2c_imx_clk_notifier_call().
Related show

Commit Message

Krzysztof =?utf-8?Q?Ha=C5=82asa?= Dec. 3, 2018, 11:13 a.m.
Signed-off-by: Krzysztof Halasa <khalasa@piap.pl>

Comments

Fabio Estevam Dec. 3, 2018, 11:21 a.m. | #1
Hi Krzysztof,

On Mon, Dec 3, 2018 at 9:13 AM Krzysztof HaƂasa <khalasa@piap.pl> wrote:
>
> Signed-off-by: Krzysztof Halasa <khalasa@piap.pl>

Please provide a commit log, giving some context to your fix.

Is this a regression?
Krzysztof =?utf-8?Q?Ha=C5=82asa?= Dec. 3, 2018, 1:28 p.m. | #2
Hi Fabio,

Fabio Estevam <festevam@gmail.com> writes:

> Please provide a commit log, giving some context to your fix.

Well, I hope Lucas could add something here. I am uncertain how it was
supposed to work, the ndata->clk (the pointer, not the clk pointed by
it) can't be at the same time a member of imx_i2c_struct, and I believe
the macro only does simple arithmetics to get to the outer struct.

@@ -510,9 +510,9 @@ static int i2c_imx_clk_notifier_call(struct notifier_block *nb,
 				     unsigned long action, void *data)
 {
 	struct clk_notifier_data *ndata = data;
-	struct imx_i2c_struct *i2c_imx = container_of(&ndata->clk,
+	struct imx_i2c_struct *i2c_imx = container_of(nb,
 						      struct imx_i2c_struct,
-						      clk);
+						      clk_change_nb);

> Is this a regression?

Probably (it went in between 4.16 and 4.17, commit id is
90ad2cbe88c22d0215225ab9594eeead0eb24fde). However this part may be
unused on many boards (apparently it only fires up if the "IPG" clock
rate changes), so it may not manifest itself. I only hit it when I added
a custom driver (using/requesting a special clock derived from IPG).

Patch

--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -510,9 +510,9 @@  static int i2c_imx_clk_notifier_call(struct notifier_block *nb,
 				     unsigned long action, void *data)
 {
 	struct clk_notifier_data *ndata = data;
-	struct imx_i2c_struct *i2c_imx = container_of(&ndata->clk,
+	struct imx_i2c_struct *i2c_imx = container_of(nb,
 						      struct imx_i2c_struct,
-						      clk);
+						      clk_change_nb);
 
 	if (action & POST_RATE_CHANGE)
 		i2c_imx_set_clk(i2c_imx, ndata->new_rate);