esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
701 stars 192 forks source link

QOL: Do something about `regi2c_*` functions. #1740

Open playfulFence opened 2 months ago

playfulFence commented 2 months ago

It would be nice to do something about them regi2c_* functions (see example below), because honestly, every time I need to use them is a huge pain + a lot of noise in the code base: extra raw register address definitions, basic low-level functions (which are repeating through the whole code base) and so on.

As you can see on the example below, one operation requires a lot of collateral registers/values definitions, functions and other things, which create A) Inconvenience of work B) Noise in a codebase.

I see couple of reasons why we didn't solve it yet: A) Some of required registers are not in PACs yet, so we can't use related functions ( e.g. .modify()) B) Lack of knowledge about this functionality (It is documented literally nowhere, neither in the IDF nor in TRM)

At this point the code pretty much copies the IDF approach (mostly due to the reason B from above šŸ˜… ), but I'm sure we can make it more convenient.

Example:

const REGI2C_BBPLL: u8 = 0x66;
const REGI2C_BIAS: u8 = 0x6a;
const REGI2C_PMU_REG: u8 = 0x6d;
const REGI2C_ULP_CAL: u8 = 0x61;
const REGI2C_SAR_I2C: u8 = 0x69;

const REGI2C_BBPLL_DEVICE_EN: u32 = 1 << 9; // (1 << 5) << 4;
const REGI2C_BIAS_DEVICE_EN: u32 = 1 << 8; // (1 << 4) << 4;
const REGI2C_PMU_DEVICE_EN: u32 = 1 << 12; // (1 << 8) << 4;
const REGI2C_ULP_CAL_DEVICE_EN: u32 = 1 << 10; // (1 << 6) << 4;
const REGI2C_SAR_I2C_DEVICE_EN: u32 = 1 << 11; // (1 << 7) << 4;

const REGI2C_RTC_SLAVE_ID_V: u8 = 0xFF;
const REGI2C_RTC_SLAVE_ID_S: u8 = 0;
const REGI2C_RTC_ADDR_V: u8 = 0xFF;
const REGI2C_RTC_ADDR_S: u8 = 8;
const REGI2C_RTC_WR_CNTL_V: u8 = 0x1;
const REGI2C_RTC_WR_CNTL_S: u8 = 24;
const REGI2C_RTC_DATA_V: u8 = 0xFF;
const REGI2C_RTC_DATA_S: u8 = 16;

fn reg_get_bit(reg: u32, b: u32) -> u32 {
    unsafe { (reg as *mut u32).read_volatile() & b }
}

fn reg_write(reg: u32, v: u32) {
    unsafe {
        (reg as *mut u32).write_volatile(v);
    }
}

fn reg_get_field(reg: u32, s: u32, v: u32) -> u32 {
    unsafe { ((reg as *mut u32).read_volatile() >> s) & v }
}

fn reg_clr_bit(reg: u32, bit: u32) {
    unsafe {
        (reg as *mut u32).write_volatile((reg as *mut u32).read_volatile() & !bit);
    }
}

fn regi2c_enable_block(block: u8) {
    reg_set_bit(MODEM_LPCON_CLK_CONF_REG, MODEM_LPCON_CLK_I2C_MST_EN);
    reg_set_bit(I2C_MST_DATE_REG, I2C_MST_CLK_EN);

    // Make I2C_MST_ANA_CONF2 in I2C_MST_ANA_CONF2_REG be 0
    unsafe {
        (I2C_MST_ANA_CONF2_REG as *mut u32).write_volatile(
            // (1 << 18)
            (I2C_MST_ANA_CONF2_REG as *mut u32).read_volatile() & !I2C_MST_ANA_CONF2,
        );
    }

    // Before config I2C register, enable corresponding slave.
    match block {
        REGI2C_BBPLL => {
            reg_set_bit(I2C_MST_ANA_CONF2_REG, REGI2C_BBPLL_DEVICE_EN);
        }
        REGI2C_BIAS => {
            reg_set_bit(I2C_MST_ANA_CONF2_REG, REGI2C_BIAS_DEVICE_EN);
        }
        REGI2C_PMU_REG => {
            reg_set_bit(I2C_MST_ANA_CONF2_REG, REGI2C_PMU_DEVICE_EN);
        }
        REGI2C_ULP_CAL => {
            reg_set_bit(I2C_MST_ANA_CONF2_REG, REGI2C_ULP_CAL_DEVICE_EN);
        }
        REGI2C_SAR_I2C => {
            reg_set_bit(I2C_MST_ANA_CONF2_REG, REGI2C_SAR_I2C_DEVICE_EN);
        }
        _ => (),
    }
}

