Re: [RFC PATCH 1/1] hw: aspeed_adc: Add initial Aspeed ADC support

[Cédric Le Goater]

> > FYI, these series was sent by Andrew in 2017 and I have been keeping
> > it alive since in the aspeed-x.y branches :
> >
> > * memory: Support unaligned accesses on aligned-only models
> >   
> > https://github.com/legoater/qemu/commit/1960ba6bde27b91edb5336985a9210260a4c8938
> >
> >   That was requested by Phil I think.
> >
> > * hw/adc: Add basic Aspeed ADC model
> >   
> > https://github.com/legoater/qemu/commit/1eff7b1cf10d1777635f7d2cef8ecb441cc607c4
> >
> >   This is the initial patch. I added multi-engine support recently
> >   for the fuji.
> >
> > * hw/arm: Integrate ADC model into Aspeed SoC
> >   
> > https://github.com/legoater/qemu/commit/3052f9d8ccdaf78b753e53574b7e8cc2ee01429f
> >
> >   That one is trivial.
> >
> >
> > Overall comments :
> >
> > I prefer the 'regs' array approach of your proposal.
>
> Ok (I was just following patterns from aspeed_scu.c), I’ll keep that aspect.
>
> > I think the AspeedADCEngine should appear as a QOM object. Check
> > the patches above.
>
> I see, I’ll make sure to test this.

The engines are behind the same BAR and they share the IRQ. So
it makes sense I think. And it shows up nicely under the monitor :

    ...
    000000001e6e7000-000000001e6e7fff (prio 0, i/o): aspeed.xdma
    000000001e6e9000-000000001e6e9fff (prio 0, i/o): aspeed.adc
      000000001e6e9000-000000001e6e90ff (prio 0, i/o): aspeed.adc.engine.0
      000000001e6e9100-000000001e6e91ff (prio 0, i/o): aspeed.adc.engine.1
    000000001e700000-000000001e700fff (prio -1000, i/o): aspeed.video
    ...


    /adc (aspeed.adc-ast2600)
      /aspeed.adc[0] (memory-region)
      /engine[0] (aspeed.adc.engine)
        /aspeed.adc.engine.0[0] (memory-region)
      /engine[1] (aspeed.adc.engine)
        /aspeed.adc.engine.1[0] (memory-region)
      /unnamed-gpio-in[0] (irq)
      /unnamed-gpio-in[1] (irq)


> > To move on, maybe, you could rework the initial series and take
> > ownership ?
>
> Yeah definitely! I’ll resubmit once I’ve reworked it. I don’t intend
> to include the unaligned-access support though, at least not w/ the ADC
> changes.

Yeah. This can come later.

Thanks,

C.