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

On 05 Apr 14:16, Adrian McMenamin wrote:
> 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

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