On Tue, 15 Dec 2020 at 00:13, Hao Wu <wuhao...@google.com> wrote:
>
> The PWM module is part of NPCM7XX module. Each NPCM7XX module has two
> identical PWM modules. Each module contains 4 PWM entries. Each PWM has
> two outputs: frequency and duty_cycle. Both are computed using inputs
> from software side.
>
> This module does not model detail pulse signals since it is expensive.
> It also does not model interrupts and watchdogs that are dependant on
> the detail models. The interfaces for these are left in the module so
> that anyone in need for these functionalities can implement on their
> own.
>
> The user can read the duty cycle and frequency using qom-get command.
>
> Reviewed-by: Havard Skinnemoen <hskinnem...@google.com>
> Reviewed-by: Tyrone Ting <kft...@nuvoton.com>
> Signed-off-by: Hao Wu <wuhao...@google.com>
> +static void npcm7xx_pwm_init(Object *obj)
> +{
> + NPCM7xxPWMState *s = NPCM7XX_PWM(obj);
> + SysBusDevice *sbd = &s->parent;
This isn't right. A device shouldn't be poking around
in the 'parent' or 'parentobj' member of its struct --
that is a QOM internal. If you want "this device, cast
to a SysBusDevice", the way to write that is:
SysBusDevice *sbd = SYS_BUS_DEVICE(s);
(or you could pass 'obj'; same thing).
Looking at the code currently in the tree it also is making this
same mistake:
$ git grep -- '->parent' hw/*/npcm*
hw/arm/npcm7xx_boards.c: MachineClass *mc = &nmc->parent;
hw/mem/npcm7xx_mc.c: sysbus_init_mmio(&s->parent, &s->mmio);
hw/misc/npcm7xx_clk.c: sysbus_init_mmio(&s->parent, &s->iomem);
hw/misc/npcm7xx_gcr.c: sysbus_init_mmio(&s->parent, &s->iomem);
hw/misc/npcm7xx_rng.c: sysbus_init_mmio(&s->parent, &s->iomem);
hw/nvram/npcm7xx_otp.c: SysBusDevice *sbd = &s->parent;
hw/ssi/npcm7xx_fiu.c: SysBusDevice *sbd = &s->parent;
hw/timer/npcm7xx_timer.c: SysBusDevice *sbd = &s->parent;
These all should be using QOM cast macros. Would somebody
who's working on these devices like to send a patch ?
thanks
-- PMM
