On Thu, 27 Jan 2022 at 20:54, Titus Rwantare <tit...@google.com> wrote: > > On Thu, 27 Jan 2022 at 11:39, Peter Maydell <peter.mayd...@linaro.org> wrote: > > > > On Thu, 6 Jan 2022 at 23:19, Titus Rwantare <tit...@google.com> wrote: > > > > > > > +static uint8_t isl_pmbus_read_byte(PMBusDevice *pmdev) > > > +{ > > > + qemu_log_mask(LOG_GUEST_ERROR, > > > + "%s: reading from unsupported register: 0x%02x\n", > > > + __func__, pmdev->code); > > > + return 0xFF; > > > +} > > > + > > > +static int isl_pmbus_write_data(PMBusDevice *pmdev, const uint8_t *buf, > > > + uint8_t len) > > > +{ > > > + qemu_log_mask(LOG_GUEST_ERROR, > > > + "%s: write to unsupported register: 0x%02x\n", > > > + __func__, pmdev->code); > > > + return 0xFF; > > > +} > > > > This device appears to have no implemented guest visible > > interface at all, and yet it has a lot of object properties. > > What's going on here ?
> This device relies on read_byte and write_data implemented in > pmbus_device.c. Those generic implementations fall through to the > device specific implementations for registers not in the standard. > This qemu model happens not to include additional registers. However, > I must change these to LOG_UNIMP which is more appropriate to what's > going on. Ah, right. A comment noting something like that might also be helpful. Something I forgot to note -- the filenames isl_pmbus.[ch] seem a bit overly generic for "PMBus device for Renesas Digital Multiphase Voltage Regulators" -- could you rename to something a bit more specific? This will help to avoid in future people lumping lots of different unrelated device models into the same source file. thanks -- PMM