Re: [PATCH 2/4] hw/misc: Add NPCM7XX MFT Module

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

On Fri, 5 Mar 2021 at 18:40, Hao Wu <wuhaotsh@google.com> wrote:
>
> This patch adds Multi Function Timer (MFT) module for NPCM7XX Soc.
> This module is mainly used to configure PWM fans. It has just enough
> functionality to make the PWM fan kernel module work.
>
> The module takes two input, the max_rpm of a fan (modifiable via QMP)
> and duty cycle (a GPIO from the PWM module.) The actual measured RPM
> is equal to max_rpm * duty_cycle / NPCM7XX_PWM_MAX_DUTY. The RPM is
> measured as a counter compared to a prescaled input clock. The kernel
> driver reads this counter and report to user space.
>
> Refs:
> https://github.com/torvalds/linux/blob/master/drivers/hwmon/npcm750-pwm-fan.c
>
> Reviewed-by: Doug Evans <dje@google.com>
> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  docs/system/arm/nuvoton.rst   |   2 +-
>  hw/arm/npcm7xx.c              |  45 ++-
>  hw/misc/meson.build           |   1 +
>  hw/misc/npcm7xx_mft.c         | 541 ++++++++++++++++++++++++++++++++++
>  hw/misc/trace-events          |   8 +
>  include/hw/arm/npcm7xx.h      |   2 +
>  include/hw/misc/npcm7xx_mft.h |  70 +++++
>  7 files changed, 660 insertions(+), 9 deletions(-)
>  create mode 100644 hw/misc/npcm7xx_mft.c
>  create mode 100644 include/hw/misc/npcm7xx_mft.h

Can you split "implement new device" and "add new device to board"
into separate patches, please?


> +static void npcm7xx_mft_init(Object *obj)
> +{
> +    NPCM7xxMFTState *s = NPCM7XX_MFT(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    DeviceState *dev = DEVICE(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &npcm7xx_mft_ops, s,
> +                          TYPE_NPCM7XX_MFT, 4 * KiB);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    s->clock_in = qdev_init_clock_in(dev, "clock-in",
> +                                     npcm7xx_mft_update_clock, s);

You'll need to update this for the change to the Clock API that's
now in master, I'm afraid.

(You'll also find some conflicts in the docs and in the board .c file
when you rebase now the ethernet device patchset has landed, but those
are easy to fix.)

thanks
-- PMM