[Patch V1] i2c: imx: init bus recovery info before adding i2c adapter

[Patch V1] i2c: imx: init bus recovery info before adding i2c adapter

[Gao Pan]

During driver probe, i2c_imx_init_recovery_info() must come before
i2c_add_numbered_adapter(), because the get/set_scl() functions
are assigned in i2c_register_adapter() under the conditon that bus
recover_info are initialized. Otherwise, get/set_scl() function
pointers never get assigned.

In such case, when i2c_generic_gpio_recovery() is used for bus recovery,
there will be kernel crash because bri->set_scl is NULL.

The solution to this bug is moving i2c_imx_init_recovery_info() before
i2c_register_adapter().

Signed-off-by: Gao Pan <b54642@freescale.com>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/i2c/busses/i2c-imx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 6dcfff5..0f41408 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1139,6 +1139,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
 			i2c_imx, IMX_I2C_I2CR);
 	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
 
+	i2c_imx_init_recovery_info(i2c_imx, pdev);
+
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
 	if (ret < 0) {
@@ -1146,8 +1148,6 @@ static int i2c_imx_probe(struct platform_device *pdev)
 		goto rpm_disable;
 	}
 
-	i2c_imx_init_recovery_info(i2c_imx, pdev);
-
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
 
-- 
1.9.1

Re: [Patch V1] i2c: imx: init bus recovery info before adding i2c adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

On Fri, Dec 04, 2015 at 05:33:14PM +0800, Gao Pan wrote:
> During driver probe, i2c_imx_init_recovery_info() must come before
> i2c_add_numbered_adapter(), because the get/set_scl() functions
> are assigned in i2c_register_adapter() under the conditon that bus
> recover_info are initialized. Otherwise, get/set_scl() function
> pointers never get assigned.
> 
> In such case, when i2c_generic_gpio_recovery() is used for bus recovery,
> there will be kernel crash because bri->set_scl is NULL.
> 
> The solution to this bug is moving i2c_imx_init_recovery_info() before
> i2c_register_adapter().
> 
> Signed-off-by: Gao Pan <b54642@freescale.com>
> Signed-off-by: Fugang Duan <B38611@freescale.com>

Makes sense, but I can't apply this one. What is it based on?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [Patch V1] i2c: imx: init bus recovery info before adding i2c adapter

[Gao Pandy]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

Hi Wolfram, 

	Sorry for it. 
	
	I have sent out an i2c runtime patch which was acked by Uwe Kleine. I thought it would be accepted, so I sent out this one based on that one. 


Best Regards
Gao Pan  

> -----Original Message-----
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Sent: Sunday, December 06, 2015 3:08 AM
> To: Gao Pan-B54642
> Cc: u.kleine-koenig@pengutronix.de; linux-i2c@vger.kernel.org; Li Frank-B20596;
> Duan Fugang-B38611; kernel@pengutronix.de
> Subject: Re: [Patch V1] i2c: imx: init bus recovery info before adding i2c
> adapter
> 
> On Fri, Dec 04, 2015 at 05:33:14PM +0800, Gao Pan wrote:
> > During driver probe, i2c_imx_init_recovery_info() must come before
> > i2c_add_numbered_adapter(), because the get/set_scl() functions are
> > assigned in i2c_register_adapter() under the conditon that bus
> > recover_info are initialized. Otherwise, get/set_scl() function
> > pointers never get assigned.
> >
> > In such case, when i2c_generic_gpio_recovery() is used for bus
> > recovery, there will be kernel crash because bri->set_scl is NULL.
> >
> > The solution to this bug is moving i2c_imx_init_recovery_info() before
> > i2c_register_adapter().
> >
> > Signed-off-by: Gao Pan <b54642@freescale.com>
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> 
> Makes sense, but I can't apply this one. What is it based on?


[-- Attachment #2: Type: message/rfc822, Size: 3369 bytes --]

From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Gao Pandy <gaopan@freescale.com>
Cc: "wsa@the-dreams.de" <wsa@the-dreams.de>, Li Frank <Frank.Li@freescale.com>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, "kernel@pengutronix.de" <kernel@pengutronix.de>, Duan Andy <fugang.duan@freescale.com>, "hkallweit1@gmail.com" <hkallweit1@gmail.com>
Subject: Re: [Patch V9] i2c: imx: add runtime pm support to improve the performance
Date: Mon, 30 Nov 2015 08:06:30 +0000
Message-ID: <20151130080630.GG10431@pengutronix.de>

Hello,

On Mon, Nov 30, 2015 at 03:42:15AM +0000, Gao Pandy wrote:
> Ping...

The things I pointed out were fixed. I didn't test it, but the patch
looks reasonable. I'd say it's ready to cook in next until the merge
window opens next time.

If you want to interpret this as an Ack, feel free to do this.

Best regards
Uwe

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

Re: [Patch V1] i2c: imx: init bus recovery info before adding i2c adapter

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

> 	I have sent out an i2c runtime patch which was acked by Uwe Kleine. I thought it would be accepted, so I sent out this one based on that one. 

Ah, I see. That is okay to assume. However, this patch is 4.4 material,
while the other patch is 4.5 material, so I needed that rebase.

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]