fn regi2c_disable_block(block: u8) {
    match block {
        REGI2C_BBPLL => {
            reg_clr_bit(I2C_MST_ANA_CONF2_REG, REGI2C_BBPLL_DEVICE_EN);
        }
        REGI2C_BIAS => {
            reg_clr_bit(I2C_MST_ANA_CONF2_REG, REGI2C_BIAS_DEVICE_EN);
        }
        REGI2C_PMU_REG => {
            reg_clr_bit(I2C_MST_ANA_CONF2_REG, REGI2C_PMU_DEVICE_EN);
        }
        REGI2C_ULP_CAL => {
            reg_clr_bit(I2C_MST_ANA_CONF2_REG, REGI2C_ULP_CAL_DEVICE_EN);
        }
        REGI2C_SAR_I2C => {
            reg_clr_bit(I2C_MST_ANA_CONF2_REG, REGI2C_SAR_I2C_DEVICE_EN);
        }
        _ => (),
    }
}

pub(crate) fn regi2c_write_mask(block: u8, _host_id: u8, reg_add: u8, msb: u8, lsb: u8, data: u8) {
    assert!(msb - lsb < 8);
    regi2c_enable_block(block);

    // Read the i2c bus register
    while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}

    let mut temp: u32 = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32)
        << REGI2C_RTC_SLAVE_ID_S as u32)
        | (reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32;
    reg_write(I2C_MST_I2C0_CTRL_REG, temp);
    while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}
    temp = reg_get_field(
        I2C_MST_I2C0_CTRL_REG,
        REGI2C_RTC_DATA_S as u32,
        REGI2C_RTC_DATA_V as u32,
    );
    // Write the i2c bus register
    temp &= (!(0xFFFFFFFF << lsb)) | (0xFFFFFFFF << (msb + 1));
    temp |= (data as u32 & (!(0xFFFFFFFF << (msb as u32 - lsb as u32 + 1)))) << (lsb as u32);
    temp = ((block as u32 & REGI2C_RTC_SLAVE_ID_V as u32) << REGI2C_RTC_SLAVE_ID_S as u32)
        | ((reg_add as u32 & REGI2C_RTC_ADDR_V as u32) << REGI2C_RTC_ADDR_S as u32)
        | ((0x1 & REGI2C_RTC_WR_CNTL_V as u32) << REGI2C_RTC_WR_CNTL_S as u32)
        | ((temp & REGI2C_RTC_DATA_V as u32) << REGI2C_RTC_DATA_S as u32);
    reg_write(I2C_MST_I2C0_CTRL_REG, temp);
    while reg_get_bit(I2C_MST_I2C0_CTRL_REG, REGI2C_RTC_BUSY) != 0 {}

    regi2c_disable_block(block);
}
Frostie314159 commented 3 weeks ago

With the esp32-open-mac project, we discovered this as well, since the proprietary Wi-Fi stack calls i2c_readReg_Mask at least 30 times. My hypothesis is, that the RF frontend, which Espressif licensed from Ceva, is intended to be integrable into another chip separate from the baseband and be addressed over I2C. We're ignoring the RF init for now.

I know you likely can't share info on this, but that's what we found.