Re: [PATCH] i2c/synquacer: Deal with optional PCLK correctly

[Christophe JAILLET]

Le 12/09/2024 à 19:12, Ard Biesheuvel a écrit :
> On Thu, 12 Sept 2024 at 19:11, Marion & Christophe JAILLET
> <christophe.jaillet@xxxxxxxxxx> 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@xxxxxxxxxx>
> > >
> > > 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.