From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()") Cc: stable@vger.kernel.org Cc: Andi Shyti andi.shyti@kernel.org Cc: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Ard Biesheuvel ardb@kernel.org --- https://lore.kernel.org/all/CAMj1kXFH+zB_YuUS+vaEpguhuVGLYbQw55VNDCxnBfSPe6b...
drivers/i2c/busses/i2c-synquacer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c index 4eccbcd0fbfc..bbb9062669e4 100644 --- a/drivers/i2c/busses/i2c-synquacer.c +++ b/drivers/i2c/busses/i2c-synquacer.c @@ -550,12 +550,13 @@ static int synquacer_i2c_probe(struct platform_device *pdev) device_property_read_u32(&pdev->dev, "socionext,pclk-rate", &i2c->pclkrate);
- pclk = devm_clk_get_enabled(&pdev->dev, "pclk"); + pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk"); if (IS_ERR(pclk)) return dev_err_probe(&pdev->dev, PTR_ERR(pclk), "failed to get and enable clock\n");
- i2c->pclkrate = clk_get_rate(pclk); + if (pclk) + i2c->pclkrate = clk_get_rate(pclk);
if (i2c->pclkrate < SYNQUACER_I2C_MIN_CLK_RATE || i2c->pclkrate > SYNQUACER_I2C_MAX_CLK_RATE)
(trying to merge t and cc fields from Ard's and Andy's messages)
Le 12/09/2024 à 12:46, Ard Biesheuvel a écrit :
From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()")
Hi,
If that matters, I'm not sure that the Fixes tag is correct.
IIUC, either it is a new functionally that is added (now it works with ACPI...), or if considered as a fix, then I think that it is linked to commit 0d676a6c4390 ("i2c: add support for Socionext SynQuacer I2C controller").
I don't think that 55750148e559 introduced a regression. The issue seems to be there since the beginning. Agreed?
If yes, then it may be needed to backport it in older kernels too.
CJ
Cc: stable@vger.kernel.org Cc: Andi Shyti andi.shyti@kernel.org Cc: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Ard Biesheuvel ardb@kernel.org
https://lore.kernel.org/all/CAMj1kXFH+zB_YuUS+vaEpguhuVGLYbQw55VNDCxnBfSPe6b...
drivers/i2c/busses/i2c-synquacer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c index 4eccbcd0fbfc..bbb9062669e4 100644 --- a/drivers/i2c/busses/i2c-synquacer.c +++ b/drivers/i2c/busses/i2c-synquacer.c @@ -550,12 +550,13 @@ static int synquacer_i2c_probe(struct platform_device *pdev) device_property_read_u32(&pdev->dev, "socionext,pclk-rate", &i2c->pclkrate);
- pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
- pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk"); if (IS_ERR(pclk)) return dev_err_probe(&pdev->dev, PTR_ERR(pclk), "failed to get and enable clock\n");
- i2c->pclkrate = clk_get_rate(pclk);
- if (pclk)
i2c->pclkrate = clk_get_rate(pclk);if (i2c->pclkrate < SYNQUACER_I2C_MIN_CLK_RATE || i2c->pclkrate > SYNQUACER_I2C_MAX_CLK_RATE)
On Thu, 12 Sept 2024 at 19:11, Marion & Christophe JAILLET christophe.jaillet@wanadoo.fr wrote:
(trying to merge t and cc fields from Ard's and Andy's messages)
Le 12/09/2024 à 12:46, Ard Biesheuvel a écrit :
From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()")
Hi,
If that matters, I'm not sure that the Fixes tag is correct.
IIUC, either it is a new functionally that is added (now it works with ACPI...), or if considered as a fix, then I think that it is linked to commit 0d676a6c4390 ("i2c: add support for Socionext SynQuacer I2C controller").
I don't think that 55750148e559 introduced a regression. The issue seems to be there since the beginning. Agreed?
No.
The original code used IS_ERR_OR_NULL() to explicitly permit the case where no clock exists at all.
This has worked fine with ACPI boot for many years before this fix was applied.
If yes, then it may be needed to backport it in older kernels too.
No, it used to work. The fix is what broke ACPI boot.
Le 12/09/2024 à 19:12, Ard Biesheuvel a écrit :
On Thu, 12 Sept 2024 at 19:11, Marion & Christophe JAILLET christophe.jaillet@wanadoo.fr wrote:
(trying to merge t and cc fields from Ard's and Andy's messages)
Le 12/09/2024 à 12:46, Ard Biesheuvel a écrit :
From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()")
Hi,
If that matters, I'm not sure that the Fixes tag is correct.
IIUC, either it is a new functionally that is added (now it works with ACPI...), or if considered as a fix, then I think that it is linked to commit 0d676a6c4390 ("i2c: add support for Socionext SynQuacer I2C controller").
I don't think that 55750148e559 introduced a regression. The issue seems to be there since the beginning. Agreed?
No.
The original code used IS_ERR_OR_NULL() to explicitly permit the case where no clock exists at all.
Got it, this is not related to the removed _OR_NULL, but to the change of logic I've introduce.
if (!IS_ERR) do_something_and_continue; if_NOENT_was_returned_we_still_get_there();
which became: if (IS_ERR) return; we_can_get_there_with_NOENT_anymore();
My bad!
CJ
This has worked fine with ACPI boot for many years before this fix was applied.
If yes, then it may be needed to backport it in older kernels too.
No, it used to work. The fix is what broke ACPI boot.
Hi Ard,
On Thu, Sep 12, 2024 at 12:46:31PM GMT, Ard Biesheuvel wrote:
From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()") Cc: stable@vger.kernel.org Cc: Andi Shyti andi.shyti@kernel.org Cc: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Ard Biesheuvel ardb@kernel.org
I'm sorry for the delay, but I needed to wait for the previous batch of fixes to be merged.
Merged to i2c/i2c-host-next.
Thanks, Andi
On Fri, 20 Sept 2024 at 11:03, Andi Shyti andi.shyti@kernel.org wrote:
Hi Ard,
On Thu, Sep 12, 2024 at 12:46:31PM GMT, Ard Biesheuvel wrote:
From: Ard Biesheuvel ardb@kernel.org
ACPI boot does not provide clocks and regulators, but instead, provides the PCLK rate directly, and enables the clock in firmware. So deal gracefully with this.
Fixes: 55750148e559 ("i2c: synquacer: Fix an error handling path in synquacer_i2c_probe()") Cc: stable@vger.kernel.org Cc: Andi Shyti andi.shyti@kernel.org Cc: Christophe JAILLET christophe.jaillet@wanadoo.fr Signed-off-by: Ard Biesheuvel ardb@kernel.org
I'm sorry for the delay, but I needed to wait for the previous batch of fixes to be merged.
Merged to i2c/i2c-host-next.
Thanks. No need to treat it with urgency on my behalf.
linux-stable-mirror@lists.linaro.org
-
Andi Shyti -
Ard Biesheuvel -
Ard Biesheuvel -
Christophe JAILLET -
Marion & Christophe JAILLET