| Message ID | 20240307140427.1942235-1-andi.shyti@kernel.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | i2c: exynos5: Revert "i2c: exynos5: Init data before registering interrupt handler" | expand |
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
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 >
Hi,
...
> Can anyone please ack here?
OK, I have just removed this patch and force pushed the
i2c-host-fixes.
Andi
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:
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(-)