Re: [PATCH 1/2] cdrom: gdrom: replace port I/O with MMIO accessors
Adrian McMenamin <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.ports.sh.devel |
|---|---|
| Message-ID | <CACwZE5QeuDnEOGar6-0tfkhO2s8UuvdcjMMf7ubFhVUeh=7JcA@mail.gmail.com> |
On Sun, 5 Apr 2026 at 09:23, Florian Fuchs <[email protected]> wrote: > > GDROM_DATA_REG is a memory-mapped data register, but the driver uses > outsw() and insw() only for this register. Replace this with local > helpers using MMIO accessors ioread16_rep() / iowrite16_rep(). > > Before, it oopsed accessing the data register, as the io_port_base > P2SEG gets added to the argument in outsw() / insw(), which leads to an > unusable drive: > > BUG: unable to handle kernel paging request at 405f7080 > PC: [<8c28d5b4>] gdrom_spicommand+0x6c/0xb0 > > Signed-off-by: Florian Fuchs <[email protected]> > --- > The original Oops can be reproduced just by mounting a disc, like: > mount -t iso9660 -o ro /dev/gdrom /mnt > --- > drivers/cdrom/gdrom.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c > index 4ba4dd06cbf4..dccf41fa5d0a 100644 > --- a/drivers/cdrom/gdrom.c > +++ b/drivers/cdrom/gdrom.c > @@ -171,6 +171,16 @@ static void gdrom_identifydevice(void *buf) > data[c] = __raw_readw(GDROM_DATA_REG); > } > > +static void gdrom_fifo_readw(void *buf, unsigned int words) > +{ > + ioread16_rep((void __iomem *)GDROM_DATA_REG, buf, words); > +} > + > +static void gdrom_fifo_writew(const void *buf, unsigned int words) > +{ > + iowrite16_rep((void __iomem *)GDROM_DATA_REG, buf, words); > +} > + > static void gdrom_spicommand(void *spi_string, int buflen) > { > short *cmd = spi_string; > @@ -198,7 +208,7 @@ static void gdrom_spicommand(void *spi_string, int buflen) > gdrom_getsense(NULL); > return; > } > - outsw(GDROM_DATA_REG, cmd, 6); > + gdrom_fifo_writew(cmd, 6); > } > This is one of those "how did this ever work to begin with" bugs when examined today - bur rather than introduce new local functions can we not just use either readsw(p, d, l)/writesw(p, d, l) or __raw_writew(p,d,l) and __raw_read(p,d,l) directly? Adrian