While I had thought I had fixed this issue in:
commit 342406e4fb
("drm/nouveau/i2c: Disable i2c bus access after
->fini()")
It turns out that while I did fix the error messages I was seeing on my
P50 when trying to access i2c busses with the GPU in runtime suspend, I
accidentally had missed one important detail that was mentioned on the
bug report this commit was supposed to fix: that the CPU would only lock
up when trying to access i2c busses _on connected devices_ _while the
GPU is not in runtime suspend_. Whoops. That definitely explains why I
was not able to get my machine to hang with i2c bus interactions until
now, as plugging my P50 into it's dock with an HDMI monitor connected
allowed me to finally reproduce this locally.
Now that I have managed to reproduce this issue properly, it looks like
the problem is much simpler then it looks. It turns out that some
connected devices, such as MST laptop docks, will actually ACK i2c reads
even if no data was actually read:
[ 275.063043] nouveau 0000:01:00.0: i2c: aux 000a: 1: 0000004c 1
[ 275.063447] nouveau 0000:01:00.0: i2c: aux 000a: 00 01101000 10040000
[ 275.063759] nouveau 0000:01:00.0: i2c: aux 000a: rd 00000001
[ 275.064024] nouveau 0000:01:00.0: i2c: aux 000a: rd 00000000
[ 275.064285] nouveau 0000:01:00.0: i2c: aux 000a: rd 00000000
[ 275.064594] nouveau 0000:01:00.0: i2c: aux 000a: rd 00000000
Because we don't handle the situation of i2c ack without any data, we
end up entering an infinite loop in nvkm_i2c_aux_i2c_xfer() since the
value of cnt always remains at 0. This finally properly explains how
this could result in a CPU hang like the ones observed in the
aforementioned commit.
So, fix this by retrying transactions if no data is written or received,
and give up and fail the transaction if we continue to not write or
receive any data after 32 retries.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
212 lines
4.9 KiB
C
212 lines
4.9 KiB
C
/*
|
|
* Copyright 2009 Red Hat Inc.
|
|
*
|
|
* Permission is hereby granted, free of charge, to any person obtaining a
|
|
* copy of this software and associated documentation files (the "Software"),
|
|
* to deal in the Software without restriction, including without limitation
|
|
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
|
|
* and/or sell copies of the Software, and to permit persons to whom the
|
|
* Software is furnished to do so, subject to the following conditions:
|
|
*
|
|
* The above copyright notice and this permission notice shall be included in
|
|
* all copies or substantial portions of the Software.
|
|
*
|
|
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
|
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
|
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
|
|
* THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
|
|
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
|
|
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
|
|
* OTHER DEALINGS IN THE SOFTWARE.
|
|
*
|
|
* Authors: Ben Skeggs
|
|
*/
|
|
#include "aux.h"
|
|
#include "pad.h"
|
|
|
|
static int
|
|
nvkm_i2c_aux_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
|
|
{
|
|
struct nvkm_i2c_aux *aux = container_of(adap, typeof(*aux), i2c);
|
|
struct i2c_msg *msg = msgs;
|
|
int ret, mcnt = num;
|
|
|
|
ret = nvkm_i2c_aux_acquire(aux);
|
|
if (ret)
|
|
return ret;
|
|
|
|
while (mcnt--) {
|
|
u8 remaining = msg->len;
|
|
u8 *ptr = msg->buf;
|
|
|
|
while (remaining) {
|
|
u8 cnt, retries, cmd;
|
|
|
|
if (msg->flags & I2C_M_RD)
|
|
cmd = 1;
|
|
else
|
|
cmd = 0;
|
|
|
|
if (mcnt || remaining > 16)
|
|
cmd |= 4; /* MOT */
|
|
|
|
for (retries = 0, cnt = 0;
|
|
retries < 32 && !cnt;
|
|
retries++) {
|
|
cnt = min_t(u8, remaining, 16);
|
|
ret = aux->func->xfer(aux, true, cmd,
|
|
msg->addr, ptr, &cnt);
|
|
if (ret < 0)
|
|
goto out;
|
|
}
|
|
if (!cnt) {
|
|
AUX_TRACE(aux, "no data after 32 retries");
|
|
ret = -EIO;
|
|
goto out;
|
|
}
|
|
|
|
ptr += cnt;
|
|
remaining -= cnt;
|
|
}
|
|
|
|
msg++;
|
|
}
|
|
|
|
ret = num;
|
|
out:
|
|
nvkm_i2c_aux_release(aux);
|
|
return ret;
|
|
}
|
|
|
|
static u32
|
|
nvkm_i2c_aux_i2c_func(struct i2c_adapter *adap)
|
|
{
|
|
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
|
|
}
|
|
|
|
static const struct i2c_algorithm
|
|
nvkm_i2c_aux_i2c_algo = {
|
|
.master_xfer = nvkm_i2c_aux_i2c_xfer,
|
|
.functionality = nvkm_i2c_aux_i2c_func
|
|
};
|
|
|
|
void
|
|
nvkm_i2c_aux_monitor(struct nvkm_i2c_aux *aux, bool monitor)
|
|
{
|
|
struct nvkm_i2c_pad *pad = aux->pad;
|
|
AUX_TRACE(aux, "monitor: %s", monitor ? "yes" : "no");
|
|
if (monitor)
|
|
nvkm_i2c_pad_mode(pad, NVKM_I2C_PAD_AUX);
|
|
else
|
|
nvkm_i2c_pad_mode(pad, NVKM_I2C_PAD_OFF);
|
|
}
|
|
|
|
void
|
|
nvkm_i2c_aux_release(struct nvkm_i2c_aux *aux)
|
|
{
|
|
struct nvkm_i2c_pad *pad = aux->pad;
|
|
AUX_TRACE(aux, "release");
|
|
nvkm_i2c_pad_release(pad);
|
|
mutex_unlock(&aux->mutex);
|
|
}
|
|
|
|
int
|
|
nvkm_i2c_aux_acquire(struct nvkm_i2c_aux *aux)
|
|
{
|
|
struct nvkm_i2c_pad *pad = aux->pad;
|
|
int ret;
|
|
|
|
AUX_TRACE(aux, "acquire");
|
|
mutex_lock(&aux->mutex);
|
|
|
|
if (aux->enabled)
|
|
ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_AUX);
|
|
else
|
|
ret = -EIO;
|
|
|
|
if (ret)
|
|
mutex_unlock(&aux->mutex);
|
|
return ret;
|
|
}
|
|
|
|
int
|
|
nvkm_i2c_aux_xfer(struct nvkm_i2c_aux *aux, bool retry, u8 type,
|
|
u32 addr, u8 *data, u8 *size)
|
|
{
|
|
if (!*size && !aux->func->address_only) {
|
|
AUX_ERR(aux, "address-only transaction dropped");
|
|
return -ENOSYS;
|
|
}
|
|
return aux->func->xfer(aux, retry, type, addr, data, size);
|
|
}
|
|
|
|
int
|
|
nvkm_i2c_aux_lnk_ctl(struct nvkm_i2c_aux *aux, int nr, int bw, bool ef)
|
|
{
|
|
if (aux->func->lnk_ctl)
|
|
return aux->func->lnk_ctl(aux, nr, bw, ef);
|
|
return -ENODEV;
|
|
}
|
|
|
|
void
|
|
nvkm_i2c_aux_del(struct nvkm_i2c_aux **paux)
|
|
{
|
|
struct nvkm_i2c_aux *aux = *paux;
|
|
if (aux && !WARN_ON(!aux->func)) {
|
|
AUX_TRACE(aux, "dtor");
|
|
list_del(&aux->head);
|
|
i2c_del_adapter(&aux->i2c);
|
|
kfree(*paux);
|
|
*paux = NULL;
|
|
}
|
|
}
|
|
|
|
void
|
|
nvkm_i2c_aux_init(struct nvkm_i2c_aux *aux)
|
|
{
|
|
AUX_TRACE(aux, "init");
|
|
mutex_lock(&aux->mutex);
|
|
aux->enabled = true;
|
|
mutex_unlock(&aux->mutex);
|
|
}
|
|
|
|
void
|
|
nvkm_i2c_aux_fini(struct nvkm_i2c_aux *aux)
|
|
{
|
|
AUX_TRACE(aux, "fini");
|
|
mutex_lock(&aux->mutex);
|
|
aux->enabled = false;
|
|
mutex_unlock(&aux->mutex);
|
|
}
|
|
|
|
int
|
|
nvkm_i2c_aux_ctor(const struct nvkm_i2c_aux_func *func,
|
|
struct nvkm_i2c_pad *pad, int id,
|
|
struct nvkm_i2c_aux *aux)
|
|
{
|
|
struct nvkm_device *device = pad->i2c->subdev.device;
|
|
|
|
aux->func = func;
|
|
aux->pad = pad;
|
|
aux->id = id;
|
|
mutex_init(&aux->mutex);
|
|
list_add_tail(&aux->head, &pad->i2c->aux);
|
|
AUX_TRACE(aux, "ctor");
|
|
|
|
snprintf(aux->i2c.name, sizeof(aux->i2c.name), "nvkm-%s-aux-%04x",
|
|
dev_name(device->dev), id);
|
|
aux->i2c.owner = THIS_MODULE;
|
|
aux->i2c.dev.parent = device->dev;
|
|
aux->i2c.algo = &nvkm_i2c_aux_i2c_algo;
|
|
return i2c_add_adapter(&aux->i2c);
|
|
}
|
|
|
|
int
|
|
nvkm_i2c_aux_new_(const struct nvkm_i2c_aux_func *func,
|
|
struct nvkm_i2c_pad *pad, int id,
|
|
struct nvkm_i2c_aux **paux)
|
|
{
|
|
if (!(*paux = kzalloc(sizeof(**paux), GFP_KERNEL)))
|
|
return -ENOMEM;
|
|
return nvkm_i2c_aux_ctor(func, pad, id, *paux);
|
|
}
|