Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 22 Mar 2023 12:12:19 -0500 | Subject | Re: [PATCH v5 3/3] i2c: designware: Add doorbell support for Mendocino | From | Mario Limonciello <> |
| |
On 3/22/23 12:01, Mark Hasemeyer wrote: >> +static int psp_send_i2c_req_mendocino(enum psp_i2c_req_type i2c_req_type) >> +{ >> + int status, ret; >> + >> + ret = read_poll_timeout(psp_ring_platform_doorbell, status, >> + (status != -EBUSY), >> + PSP_I2C_REQ_RETRY_DELAY_US, >> + PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US, >> + 0, i2c_req_type); >> + if (ret) >> + dev_err(psp_i2c_dev, "Timed out waiting for PSP to %s I2C bus\n", >> + (i2c_req_type == PSP_I2C_REQ_ACQUIRE) ? >> + "release" : "acquire"); >> + >> + return ret ? ret : status; >> +} > I think we need the value of the PSP_CMDRESP_STS field returned to the caller > and its status checked like in psp_send_i2c_req_cezanne. Otherwise the function > won't continue to poll when the PSP_I2C_REQ_STS_BUS_BUSY bit is set.
In that case it looks like psp_send_check_i2c_req isn't handling this properly for Cezanne either in this series. I think psp_send_platform_access_msg returning -EIO is going to mean that check_i2c_req_sts never gets run.
So either psp_send_platform_access_msg should return 0 for that case (expecting caller to investigate more closely) or psp_send_check_i2c_req needs to special case -EIO for more investigation.
I lean upon the latter unless you have a strong opinion.
> > FYI - there's a test on ChromeOS to stress test I2C bus arbitration: > https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/tast-tests/src/chromiumos/tast/local/bundles/cros/hwsec/tpm_contest.go > I can try to run it assuming the ToT kernel runs on skyrim.
I would expect ToT should run fine on Skyrim. I'll adjust as you suggested if you can please test it.
| ![\](/images/icornerr.gif) |