Re: [PATCH 1/2] cdrom: gdrom: replace port I/O with MMIO accessors

Florian Fuchs
2026-04-06T09:05:39Z

On 05 Apr 14:16, Adrian McMenamin wrote:
> On Sun, 5 Apr 2026 at 09:23, Florian Fuchs <fuchsfl@gmail.com> 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 <fuchsfl@gmail.com>
> > ---
> > 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

Yeah, totally, I put them inline. I first had a loop with the __raw.*
functions, that made more sense to extract.

Will send it, Thanks!
Florian