Ack. All errors for PMBus should ideally be reflected in status and status_cml registers instead of carrying meaning in return values. I'll have to separately go through the existing code to make it consistent.
On Fri, 4 Mar 2022 at 16:08, Philippe Mathieu-Daudé <philippe.mathieu.da...@gmail.com> wrote: > > On 2/3/22 02:50, Titus Rwantare wrote: > > Signed-off-by: Titus Rwantare <tit...@google.com> > > --- > > hw/i2c/pmbus_device.c | 41 ++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > static uint8_t pmbus_receive_byte(SMBusDevice *smd) > > { > > PMBusDevice *pmdev = PMBUS_DEVICE(smd); > > PMBusDeviceClass *pmdc = PMBUS_DEVICE_GET_CLASS(pmdev); > > uint8_t ret = 0xFF; > > - uint8_t index = pmdev->page; > > + uint8_t index; > > > > if (pmdev->out_buf_len != 0) { > > ret = pmbus_out_buf_pop(pmdev); > > return ret; > > } > > > > + /* > > + * Reading from all pages will return the value from page 0, > > + * this is unspecified behaviour in general. > > + */ > > + if (pmdev->page == PB_ALL_PAGES) { > > + index = 0; > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: tried to read from all pages\n", > > + __func__); > > + pmbus_cml_error(pmdev); > > + } else if (pmdev->page > pmdev->num_pages - 1) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: page %d is out of range\n", > > + __func__, pmdev->page); > > + pmbus_cml_error(pmdev); > > + return -1; > > This file returns a mix of 0xFF/-1 for error. It would be nice > to pick one. Adding a definition (PMBUS_ERR_BYTE?) could help. > > Preferably with error unified: > Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>