linux-kernel - Re: [PATCH v2 8/9] i2c: designware: Add doorbell support for Skyrim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZAXQffqGWJLm8vFQ@smile.fi.intel.com>
Date:   Mon, 6 Mar 2023 13:37:33 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     "Limonciello, Mario" <Mario.Limonciello@....com>
Cc:     Grzegorz Bernacki <gjb@...ihalf.com>,
        Jan Dąbroś <jsd@...ihalf.com>,
        "Thomas, Rijo-john" <Rijo-john.Thomas@....com>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 8/9] i2c: designware: Add doorbell support for Skyrim

On Fri, Mar 03, 2023 at 03:00:55PM +0000, Limonciello, Mario wrote:
> > -----Original Message-----
> > From: Grzegorz Bernacki <gjb@...ihalf.com>
> > Sent: Friday, March 3, 2023 06:00

> > I am not sure if adding a new ACPI ID is a good idea. Actually we are
> > talking about the same devices. The only difference is in the
> > communication protocol between PSP and CPU. This could be easily
> > detected at runtime by checking cpu id. There is no need to introduce
> > a new id and create dependency on the FW version.
> 
> An ACPI ID seems more scalable to me to represent the difference in protocol.
> Otherwise what happens when the follow on to Skyrim comes?  Do you add
> a new ID/case to the code?  If it was an ACPI ID then it's a one line change
> in the firmware to represent this.
> 
> What I'll do for v3 is do with a CPU ID in this patch, and then introduce an ACPI
> ID in a new patch.  You can test it without the ACPI ID, and when it's working
> patch the BIOS with the new ID and see if that continues to work.

I don't think we need this complexity.

Please, use just an ACPI ID and that's it. For testing one may apply additional
(HACK) patch(es).

As telling from my experience, to distinguish such things via CPU ID is a bad
idea. Yes, we (used?) to have a PITA (find yourself what this stands for :)
times with the (mixed) approach. So, no, please don't add CPU ID into the I²C
platform driver (keyword: platform).

-- 
With Best Regards,
Andy Shevchenko