Re: [PATCH 5/9] hw/i2c: pmbus: Page #255 is valid page for read requests.

[Titus Rwantare]

On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> 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 socuments or the Applicatin Profile Specification as 
> needed."
>
Thanks for this, the copy of the spec I used was older.


> 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>
> ---
>  hw/i2c/pmbus_device.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 62885fa6a15e..7db3343a83b6 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -291,7 +291,6 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
>          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",
> --
> 2.25.1
>

Please also update the stale comment just above, since this is now
specified behaviour.

Reviewed-by: Titus Rwantare <titusr@google.com>