On 6/27/22 17:46, Jae Hyun Yoo wrote:
> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>
> Current implementation of the pmbus core driver treats the read request
> for page 255 as invalid request and sets the invalid command bit (bit 7)
> in the STATUS_CML register. As per the PMBus specification it is a valid
> request.
>
> Refer to the PMBus specification, revision 1.3.1, section 11.10 PAGE,
> on the page 58:
> "Setting the PAGE to FFh means that all subsequent comands are to be
> applied to all outputs.
>
> Some commands, such as READ_TEMPERATURE, may use a common sensor but
> be available on all pages of a device. Such implementations are the
> decision of each device manufacturer or are specified in a PMBus
> Application Profile. Consult the manufacturer's documents or the
> Application Profile Specification as needed."
>
> For e.g.,
> The VOUT_MODE is a valid command for page 255 for maxim 31785 device.
> refer to Table 1. PMBus Command Codes on page 14 in the datasheet.
> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>
> Fixes: 38870253f1d1 ("hw/i2c: pmbus: fix error returns and guard against out of range accesses")
>
> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
> Reviewed-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> Changes in v2:
> * Fixed comment for a case of PB_ALL_PAGES. (Titus)
> * Removed an error log printing when it handles PB_ALL_PAGES. (Jae)
>
> hw/i2c/pmbus_device.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 62885fa6a15e..749a33af827b 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -284,14 +284,10 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
>
> /*
> * Reading from all pages will return the value from page 0,
> - * this is unspecified behaviour in general.
> + * means that all subsequent commands are to be applied to all output.
> */
> 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",