On Thu, 14 Jul 2022 at 22:14, Maheswara Kurapati <quic_mkura...@quicinc.com> wrote: > 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'm not entirely sure what you have in mind here, but QEMU doesn't generally simulate device error injection. > > 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. Yes, that is my point. My impression is that you've provided properties that directly match the register format of this device because that's easy. I think that instead we should consider what the properties are intended to do, and perhaps have a standard convention for what format to use for particular kinds of data, as we do for temperature already. -- PMM
