Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] platform/x86: apple-gmux: missing unlock on timeout in gmux_mmio_read8/32() - [High] platform/x86: apple-gmux: uninitialized return in gmux_mmio_read8/32() --
commit b2c93280b42eef5a3e3e707b2033f83778f7828f Author: Atharva Tiwari <[email protected]> platform/apple-gmux: Fix MMIO interface This patch updates the apple-gmux MMIO helpers to sleep for 1ms per poll iteration in gmux_mmio_wait(), check for wait timeouts before performing MMIO reads and writes, and wait before writing in gmux_mmio_write32(). > diff --git a/drivers/platform/x86/apple-gmux.c > b/drivers/platform/x86/apple-gmux.c > index 4e8786a5e58ec..076ed7ba6af90 100644 > --- a/drivers/platform/x86/apple-gmux.c > +++ b/drivers/platform/x86/apple-gmux.c [ ... ] > @@ -250,12 +250,14 @@ static u8 gmux_mmio_read8(struct apple_gmux_data > *gmux_data, int port) > u8 val; > > mutex_lock(&gmux_data->index_lock); > - gmux_mmio_wait(gmux_data); > - iowrite8((port & 0xff), gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT); > - iowrite8(GMUX_MMIO_READ | sizeof(val), > - gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND); > - gmux_mmio_wait(gmux_data); > - val = ioread8(gmux_data->iomem_base); > + if (gmux_mmio_wait(gmux_data)) { > + iowrite8((port & 0xff), gmux_data->iomem_base + > GMUX_MMIO_PORT_SELECT); > + iowrite8(GMUX_MMIO_READ | sizeof(val), > + gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND); > + if (!gmux_mmio_wait(gmux_data)) > + return 0; [Severity: High] Does this early return in gmux_mmio_read8() in drivers/platform/x86/apple-gmux.c leave gmux_data->index_lock locked when the second gmux_mmio_wait(gmux_data) call times out and returns 0? The same early return while holding gmux_data->index_lock also appears in gmux_mmio_read32() below, which will cause subsequent GMIO accesses to deadlock on mutex_lock(&gmux_data->index_lock). > + val = ioread8(gmux_data->iomem_base); > + } > mutex_unlock(&gmux_data->index_lock); > > return val; > } [ ... ] > @@ -281,12 +286,16 @@ static u32 gmux_mmio_read32(struct apple_gmux_data > *gmux_data, int port) > u32 val; > > mutex_lock(&gmux_data->index_lock); > - gmux_mmio_wait(gmux_data); > - iowrite8((port & 0xff), gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT); > - iowrite8(GMUX_MMIO_READ | sizeof(val), > - gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND); > - gmux_mmio_wait(gmux_data); > - val = ioread32be(gmux_data->iomem_base); > + > + if (gmux_mmio_wait(gmux_data)) { > + iowrite8((port & 0xff), gmux_data->iomem_base + > GMUX_MMIO_PORT_SELECT); > + iowrite8(GMUX_MMIO_READ | sizeof(val), > + gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND); > + if (!gmux_mmio_wait(gmux_data)) > + return 0; > + > + val = ioread32be(gmux_data->iomem_base); > + } > mutex_unlock(&gmux_data->index_lock); > > return val; > } [Severity: High] Can gmux_mmio_read32() and gmux_mmio_read8() in drivers/platform/x86/apple-gmux.c return an uninitialized val if the initial gmux_mmio_wait(gmux_data) call times out and returns 0? In both functions, val is declared without an initializer and is only assigned inside the if (gmux_mmio_wait(gmux_data)) block before being returned after mutex_unlock(&gmux_data->index_lock). Could val be initialized to 0 (and returned after unlocking on both timeout paths)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
