Hello Peter,
Thank you for the review. Please see my comments inline.
Thank you,
Mahesh
On 7/14/22 8:10 AM, Peter Maydell wrote:
On Thu, 14 Jul 2022 at 14:04, Maheswara Kurapati
<quic_mkura...@quicinc.com> wrote:
This fix adds object properties for the FAN_COMMAND_1 (3Bh), STATUS_FANS_1_2
(81h),
READ_FAN_SPEED_1 (90h) registers for the MAX31785 instrumentation. An additional
property tach_margin_percent updates the tachs for a configured percent of
FAN_COMMAND_1 value.
Register property
--------------------------------------
FAN_COMMAND_1 (3Bh) fan_target
STATUS_FANS_1_2 (81h) status_fans_1_2
READ_FAN_SPEED_1 (90h) fan_input
This commit message is missing the rationale -- why do we need this?
The STATUS_FANS_1_2, and READ_FAN_SPEED_1 registers are read-only. I
added these properties to simulate the error device faults.
I am also not sure that we should be defining properties that are
just straight 1:1 with the device registers. Compare the way we
handle temperature-sensor values, where the property values are
defined in a generic manner (same units representation) regardless
of the underlying device and the device's property-set-get implementation
then handles converting that to and from whatever internal implementation
representation the device happens to use.
I am not sure I understood your comment. I checked hw/sensors/tmp105.c,
in which a "temperature" property is added for the tmp_input field in
almost the similar way what I did, except that the registers in the
MAX31785 are in direct format.
thanks
-- PMM
