Mendocino and later platform don't use the platform feature mailbox for
communication for I2C arbitration, they rely upon ringing a doorbell.
Detect the platform by the device ID of the root port and choose the
appropriate method.
Link: https://lore.kernel.org/linux-i2c/20220916131854.687371-3-jsd@semihalf.com/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v5->v6:
* Handle Mendocino busy code like Cezanne
v4->v5:
* Poll for busy
* Rename to mendocino
* Add explicit dependency on PCI
v3->v4:
* Adjust to use PCI device ID and function pointers instead
v2->v3:
* Use CPU ID rather than ACPI ID, this will be pushed to a later patch
v1->v2:
* New patch
---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-designware-amdpsp.c | 25 +++++++++++++++++++++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d53bf716f97d..2aba5ffa8b03 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -568,6 +568,7 @@ config I2C_DESIGNWARE_AMDPSP
bool "AMD PSP I2C semaphore support"
depends on ACPI
depends on I2C_DESIGNWARE_PLATFORM
+ depends on PCI
imply CRYPTO_DEV_SP_PSP
help
This driver enables managed host access to the selected I2C bus shared
diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
index 12870dc44bdb..f5c754919fbd 100644
--- a/drivers/i2c/busses/i2c-designware-amdpsp.c
+++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/i2c.h>
+#include <linux/pci.h>
#include <linux/psp-platform-access.h>
#include <linux/psp.h>
#include <linux/workqueue.h>
@@ -32,6 +33,8 @@ static u32 psp_i2c_access_count;
static bool psp_i2c_mbox_fail;
static struct device *psp_i2c_dev;
+static int (*_psp_send_i2c_req)(struct psp_i2c_req *req);
+
/* Helper to verify status returned by PSP */
static int check_i2c_req_sts(struct psp_i2c_req *req)
{
@@ -72,6 +75,17 @@ static int psp_send_i2c_req_cezanne(struct psp_i2c_req *req)
return ret;
}
+static int psp_send_i2c_req_mendocino(struct psp_i2c_req *req)
+{
+ int ret;
+
+ ret = psp_ring_platform_doorbell(req->type, &req->hdr.status);
+ if (ret == -EIO)
+ return check_i2c_req_sts(req);
+
+ return ret;
+}
+
static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
{
struct psp_i2c_req *req;
@@ -87,7 +101,7 @@ static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
req->type = i2c_req_type;
start = jiffies;
- ret = read_poll_timeout(psp_send_i2c_req_cezanne, status,
+ ret = read_poll_timeout(_psp_send_i2c_req, status,
(status != -EBUSY),
PSP_I2C_REQ_RETRY_DELAY_US,
PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US,
@@ -262,6 +276,8 @@ static const struct i2c_lock_operations i2c_dw_psp_lock_ops = {
int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
{
+ struct pci_dev *rdev;
+
if (!IS_REACHABLE(CONFIG_CRYPTO_DEV_CCP_DD))
return -ENODEV;
@@ -275,6 +291,13 @@ int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
if (psp_i2c_dev)
return -EEXIST;
+ /* Cezanne uses platform mailbox, Mendocino and later use doorbell */
+ rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
+ if (rdev->device == 0x1630)
+ _psp_send_i2c_req = psp_send_i2c_req_cezanne;
+ else
+ _psp_send_i2c_req = psp_send_i2c_req_mendocino;
+
if (psp_check_platform_access_status())
return -EPROBE_DEFER;
--
2.34.1> static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
> {
> struct psp_i2c_req *req;
> @@ -87,7 +101,7 @@ static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
> req->type = i2c_req_type;
>
> start = jiffies;
> - ret = read_poll_timeout(psp_send_i2c_req_cezanne, status,
> + ret = read_poll_timeout(_psp_send_i2c_req, status,
> (status != -EBUSY),
> PSP_I2C_REQ_RETRY_DELAY_US,
> PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US,
The timeout error handling message after this has "acquire" and
"release" flopped.
> @@ -275,6 +291,13 @@ int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
> if (psp_i2c_dev)
> return -EEXIST;
>
> + /* Cezanne uses platform mailbox, Mendocino and later use doorbell */
> + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> + if (rdev->device == 0x1630)
> + _psp_send_i2c_req = psp_send_i2c_req_cezanne;
> + else
> + _psp_send_i2c_req = psp_send_i2c_req_mendocino;
Thinking about naming again, perhaps "mendocino" should be dropped
from the function name as the logic applies to all platforms except
cezanne.
[Public]
> > static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
> > {
> > struct psp_i2c_req *req;
> > @@ -87,7 +101,7 @@ static int psp_send_i2c_req(enum psp_i2c_req_type
> i2c_req_type)
> > req->type = i2c_req_type;
> >
> > start = jiffies;
> > - ret = read_poll_timeout(psp_send_i2c_req_cezanne, status,
> > + ret = read_poll_timeout(_psp_send_i2c_req, status,
> > (status != -EBUSY),
> > PSP_I2C_REQ_RETRY_DELAY_US,
> > PSP_I2C_REQ_RETRY_CNT *
> PSP_I2C_REQ_RETRY_DELAY_US,
> The timeout error handling message after this has "acquire" and
> "release" flopped.
>
Thx, will fix.
> > @@ -275,6 +291,13 @@ int i2c_dw_amdpsp_probe_lock_support(struct
> dw_i2c_dev *dev)
> > if (psp_i2c_dev)
> > return -EEXIST;
> >
> > + /* Cezanne uses platform mailbox, Mendocino and later use doorbell
> */
> > + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> > + if (rdev->device == 0x1630)
> > + _psp_send_i2c_req = psp_send_i2c_req_cezanne;
> > + else
> > + _psp_send_i2c_req = psp_send_i2c_req_mendocino;
> Thinking about naming again, perhaps "mendocino" should be dropped
> from the function name as the logic applies to all platforms except
> cezanne.
OK.
Hi Mario,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on e6af5c0c4d32a27e04a56f29aad587e03ff427f1]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/crypto-ccp-Bump-up-doorbell-debug-messages-to-error/20230323-050710
base: e6af5c0c4d32a27e04a56f29aad587e03ff427f1
patch link: https://lore.kernel.org/r/20230322210227.464-5-mario.limonciello%40amd.com
patch subject: [PATCH v6 4/4] i2c: designware: Add doorbell support for Mendocino
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230325/202303250445.xSF29gIw-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/9056f37ee3c0bd46052df6b3fb08c0ad951752a4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Mario-Limonciello/crypto-ccp-Bump-up-doorbell-debug-messages-to-error/20230323-050710
git checkout 9056f37ee3c0bd46052df6b3fb08c0ad951752a4
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303250445.xSF29gIw-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/i2c/busses/i2c-designware-amdpsp.o: in function `psp_send_i2c_req_mendocino':
>> i2c-designware-amdpsp.c:(.text+0x12): undefined reference to `psp_ring_platform_doorbell'
ld: drivers/i2c/busses/i2c-designware-amdpsp.o: in function `psp_send_i2c_req_cezanne':
i2c-designware-amdpsp.c:(.text+0x67): undefined reference to `psp_send_platform_access_msg'
ld: drivers/i2c/busses/i2c-designware-amdpsp.o: in function `i2c_dw_amdpsp_probe_lock_support':
i2c-designware-amdpsp.c:(.text+0x497): undefined reference to `psp_check_platform_access_status'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Wed, Mar 22, 2023 at 04:02:26PM -0500, Mario Limonciello wrote: > Mendocino and later platform don't use the platform feature mailbox for > communication for I2C arbitration, they rely upon ringing a doorbell. > > Detect the platform by the device ID of the root port and choose the > appropriate method. ... > - ret = read_poll_timeout(psp_send_i2c_req_cezanne, status, > + ret = read_poll_timeout(_psp_send_i2c_req, status, > (status != -EBUSY), You can place it now in the above line, but up to you. > PSP_I2C_REQ_RETRY_DELAY_US, > PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US, ... > + /* Cezanne uses platform mailbox, Mendocino and later use doorbell */ > + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); > + if (rdev->device == 0x1630) > + _psp_send_i2c_req = psp_send_i2c_req_cezanne; > + else > + _psp_send_i2c_req = psp_send_i2c_req_mendocino; Where is pci_dev_put()? -- With Best Regards, Andy Shevchenko
[Public] > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: Thursday, March 23, 2023 08:06 > To: Limonciello, Mario <Mario.Limonciello@amd.com> > Cc: Jan Dąbroś <jsd@semihalf.com>; Grzegorz Bernacki > <gjb@semihalf.com>; Mark Hasemeyer <markhas@chromium.org>; Jarkko > Nikula <jarkko.nikula@linux.intel.com>; Mika Westerberg > <mika.westerberg@linux.intel.com>; linux-i2c@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v6 4/4] i2c: designware: Add doorbell support for > Mendocino > > On Wed, Mar 22, 2023 at 04:02:26PM -0500, Mario Limonciello wrote: > > Mendocino and later platform don't use the platform feature mailbox for > > communication for I2C arbitration, they rely upon ringing a doorbell. > > > > Detect the platform by the device ID of the root port and choose the > > appropriate method. > > ... > > > - ret = read_poll_timeout(psp_send_i2c_req_cezanne, status, > > + ret = read_poll_timeout(_psp_send_i2c_req, status, > > (status != -EBUSY), > > You can place it now in the above line, but up to you. > > > PSP_I2C_REQ_RETRY_DELAY_US, > > PSP_I2C_REQ_RETRY_CNT * > PSP_I2C_REQ_RETRY_DELAY_US, > > ... > > > + /* Cezanne uses platform mailbox, Mendocino and later use doorbell > */ > > + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); > > + if (rdev->device == 0x1630) > > + _psp_send_i2c_req = psp_send_i2c_req_cezanne; > > + else > > + _psp_send_i2c_req = psp_send_i2c_req_mendocino; > > Where is pci_dev_put()? Missing, thanks for catching it!
© 2016 - 2026 Red Hat, Inc.