|
From: | Titus Rwantare |
Subject: | Re: [PATCH 1/5] hw/i2c: pmbus add support for block receive |
Date: | Thu, 30 Mar 2023 17:05:44 -0700 |
It's generally frowned upon to have empty descriptions, some rationale
would be helpful. For instance, you remove a length check from the send
string, why did you do that?
Any why is this being added? What's it supporting?
-corey
On Wed, Mar 22, 2023 at 05:55:09PM +0000, Titus Rwantare wrote:
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Signed-off-by: Titus Rwantare <titusr@google.com>
> ---
> hw/i2c/pmbus_device.c | 30 +++++++++++++++++++++++++++++-
> include/hw/i2c/pmbus_device.h | 7 +++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index c3d6046784..02647769cd 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -95,7 +95,6 @@ void pmbus_send64(PMBusDevice *pmdev, uint64_t data)
> void pmbus_send_string(PMBusDevice *pmdev, const char *data)
> {
> size_t len = strlen(data);
> - g_assert(len > 0);
> g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
> pmdev->out_buf[len + pmdev->out_buf_len] = len;
>
> @@ -105,6 +104,35 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
> pmdev->out_buf_len += len + 1;
> }
>
> +uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len)
> +{
> + /* dest may contain data from previous writes */
> + memset(dest, 0, len);
> +
> + /* Exclude command code from return value */
> + pmdev->in_buf++;
> + pmdev->in_buf_len--;
> +
> + /* The byte after the command code denotes the length */
> + uint8_t sent_len = pmdev->in_buf[0];
> +
> + if (sent_len != pmdev->in_buf_len - 1) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: length mismatch. Expected %d bytes, got %d bytes\n",
> + __func__, sent_len, pmdev->in_buf_len - 1);
> + }
> +
> + /* exclude length byte */
> + pmdev->in_buf++;
> + pmdev->in_buf_len--;
> +
> + if (pmdev->in_buf_len < len) {
> + len = pmdev->in_buf_len;
> + }
> + memcpy(dest, pmdev->in_buf, len);
> + return len;
> +}
> +
>
> static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
> {
> diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
> index 93f5d57c9d..7dc00cc4d9 100644
> --- a/include/hw/i2c/pmbus_device.h
> +++ b/include/hw/i2c/pmbus_device.h
> @@ -501,6 +501,13 @@ void pmbus_send64(PMBusDevice *state, uint64_t data);
> */
> void pmbus_send_string(PMBusDevice *state, const char *data);
>
> +/**
> + * @brief Receive data sent with Block Write.
> + * @param dest - memory with enough capacity to receive the write
> + * @param len - the capacity of dest
> + */
> +uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len);
> +
> /**
> * @brief Receive data over PMBus
> * These methods help track how much data is being received over PMBus
> --
> 2.40.0.rc1.284.g88254d51c5-goog
>
[Prev in Thread] | Current Thread | [Next in Thread] |