Date:   Mon, 6 Mar 2023 13:37:33 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     "Limonciello, Mario" <Mario.Limonciello@amd.com>
Cc:     Grzegorz Bernacki <gjb@semihalf.com>,
        Jan =?utf-8?B?RMSFYnJvxZs=?= <jsd@semihalf.com>,
        "Thomas, Rijo-john" <Rijo-john.Thomas@amd.com>,
        "Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        Jarkko Nikula <jarkko.nikula@linux.intel.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 8/9] i2c: designware: Add doorbell support for Skyrim
Message-ID: <ZAXQffqGWJLm8vFQ@smile.fi.intel.com>
References: <20230302194235.1724-1-mario.limonciello@amd.com>
 <20230302194235.1724-9-mario.limonciello@amd.com>
 <CAA2Cew5YyufrBZqAA4A5R=1vf_dn=c3ftwziTjzFSm8S5LJZGg@mail.gmail.com>
 <MN0PR12MB61010A9EEBEC94330A7E9702E2B39@MN0PR12MB6101.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <MN0PR12MB61010A9EEBEC94330A7E9702E2B39@MN0PR12MB6101.namprd12.prod.outlook.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Fri, Mar 03, 2023 at 03:00:55PM +0000, Limonciello, Mario wrote:
> > -----Original Message-----
> > From: Grzegorz Bernacki <gjb@semihalf.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