[PATCH] i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

[PATCH] i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

[Andi Shyti]

Marek hs reported that commit 51130d52b84c ("i2c: exynos5: Init
data before registering interrupt handler") is breaking things.

This is a regression and until we find out what happens this
should be reverted.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/i2c/busses/i2c-exynos5.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 8458e22313a7f..385ef9d9e4d4c 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -906,9 +906,23 @@ static int exynos5_i2c_probe(struct platform_device *pdev)
 	i2c->adap.algo_data = i2c;
 	i2c->adap.dev.parent = &pdev->dev;
 
+	/* Clear pending interrupts from u-boot or misc causes */
+	exynos5_i2c_clr_pend_irq(i2c);
+
 	spin_lock_init(&i2c->lock);
 	init_completion(&i2c->msg_complete);
 
+	i2c->irq = ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		goto err_clk;
+
+	ret = devm_request_irq(&pdev->dev, i2c->irq, exynos5_i2c_irq,
+			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "cannot request HS-I2C IRQ %d\n", i2c->irq);
+		goto err_clk;
+	}
+
 	i2c->variant = of_device_get_match_data(&pdev->dev);
 
 	ret = exynos5_hsi2c_clock_setup(i2c);
@@ -926,21 +940,6 @@ static int exynos5_i2c_probe(struct platform_device *pdev)
 	clk_disable(i2c->clk);
 	clk_disable(i2c->pclk);
 
-	/* Clear pending interrupts from u-boot or misc causes */
-	exynos5_i2c_clr_pend_irq(i2c);
-
-	ret = platform_get_irq(pdev, 0);
-	if (ret < 0)
-		goto err_clk;
-	i2c->irq = ret;
-
-	ret = devm_request_irq(&pdev->dev, i2c->irq, exynos5_i2c_irq,
-			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "cannot request HS-I2C IRQ %d\n", i2c->irq);
-		goto err_clk;
-	}
-
 	return 0;
 
  err_clk:
-- 
2.43.0

Re: [PATCH] i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

[Krzysztof Kozlowski]

On 07/03/2024 15:04, Andi Shyti wrote:
> Marek hs reported that commit 51130d52b84c ("i2c: exynos5: Init
> data before registering interrupt handler") is breaking things.
> 
> This is a regression and until we find out what happens this
> should be reverted.

We know exactly what's wrong. This should have never been applied.

Best regards,
Krzysztof

Re: [PATCH] i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

[Andi Shyti]

Hi Krzysztof,

On Thu, Mar 07, 2024 at 03:05:50PM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2024 15:04, Andi Shyti wrote:
> > Marek hs reported that commit 51130d52b84c ("i2c: exynos5: Init
> > data before registering interrupt handler") is breaking things.
> > 
> > This is a regression and until we find out what happens this
> > should be reverted.
> 
> We know exactly what's wrong. This should have never been applied.

when you wrote me about the regression I hadn't read Marek's mail
yet but I rushed to prepare the revert in order to have the fix
asap first. I'm going to fix the commit quoting Marek:

"
i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

Marek hs reported about commit 51130d52b84c ("i2c: exynos5: Init
data before registering interrupt handler"):

Just above exynos5_i2c_clr_pend_irq() the clocks have been
disabled, so any access to the i2c host registers will result in
freeze or external abort (depending on the soc/cpu).

To make things worse, this patch moved registering the interrupt
handler after the i2c_add_adapter() call. This means that all i2c
devices that will be probbed directly from i2c_add_adapter()
won't be able to access the i2c bus, as the host controller is
still not fully functional that time yet.

This breaks all Exynos5+ platforms.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reported-by: Markus Reichl <m.reichl@fivetechno.de>
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
"

Thank you very much, Marek, for reporting this.

I prefer the revert not to break the i2c/i2c-host-fixes branch,
but if Wlfram prefers, I can remove it from the pull request
which I'm going to send tomorrow in the morning.

Can anyone please ack here?

And... Jesper, how did you test this patch?

Thanks,
Andi

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH] i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler"

[Andi Shyti]

Hi,

...

> Can anyone please ack here?

OK, I have just removed this patch and force pushed the
i2c-host-fixes.

Andi