RE: [PATCH v3 9/9] i2c: designware: Add support for AMDI0020 ACPI ID

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Limonciello, Mario
> Sent: Monday, March 6, 2023 07:12
> To: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>; Jarkko Nikula
> <jarkko.nikula@xxxxxxxxxxxxxxx>
> Cc: Jan Dąbroś <jsd@xxxxxxxxxxxx>; Grzegorz Bernacki
> <gjb@xxxxxxxxxxxx>; Thomas, Rijo-john <Rijo-john.Thomas@xxxxxxx>;
> Lendacky, Thomas <Thomas.Lendacky@xxxxxxx>;
> herbert@xxxxxxxxxxxxxxxxxxx; Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx>; linux-i2c@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 9/9] i2c: designware: Add support for AMDI0020 ACPI
> ID
> 
> On 3/6/23 06:55, Andy Shevchenko wrote:
> > On Mon, Mar 06, 2023 at 02:28:05PM +0200, Jarkko Nikula wrote:
> >> On 3/6/23 14:04, Andy Shevchenko wrote:
> >>> On Fri, Mar 03, 2023 at 10:50:47AM -0600, Mario Limonciello wrote:
> >>>> Cezanne and Skyrim have the same PSP hardware but use a different
> >>>> protocol to negotiate I2C arbitration. To disambiguate this going
> >>>> forward introduce a new ACPI ID to represent the protocol that utilizes
> >>>> a doorbell.
> >
> > ...
> >
> >>>> -	if (boot_cpu_data.x86 == 25 && boot_cpu_data.x86_model == 80)
> >>>
> >>> Ah, in this form it's getting better than I thought!
> >>>
> >> These removed lines were added by previous patch. I think a bit too short
> >> lived if the same patchset adds and then removes lines?
> >
> > That what I have missed. Okay, coming to square 1, i.e. dropping CPU ID
> > completely from the series.
> >
> > Note, for testing purposes you may always add a HACK patch at the end of
> the
> > series, marking it respectively. So, people may test it all and maintainer
> > apply w/o unneeded tail.
> >
> 
> If it still works then new ID can be reserved and patches 8 and 9 could
> be squashed together either by subsystem maintainer when merging or for
> v4.  My apologies if this wasn't obvious to reviewers.  My goal was to
> separate the scalability and functionality for test purposes.
> 
> The way I did it was the series could be tested with patches 1-8 on both
> Cezanne and Skyrim platforms and no BIOS changes.  If it works, BIOS for
> Skyrim can be patched and patch 9 could be added to test kernel.

I've found that AMDI0020 is already reserved and also in use for a while.
f5eda99ee6c0c ("ACPI / APD: Add device HID for future AMD UART controller")

Even if patches 1-9 all work with a patched BIOS to advertise AMDI0020 instead
of AMDI0019, besides squashing patch 8 and 9 will need to discuss what ID to
use.

For this reason, I would suggest that if 1-8 work and there is agreement on them
then merge 1-8 and patch 9 can be a later follow up if/after that discussion and
alignment with stakeholders.