On 18/03/2019 18.15, Paolo Bonzini wrote:
> The functions to read/write 8-bit or 16-bit registers are the same
> in tmp105 and pca9552 tests, and in fact they are a special case of
> "read block"/"write block" functionality; read block in turn is used
> in ds1338-test.
>
> Move everything inside libqos-test, removing the duplication. Account
> for the small differences by adding to tmp105-test.c the "read register
> after writing" behavior that is specific to it.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> tests/ds1338-test.c | 8 +----
> tests/libqos/i2c.c | 47 ++++++++++++++++++++++++++
> tests/libqos/i2c.h | 11 ++++++
> tests/pca9552-test.c | 37 +++++---------------
> tests/tmp105-test.c | 80 ++++++++++++--------------------------------
> 5 files changed, 88 insertions(+), 95 deletions(-)
>
> diff --git a/tests/ds1338-test.c b/tests/ds1338-test.c
> index 742dad9113..88f829f241 100644
> --- a/tests/ds1338-test.c
> +++ b/tests/ds1338-test.c
> @@ -35,17 +35,11 @@ static inline uint8_t bcd2bin(uint8_t x)
>
> static void send_and_receive(void)
> {
> - uint8_t cmd[1];
> uint8_t resp[7];
> time_t now = time(NULL);
> struct tm *tm_ptr = gmtime(&now);
>
> - /* reset the index in the RTC memory */
> - cmd[0] = 0;
> - i2c_send(i2c, addr, cmd, 1);
> -
> - /* retrieve the date */
> - i2c_recv(i2c, addr, resp, 7);
> + i2c_read_block(i2c, addr, 0, resp, sizeof(resp));
>
> /* check retrieved time againt local time */
> g_assert_cmpuint(bcd2bin(resp[4]), == , tm_ptr->tm_mday);
> diff --git a/tests/libqos/i2c.c b/tests/libqos/i2c.c
> index 23bc2a3eb2..daf9a96617 100644
> --- a/tests/libqos/i2c.c
> +++ b/tests/libqos/i2c.c
> @@ -21,3 +21,50 @@ void i2c_recv(I2CAdapter *i2c, uint8_t addr,
> {
> i2c->recv(i2c, addr, buf, len);
> }
> +
> +void i2c_read_block(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> + uint8_t *buf, uint16_t len)
> +{
> + i2c_send(i2c, addr, ®, 1);
> + i2c_recv(i2c, addr, buf, len);
> +}
> +
> +void i2c_write_block(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> + const uint8_t *buf, uint16_t len)
> +{
> + uint8_t *cmd = g_malloc(len + 1);
> + cmd[0] = reg;
> + memcpy(&cmd[1], buf, len);
> + i2c_send(i2c, addr, cmd, len + 1);
> + g_free(cmd);
> +}
So the i2c_write_block function only uses i2c_send() ...
> +uint8_t i2c_get8(I2CAdapter *i2c, uint8_t addr, uint8_t reg)
> +{
> + uint8_t resp[1];
> + i2c_read_block(i2c, addr, reg, resp, sizeof(resp));
> + return resp[0];
> +}
> +
> +uint16_t i2c_get16(I2CAdapter *i2c, uint8_t addr, uint8_t reg)
> +{
> + uint8_t resp[2];
> + i2c_read_block(i2c, addr, reg, resp, sizeof(resp));
> + return (resp[0] << 8) | resp[1];
> +}
> +
> +void i2c_set8(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> + uint8_t value)
> +{
> + i2c_write_block(i2c, addr, reg, &value, 1);
> +}
> +
> +void i2c_set16(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> + uint16_t value)
> +{
> + uint8_t data[2];
> +
> + data[0] = value >> 8;
> + data[1] = value & 255;
> + i2c_write_block(i2c, addr, reg, data, sizeof(data));
> +}
... i.e. the i2c_set8/16() functions also only use i2c_send()...
> -static void tmp105_set8(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> - uint8_t value)
> -{
> - uint8_t cmd[2];
> - uint8_t resp[1];
> -
> - cmd[0] = reg;
> - cmd[1] = value;
> - i2c_send(i2c, addr, cmd, 2);
> - i2c_recv(i2c, addr, resp, 1);
> - g_assert_cmphex(resp[0], ==, cmd[1]);
> -}
> -
> -static void tmp105_set16(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
> - uint16_t value)
> -{
> - uint8_t cmd[3];
> - uint8_t resp[2];
> -
> - cmd[0] = reg;
> - cmd[1] = value >> 8;
> - cmd[2] = value & 255;
> - i2c_send(i2c, addr, cmd, 3);
> - i2c_recv(i2c, addr, resp, 2);
> - g_assert_cmphex(resp[0], ==, cmd[1]);
> - g_assert_cmphex(resp[1], ==, cmd[2]);
> -}
... but the old set8/16 functions also used i2c_recv() and
g_assert_cmphex() ... shouldn't this be added to the new functions, too?
Thomas
