michaelh/zephyr
1
0
Fork 0
Code Releases Activity

driver: i2c: npcx: simplify smb bank registers with union

Browse source 
For NPCX SMB/I2C SMB modules in FIFO mode, the registers include:

* Common registers, offset 0x00-0x0f, accessible regardless of the value
  of BNK_SEL
* Bank 0 registers, offset 0x10-0x1e, accessible if BNK_SEL is set to 0
* Bank 1 registers, offset 0x10-0x1e, accessible if BNK_SEL
is set to 1

In the current driver, it uses two structures, `smb_reg` and
`smb_fifo_reg`, to access `Common + Bank 0` and `Common + Bank 1`
registers. But It might be easy to misunderstand that they are two
different modules.

This CL tries to simplify this by the following steps:

1. Use `union` to combine `Bank 0/1` registers in the same structure.
2. Remove `smb_fifo_reg`. We needn't use two structures to present
   SMB registers.

Signed-off-by: Mulin Chao <mlchao@nuvoton.com>
This commit is contained in:
Mulin Chao 2023-01-09 17:51:00 -08:00 • committed by Carles Cufí
commit b1214ead19
3 changed files with 91 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

63
drivers/i2c/i2c_npcx_controller.c
View file

@ -161,9 +161,6 @@ struct i2c_ctrl_data {
#define HAL_I2C_INSTANCE(dev) \
((struct smb_reg *)((const struct i2c_ctrl_config *)(dev)->config)->base)
#define HAL_I2C_FIFO_INSTANCE(dev) \
((struct smb_fifo_reg *)((const struct i2c_ctrl_config *)(dev)->config)->base)
/* Recommended I2C timing values are based on 15 MHz */
static const struct npcx_i2c_timing_cfg npcx_15m_speed_confs[] = {
[NPCX_I2C_BUS_SPEED_100KHZ] = {.HLDT = 15, .k1 = 76, .k2 = 0},
@ -180,23 +177,23 @@ static const struct npcx_i2c_timing_cfg npcx_20m_speed_confs[] = {
/* I2C controller inline functions access shared registers */
static inline void i2c_ctrl_start(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
inst_fifo->SMBCTL1 |= BIT(NPCX_SMBCTL1_START);
inst->SMBCTL1 |= BIT(NPCX_SMBCTL1_START);
}
static inline void i2c_ctrl_stop(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
inst_fifo->SMBCTL1 |= BIT(NPCX_SMBCTL1_STOP);
inst->SMBCTL1 |= BIT(NPCX_SMBCTL1_STOP);
}
static inline int i2c_ctrl_bus_busy(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
return IS_BIT_SET(inst_fifo->SMBCST, NPCX_SMBCST_BB);
return IS_BIT_SET(inst->SMBCST, NPCX_SMBCST_BB);
}
static inline void i2c_ctrl_bank_sel(const struct device *dev, int bank)
@ -282,54 +279,54 @@ static inline void i2c_ctrl_norm_free_sda(const struct device *dev)
/* I2C controller inline functions access registers in 'FIFO' bank */
static inline void i2c_ctrl_fifo_write(const struct device *dev, uint8_t data)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
inst_fifo->SMBSDA = data;
inst->SMBSDA = data;
}
static inline uint8_t i2c_ctrl_fifo_read(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
return inst_fifo->SMBSDA;
return inst->SMBSDA;
}
static inline int i2c_ctrl_fifo_tx_avail(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
return NPCX_I2C_FIFO_MAX_SIZE - (inst_fifo->SMBTXF_STS & 0x3f);
return NPCX_I2C_FIFO_MAX_SIZE - (inst->SMBTXF_STS & 0x3f);
}
static inline int i2c_ctrl_fifo_rx_occupied(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
return inst_fifo->SMBRXF_STS & 0x3f;
return inst->SMBRXF_STS & 0x3f;
}
static inline void i2c_ctrl_fifo_rx_setup_threshold_nack(
const struct device *dev, int threshold, int last)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
uint8_t value = MIN(threshold, NPCX_I2C_FIFO_MAX_SIZE);
SET_FIELD(inst_fifo->SMBRXF_CTL, NPCX_SMBRXF_CTL_RX_THR, value);
SET_FIELD(inst->SMBRXF_CTL, NPCX_SMBRXF_CTL_RX_THR, value);
/*
* Is it last received transaction? If so, set LAST bit. Then the
* hardware will generate NACK automatically when receiving last byte.
*/
if (last && (value == threshold)) {
inst_fifo->SMBRXF_CTL |= BIT(NPCX_SMBRXF_CTL_LAST);
inst->SMBRXF_CTL |= BIT(NPCX_SMBRXF_CTL_LAST);
}
}
static inline void i2c_ctrl_fifo_clear_status(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
inst_fifo->SMBFIF_CTS |= BIT(NPCX_SMBFIF_CTS_CLR_FIFO);
inst->SMBFIF_CTS |= BIT(NPCX_SMBFIF_CTS_CLR_FIFO);
}
/*
@ -407,7 +404,7 @@ static void i2c_ctrl_config_bus_freq(const struct device *dev,
/* I2C controller local functions */
static int i2c_ctrl_wait_stop_completed(const struct device *dev, int timeout)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
if (timeout <= 0) {
return -EINVAL;
@ -418,7 +415,7 @@ static int i2c_ctrl_wait_stop_completed(const struct device *dev, int timeout)
* Wait till i2c bus is idle. This bit is cleared to 0
* automatically after the STOP condition is generated.
*/
if (!IS_BIT_SET(inst_fifo->SMBCTL1, NPCX_SMBCTL1_STOP))
if (!IS_BIT_SET(inst->SMBCTL1, NPCX_SMBCTL1_STOP))
break;
k_msleep(1);
} while (--timeout);
@ -465,7 +462,7 @@ static int i2c_ctrl_wait_idle_completed(const struct device *dev, int timeout)
static int i2c_ctrl_recovery(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
struct i2c_ctrl_data *const data = dev->data;
int ret;
@ -482,9 +479,9 @@ static int i2c_ctrl_recovery(const struct device *dev)
* - Wait for STOP condition completed
* - Then clear BB (BUS BUSY) bit
*/
inst_fifo->SMBST = BIT(NPCX_SMBST_BER) | BIT(NPCX_SMBST_NEGACK);
inst->SMBST = BIT(NPCX_SMBST_BER) | BIT(NPCX_SMBST_NEGACK);
ret = i2c_ctrl_wait_stop_completed(dev, I2C_MAX_TIMEOUT);
inst_fifo->SMBCST |= BIT(NPCX_SMBCST_BB);
inst->SMBCST |= BIT(NPCX_SMBCST_BB);
if (ret != 0) {
LOG_ERR("Abort i2c port%02x fail! Bus might be stalled.",
data->port);
@ -496,7 +493,7 @@ static int i2c_ctrl_recovery(const struct device *dev)
* - Wait both SCL/SDA line are high
* - Enable i2c module again
*/
inst_fifo->SMBCTL2 &= ~BIT(NPCX_SMBCTL2_ENABLE);
inst->SMBCTL2 &= ~BIT(NPCX_SMBCTL2_ENABLE);
ret = i2c_ctrl_wait_idle_completed(dev, I2C_MAX_TIMEOUT);
if (ret != 0) {
LOG_ERR("Reset i2c port%02x fail! Bus might be stalled.",
@ -743,11 +740,11 @@ static int i2c_ctrl_proc_read_msg(const struct device *dev, struct i2c_msg *msg)
/* I2C controller isr function */
static void i2c_ctrl_isr(const struct device *dev)
{
struct smb_fifo_reg *const inst_fifo = HAL_I2C_FIFO_INSTANCE(dev);
struct smb_reg *const inst = HAL_I2C_INSTANCE(dev);
struct i2c_ctrl_data *const data = dev->data;
uint8_t status, tmp;
status = inst_fifo->SMBST & NPCX_VALID_SMBST_MASK;
status = inst->SMBST & NPCX_VALID_SMBST_MASK;
LOG_DBG("status: %02x, %d", status, data->oper_state);
/* A 'Bus Error' has been identified */
@ -756,7 +753,7 @@ static void i2c_ctrl_isr(const struct device *dev)
i2c_ctrl_stop(dev);
/* Clear BER Bit */
inst_fifo->SMBST = BIT(NPCX_SMBST_BER);
inst->SMBST = BIT(NPCX_SMBST_BER);
/* Make sure slave doesn't hold bus by reading FIFO again */
tmp = i2c_ctrl_fifo_read(dev);
@ -775,7 +772,7 @@ static void i2c_ctrl_isr(const struct device *dev)
i2c_ctrl_stop(dev);
/* Clear NEGACK Bit */
inst_fifo->SMBST = BIT(NPCX_SMBST_NEGACK);
inst->SMBST = BIT(NPCX_SMBST_NEGACK);
/* End transaction */
data->oper_state = NPCX_I2C_WAIT_STOP;
@ -795,7 +792,7 @@ static void i2c_ctrl_isr(const struct device *dev)
/* Clear unexpected status bits */
if (status != 0) {
inst_fifo->SMBST = status;
inst->SMBST = status;
LOG_ERR("Unexpected SMBST 0x%02x occurred on i2c port%02x!",
status, data->port);
}

139
soc/arm/nuvoton_npcx/common/reg/reg_def.h
View file

@ -1113,87 +1113,64 @@ struct smb_reg {
volatile uint8_t SMBCTL3;
/* 0x00F: SMB Bus Timeout */
volatile uint8_t SMBT_OUT;
/* 0x010: SMB Own Address 3 */
volatile uint8_t SMBADDR3;
/* 0x011: SMB Own Address 7 */
volatile uint8_t SMBADDR7;
/* 0x012: SMB Own Address 4 */
volatile uint8_t SMBADDR4;
/* 0x013: SMB Own Address 8 */
volatile uint8_t SMBADDR8;
/* 0x014: SMB Own Address 5 */
volatile uint8_t SMBADDR5;
volatile uint8_t reserved8;
/* 0x016: SMB Own Address 6 */
volatile uint8_t SMBADDR6;
volatile uint8_t reserved9;
/* 0x018: SMB Control Status 2 */
volatile uint8_t SMBCST2;
/* 0x019: SMB Control Status 3 */
volatile uint8_t SMBCST3;
/* 0x01A: SMB Control 4 */
volatile uint8_t SMBCTL4;
volatile uint8_t reserved10;
/* 0x01C: SMB SCL Low Time */
volatile uint8_t SMBSCLLT;
/* 0x01D: SMB FIFO Control */
volatile uint8_t SMBFIF_CTL;
/* 0x01E: SMB SCL High Time */
volatile uint8_t SMBSCLHT;
volatile uint8_t reserved11;
};
/*
* SMBUS (SMB) FIFO device registers
*/
struct smb_fifo_reg {
/* 0x000: SMB Serial Data */
volatile uint8_t SMBSDA;
volatile uint8_t reserved1;
/* 0x002: SMB Status */
volatile uint8_t SMBST;
volatile uint8_t reserved2;
/* 0x004: SMB Control Status */
volatile uint8_t SMBCST;
volatile uint8_t reserved3;
/* 0x006: SMB Control 1 */
volatile uint8_t SMBCTL1;
volatile uint8_t reserved4;
/* 0x008: SMB Own Address */
volatile uint8_t SMBADDR1;
volatile uint8_t reserved5;
/* 0x00A: SMB Control 2 */
volatile uint8_t SMBCTL2;
volatile uint8_t reserved6;
/* 0x00C: SMB Own Address */
volatile uint8_t SMBADDR2;
volatile uint8_t reserved7;
/* 0x00E: SMB Control 3 */
volatile uint8_t SMBCTL3;
/* 0x00F: SMB Bus Timeout */
volatile uint8_t SMBT_OUT;
/* 0x010: SMB FIFO Control */
volatile uint8_t SMBFIF_CTS;
volatile uint8_t reserved8;
/* 0x012: SMB Tx-FIFO Control */
volatile uint8_t SMBTXF_CTL;
volatile uint8_t reserved9;
/* 0x014: SMB Bus Timeout */
volatile uint8_t SMB_T_OUT;
volatile uint8_t reserved10[3];
/* 0x018: SMB Control Status 2 */
volatile uint8_t SMBCST2;
/* 0x019: SMB Control Status 3 */
volatile uint8_t SMBCST3;
/* 0x01A: SMB Tx-FIFO Status */
volatile uint8_t SMBTXF_STS;
volatile uint8_t reserved11;
/* 0x01C: SMB Rx-FIFO Status */
volatile uint8_t SMBRXF_STS;
volatile uint8_t reserved12;
/* 0x01E: SMB Rx-FIFO Control */
volatile uint8_t SMBRXF_CTL;
volatile uint8_t reserved13;
union {
/* Bank 0 */
struct {
/* 0x010: SMB Own Address 3 */
volatile uint8_t SMBADDR3;
/* 0x011: SMB Own Address 7 */
volatile uint8_t SMBADDR7;
/* 0x012: SMB Own Address 4 */
volatile uint8_t SMBADDR4;
/* 0x013: SMB Own Address 8 */
volatile uint8_t SMBADDR8;
/* 0x014: SMB Own Address 5 */
volatile uint8_t SMBADDR5;
volatile uint8_t reserved8;
/* 0x016: SMB Own Address 6 */
volatile uint8_t SMBADDR6;
volatile uint8_t reserved9;
/* 0x018: SMB Control Status 2 */
volatile uint8_t SMBCST2;
/* 0x019: SMB Control Status 3 */
volatile uint8_t SMBCST3;
/* 0x01A: SMB Control 4 */
volatile uint8_t SMBCTL4;
volatile uint8_t reserved10;
/* 0x01C: SMB SCL Low Time */
volatile uint8_t SMBSCLLT;
/* 0x01D: SMB FIFO Control */
volatile uint8_t SMBFIF_CTL;
/* 0x01E: SMB SCL High Time */
volatile uint8_t SMBSCLHT;
volatile uint8_t reserved11;
};
/* Bank 1 */
struct {
/* 0x010: SMB FIFO Control */
volatile uint8_t SMBFIF_CTS;
volatile uint8_t reserved12;
/* 0x012: SMB Tx-FIFO Control */
volatile uint8_t SMBTXF_CTL;
volatile uint8_t reserved13;
/* 0x014: SMB Bus Timeout */
volatile uint8_t SMB_T_OUT;
volatile uint8_t reserved14[3];
/* 0x018: SMB Control Status 2 (FIFO) */
volatile uint8_t SMBCST2_FIFO;
/* 0x019: SMB Control Status 3 (FIFO) */
volatile uint8_t SMBCST3_FIFO;
/* 0x01A: SMB Tx-FIFO Status */
volatile uint8_t SMBTXF_STS;
volatile uint8_t reserved15;
/* 0x01C: SMB Rx-FIFO Status */
volatile uint8_t SMBRXF_STS;
volatile uint8_t reserved16;
/* 0x01E: SMB Rx-FIFO Control */
volatile uint8_t SMBRXF_CTL;
volatile uint8_t reserved17[1];
};
};
};
/* SMB register fields */

9
soc/arm/nuvoton_npcx/common/registers.c
View file

@ -121,13 +121,10 @@ NPCX_REG_SIZE_CHECK(smb_reg, 0x020);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBCTL1, 0x006);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBT_OUT, 0x00f);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBADDR6, 0x016);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBCST2, 0x018);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBTXF_STS, 0x01a);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBSCLHT, 0x01e);
NPCX_REG_SIZE_CHECK(smb_fifo_reg, 0x020);
NPCX_REG_OFFSET_CHECK(smb_fifo_reg, SMBT_OUT, 0x00f);
NPCX_REG_OFFSET_CHECK(smb_fifo_reg, SMBCST2, 0x018);
NPCX_REG_OFFSET_CHECK(smb_fifo_reg, SMBTXF_STS, 0x01a);
NPCX_REG_OFFSET_CHECK(smb_fifo_reg, SMBRXF_CTL, 0x01e);
NPCX_REG_OFFSET_CHECK(smb_reg, SMBRXF_CTL, 0x01e);
/* ITIM register structure check */
NPCX_REG_SIZE_CHECK(itim32_reg, 0x00c);