* [PATCH] i2c/synquacer: Deal with optional PCLK correctly
@ 2024-09-12 10:46 Ard Biesheuvel
2024-09-12 17:10 ` Marion & Christophe JAILLET
2024-09-20 9:03 ` Andi Shyti
0 siblings, 2 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2024-09-12 10:46 UTC (permalink / raw)
To: linux-i2c; +Cc: Ard Biesheuvel, stable, Andi Shyti, Christophe JAILLET
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-nw@mail.gmail.com/T/#u
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)
--
2.46.0.662.g92d0881bb0-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly
2024-09-12 10:46 [PATCH] i2c/synquacer: Deal with optional PCLK correctly Ard Biesheuvel
@ 2024-09-12 17:10 ` Marion & Christophe JAILLET
2024-09-12 17:12 ` Ard Biesheuvel
2024-09-20 9:03 ` Andi Shyti
1 sibling, 1 reply; 6+ messages in thread
From: Marion & Christophe JAILLET @ 2024-09-12 17:10 UTC (permalink / raw)
To: Ard Biesheuvel, linux-i2c
Cc: Ard Biesheuvel, stable, Andi Shyti, Andy Shevchenko, Wolfram Sang,
linux-kernel@vger.kernel.org, Kernel Janitors
(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-nw@mail.gmail.com/T/#u
>
> 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)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly
2024-09-12 17:10 ` Marion & Christophe JAILLET
@ 2024-09-12 17:12 ` Ard Biesheuvel
2024-09-12 17:37 ` Christophe JAILLET
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2024-09-12 17:12 UTC (permalink / raw)
To: Marion & Christophe JAILLET
Cc: Ard Biesheuvel, linux-i2c, stable, Andi Shyti, Andy Shevchenko,
Wolfram Sang, linux-kernel@vger.kernel.org, Kernel Janitors
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.
--
Ard.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly
2024-09-12 17:12 ` Ard Biesheuvel
@ 2024-09-12 17:37 ` Christophe JAILLET
0 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2024-09-12 17:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ard Biesheuvel, linux-i2c, stable, Andi Shyti, Andy Shevchenko,
Wolfram Sang, linux-kernel@vger.kernel.org, Kernel Janitors
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.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly
2024-09-12 10:46 [PATCH] i2c/synquacer: Deal with optional PCLK correctly Ard Biesheuvel
2024-09-12 17:10 ` Marion & Christophe JAILLET
@ 2024-09-20 9:03 ` Andi Shyti
2024-09-23 15:10 ` Ard Biesheuvel
1 sibling, 1 reply; 6+ messages in thread
From: Andi Shyti @ 2024-09-20 9:03 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-i2c, Ard Biesheuvel, stable, Christophe JAILLET
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly
2024-09-20 9:03 ` Andi Shyti
@ 2024-09-23 15:10 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2024-09-23 15:10 UTC (permalink / raw)
To: Andi Shyti; +Cc: Ard Biesheuvel, linux-i2c, stable, Christophe JAILLET
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-23 15:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-12 10:46 [PATCH] i2c/synquacer: Deal with optional PCLK correctly Ard Biesheuvel
2024-09-12 17:10 ` Marion & Christophe JAILLET
2024-09-12 17:12 ` Ard Biesheuvel
2024-09-12 17:37 ` Christophe JAILLET
2024-09-20 9:03 ` Andi Shyti
2024-09-23 15:10 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).