Re: [PATCH v3] hw/sensor: Add lsm303dlhc magnetometer device

[Kevin Townsend <kevin.townsend@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 3573 bytes --]

Hi Peter,

On Mon, 27 Sept 2021 at 18:39, Peter Maydell <peter.maydell@linaro.org>
wrote:

> I thought we'd agreed to implement the whole of the auto-increment
> logic, not just for specific registers ?
>

Thanks again for the feedback. Dealing with one register value at a time
(versus a buffer of response values) does simplify the code flow.

The following code appears to handle multi-byte reads correctly. I just
wanted to confirm this is what you were looking for before moving on to
the test code?

/*
 * Callback handler whenever a 'I2C_START_RECV' (read) event is received.
 */
static void lsm303dlhc_mag_read(LSM303DLHCMagState *s)
{
    /*
     * Set the LOCK bit whenever a new read attempt is made. This will be
     * cleared in I2C_FINISH. Note that DRDY is always set to 1 in this
driver.
     */
    s->sr = 0x3;
}

/*
 * Callback handler whenever a 'I2C_FINISH' event is received.
 */
static void lsm303dlhc_mag_finish(LSM303DLHCMagState *s)
{
    /*
     * Clear the LOCK bit when the read attempt terminates.
     * This bit is initially set in the I2C_START_RECV handler.
     */
    s->sr = 0x1;
}

/*
 * Low-level slave-to-master transaction handler (read attempts).
 */
static uint8_t lsm303dlhc_mag_recv(I2CSlave *i2c)
{
    LSM303DLHCMagState *s = LSM303DLHC_MAG(i2c);

    switch (s->pointer) {
    case LSM303DLHC_MAG_REG_CRA:
        s->buf = s->cra;
        break;
    case LSM303DLHC_MAG_REG_CRB:
        s->buf = s->crb;
        break;
    case LSM303DLHC_MAG_REG_MR:
        s->buf = s->mr;
        break;
    case LSM303DLHC_MAG_REG_OUT_X_H:
        s->buf = (uint8_t)(s->x >> 8);
        break;
    case LSM303DLHC_MAG_REG_OUT_X_L:
        s->buf = (uint8_t)(s->x);
        break;
    case LSM303DLHC_MAG_REG_OUT_Z_H:
        s->buf = (uint8_t)(s->z >> 8);
        break;
    case LSM303DLHC_MAG_REG_OUT_Z_L:
        s->buf = (uint8_t)(s->z);
        break;
    case LSM303DLHC_MAG_REG_OUT_Y_H:
        s->buf = (uint8_t)(s->y >> 8);
        break;
    case LSM303DLHC_MAG_REG_OUT_Y_L:
        s->buf = (uint8_t)(s->y);
        break;
    case LSM303DLHC_MAG_REG_SR:
        s->buf = s->sr;
        break;
    case LSM303DLHC_MAG_REG_IRA:
        s->buf = s->ira;
        break;
    case LSM303DLHC_MAG_REG_IRB:
        s->buf = s->irb;
        break;
    case LSM303DLHC_MAG_REG_IRC:
        s->buf = s->irc;
        break;
    case LSM303DLHC_MAG_REG_TEMP_OUT_H:
        /* Check if the temperature sensor is enabled or not (CRA & 0x80).
*/
        if (s->cra & 0x80) {
            s->buf = (uint8_t)(s->temperature >> 8);
        } else {
            s->buf = 0;
        }
        break;
    case LSM303DLHC_MAG_REG_TEMP_OUT_L:
        if (s->cra & 0x80) {
            s->buf = (uint8_t)(s->temperature & 0xf0);
        } else {
            s->buf = 0;
        }
        break;
    default:
        s->buf = 0;
        break;
    }

    /*
     * The address pointer on the LSM303DLHC auto-increments whenever a byte
     * is read, without the master device having to request the next
address.
     *
     * The auto-increment process has the following logic:
     *
     *   - if (s->pointer == 8) then s->pointer = 3
     *   - else: if (s->pointer >= 12) then s->pointer = 0
     *   - else: s->pointer += 1
     *
     * Reading an invalid address return 0.
     */
    if (s->pointer == LSM303DLHC_MAG_REG_OUT_Y_L) {
        s->pointer = LSM303DLHC_MAG_REG_OUT_X_H;
    } else if (s->pointer >= LSM303DLHC_MAG_REG_IRC) {
        s->pointer = LSM303DLHC_MAG_REG_CRA;
    } else {
        s->pointer++;
    }

    return s->buf;
}

[-- Attachment #2: Type: text/html, Size: 4952 bytes --]