Apologies. I've updated the commit descriptions and added a sensor using
this code in v2.
While doing block receive I discovered that it is valid behaviour to erase
a field and have it be an empty string.
-Titus
On Thu, 30 Mar 2023 at 09:18, Corey Minyard <miny...@acm.org> wrote:
> 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 <wuhao...@google.com>
> > Signed-off-by: Titus Rwantare <tit...@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
> >
>
