Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Titus Rwantare]

On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>
> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
> channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
> voltages. Datasheet can be found here:
> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>
> This initial version of the driver has skeleton and support for the
> fan channels. Requests for temperature sensors, and ADC Channels the
> are serviced with the default values as per the datasheet.  No additional
> instrumentation is done.  NV Log feature is not supported.
>
> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
> ---
>  hw/arm/Kconfig        |   1 +
>  hw/arm/aspeed.c       |   6 +-
>  hw/sensor/Kconfig     |   4 +
>  hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>  hw/sensor/meson.build |   1 +
>  5 files changed, 590 insertions(+), 2 deletions(-)
>  create mode 100644 hw/sensor/max31785.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 219262a8da36..77ef0fa967b2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -408,6 +408,7 @@ config NPCM7XX
>      select SSI
>      select UNIMP
>      select PCA954X
> +    select MAX31785
>
>  config FSL_IMX25
>      bool

As this is being used with the Aspeed 2600, you may need to select
PMBUS and MAX31785 under config ASPEED_SOC in this same file.


> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
> new file mode 100644
> index 000000000000..11bf9977b6fd
> --- /dev/null
> +++ b/hw/sensor/max31785.c


Also, style nit, the checkpatch.pl script doesn't check whitespace
alignment. But the style guide
https://qemu-project.gitlab.io/qemu/devel/style.html#multiline-indent
specifies the variants we should use.

> +        pmdev->pages[i].vout_scale_monitor =
> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
> +        pmdev->pages[i].vout_ov_fault_limit =
> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
> +        pmdev->pages[i].vout_ov_warn_limit =
> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
> +    }
> +
> +}
> +
> +static const VMStateDescription vmstate_max31785 = {
> +    .name = TYPE_MAX31785,
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]){
> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64(mfr_location, MAX31785State),
> +        VMSTATE_UINT64(mfr_date, MAX31785State),
> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
There's missing indentation here for example.


Thanks,
Titus Rwantare