[PATCH linux 2/2] drivers/misc: Add Aspeed LPC mbox register driver

[Andrew Jeffery]

> > +
> > +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
> > +{
> > +       /*
> > +        * The mbox registers are actually only one byte but are addressed
> > +        * four bytes apart. The other three bytes are marked 'reserved',
> > +        * they *should* be zero but lets not rely on it.
> > +        * I am going to rely on the fact we can casually read/write to them...
> > +        */
> > +       unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> > +       int rc = regmap_read(mbox->regmap, mbox->base + reg, &val);
> > +
> > +       if (rc)
> > +               dev_err(mbox->miscdev.parent,
> > +                       "regmap_read() failed with %d (reg: 0x%08x)\n",
> > +                       rc, reg);
> > +
> > +       return val & 0xff;
> > +}
> 
> This is a mess. I'd prefer:
> 
> static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
> {
>        int val;
> 
>        regmap_read(mbox->regmap, mbox->base + reg, &val);
> 
>        return val;
> }
> 
> static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int reg)
> {
>        regmap_write(mbox->regmap, mbox->base + reg, data);
> }
> 
> Reads and writes to a mmio address will never fail. Regmap incurs
> enough overhead already; we don't need to add more in the driver.
> 

The mess was my suggestion when the patches were first posted, and I apologise for that now. Lets go with Joel's cleaned up version.

Cheers,

Andrew