Re: [Qemu-arm] [RFC PATCH] hw: arm: Add basic support for cprman (clock subsystem)

[Guenter Roeck]

On Mon, Jul 16, 2018 at 03:33:15PM +0100, Peter Maydell wrote:
> On 15 July 2018 at 23:06, Guenter Roeck <address@hidden> wrote:
> > Add basic support for BCM283x CPRMAN. Provide support for reading and
> > writing CPRMAN registers and initialize registers with sensible default
> > values. During runtime retain any written values.
> >
> > Basic CPRMAN support is necessary and sufficient to boot Linux on raspi2
> > and raspi3 systems.
> >
> > Signed-off-by: Guenter Roeck <address@hidden>
> > ---
> > I don't seriously expect this patch to get accepted, but I thought
> > it might be valuable enough for others to use it when playing with
> > raspi2 and raspi3 emulations.
> 
> Looking around I guess that a stub version of this device is
> worth having. It's not like we'd be wiring up the clock control
> information to anything anyway, given that QEMU doesn't much
> care about clocks...
> 
> > +#define CM_GNRICCTL        (0x000 / 4)
> > +#define CM_VECCTL        (0x0f8 / 4)
> > +#define CM_DFTCTL        (0x168 / 4)
> > +#define CM_EMMCCTL        (0x1c0 / 4)
> > +#define A2W_PLLA_CTRL        (0x1100 / 4)
> > +#define A2W_PLLB_CTRL        (0x11e0 / 4)
> 
> The information at https://elinux.org/BCM2835_registers#CM
> and https://elinux.org/BCM2835_registers#A2W
> suggests that the A2W is a separate thing from the CPRMAN...
>

Both are used together to define individual clocks in the Linux clock
driver (each clock has a CM register and a matching A2W register), so
they must be associated to each other. It might or might not make sense
to treat them as separate devices in qemu. Hard to say without datasheet.

> > +static void bcm2835_cprman_reset(DeviceState *dev)
> > +{
> > +    BCM2835CprmanState *s = BCM2835_CPRMAN(dev);
> > +    int i;
> > +
> > +    /*
> > +     * Available information suggests that CPRMAN registers have default
> > +     * values which are not overwritten by ROMMON (u-boot). The hardware
> > +     * default values are unknown at this time.
> > +     * The default values selected here are necessary and sufficient
> > +     * to boot Linux directly (on raspi2 and raspi3). The selected
> > +     * values enable all clocks and set clock rates to match their
> > +     * parent rates.
> > +     */
> > +    for (i = CM_GNRICCTL; i <= CM_VECCTL; i += 2) {
> > +        s->regs[i] = 0x11;
> > +        s->regs[i + 1] = 0x1000;
> > +    }
> > +    for (i = CM_DFTCTL; i <= CM_EMMCCTL; i += 2) {
> > +        s->regs[i] = 0x11;
> > +        s->regs[i + 1] = 0x1000;
> > +    }
> 
> If you believe the register lists from the links above this
> loop will set CM_PLLB, which is an exception from the
> pairs of CM_FOOCTL/CM_FOODIV) registers in this range.
> There are also a few CTL/DIV register pairs in the gap
> between CM_VECCTL and CM_EMMCCTL.
> 
Good point. I'll fix that up.

Overall it would be great to get a register dump from someone
with real hardware. I don't really like my guesswork too much.

Thanks,
Guenter