Re: [PATCH v3 14/33] serial-mm: add "regshift" property

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 20 Nov 2019 at 07:54, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Mon, Nov 18, 2019 at 6:54 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Wed, 23 Oct 2019 at 18:34, Marc-André Lureau
> > <marcandre.lureau@redhat.com> wrote:
> > > +static Property serial_mm_properties[] = {
> > > +    DEFINE_PROP_UINT8("regshift", SerialMM, regshift, 0),
> >
> > This could use a comment describing what the property does.
>
> "Set the register shift value"? Half-kidding, do you have better
> comment to make? Otherwise, it's probably not worth.
>
> From what I understand, it's just applying a shift on the IO addr,
> probably for alignment/access arch-specific reasons. You probably know
> better than me ;)

What this is doing is defining the spacing between adjacent
registers. Some MMIO-based 16550 implementations have the registers
at byte offsets from each other (that's regshift 0). Some have
them at halfword offsets (regshift 1); and some use 4-byte
offsets (regshift 2). Something like this will do:

 /*
  * Set the spacing between adjacent memory-mapped UART registers.
  * Each register will be at (1 << regshift) bytes after the previous one.
  */

(basically the comment bridges the gap between what I know as
somebody trying to use the 16550 model, ie the behaviour of
the hardware I'm trying to model, and what I need to do to configure
the QEMU code to give that behaviour.)

thanks
-- PMM