Skip to content

drivers: i3c_mcux: Improve timeout handling #97477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+60 −52
Open

drivers: i3c_mcux: Improve timeout handling #97477

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 60 additions & 52 deletions drivers/i3c/i3c_mcux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,13 @@ struct mcux_i3c_data {
bool has_mandatory_byte;
} ibi;
#endif
};

uint32_t merrwarn_reg;
static struct k_spinlock lock;
/** Copy of last errwarn from isr */
uint32_t merrwarn_reg;

/** Lock to serialize errnwarn access */
struct k_spinlock errwarn_lock;
};

/**
* @brief Read a register and test for bit matches with timeout.
Expand Down Expand Up @@ -240,19 +243,21 @@ static void mcux_i3c_interrupt_enable(I3C_Type *base, uint32_t mask)
*
* @retval errors reported or 0 if no errors.
*/
static uint32_t mcux_i3c_has_error(void)
static uint32_t mcux_i3c_has_error(struct mcux_i3c_data *data)
{
uint32_t ret = 0;

k_spinlock_key_t key = k_spin_lock(&lock);
k_spinlock_key_t key = k_spin_lock(&data->errwarn_lock);

if (merrwarn_reg) {
if (data->merrwarn_reg) {
/* Read and clear */
ret = merrwarn_reg;
merrwarn_reg = 0;
ret = data->merrwarn_reg;
data->merrwarn_reg = 0;
}

k_spin_unlock(&lock, key);
ret &= ~I3C_MERRWARN_TIMEOUT_MASK;

k_spin_unlock(&data->errwarn_lock, key);

return ret;
}
Expand Down Expand Up @@ -449,14 +454,12 @@ static inline void mcux_i3c_request_daa(I3C_Type *base)
*
* @param base Pointer to controller registers.
*/
static inline void mcux_i3c_request_auto_ibi(I3C_Type *base)
static inline int mcux_i3c_request_auto_ibi(I3C_Type *base)
{
reg32_update(&base->MCTRL,
I3C_MCTRL_REQUEST_MASK | I3C_MCTRL_IBIRESP_MASK | I3C_MCTRL_RDTERM_MASK,
I3C_MCTRL_REQUEST_AUTO_IBI | I3C_MCTRL_IBIRESP_ACK_AUTO);
base->MCTRL = I3C_MCTRL_REQUEST_AUTO_IBI | I3C_MCTRL_IBIRESP_ACK_AUTO;

/* AUTO_IBI should result in IBIWON bit being set in status */
mcux_i3c_status_wait_clear(base, I3C_MSTATUS_IBIWON_MASK);
return mcux_i3c_status_wait_timeout(base, I3C_MSTATUS_IBIWON_MASK, 100);
}

/**
Expand Down Expand Up @@ -541,8 +544,8 @@ static inline void mcux_i3c_wait_idle(struct mcux_i3c_data *dev_data, I3C_Type *
*
* @return 0 if successful, or negative if error.
*/
static int mcux_i3c_request_emit_start(I3C_Type *base, uint8_t addr, bool is_i2c,
bool is_read, size_t read_sz)
static int mcux_i3c_request_emit_start(struct mcux_i3c_data *dev_data, I3C_Type *base, uint8_t addr,
bool is_i2c, bool is_read, size_t read_sz)
{
uint32_t mctrl;
int ret = 0;
Expand All @@ -568,7 +571,7 @@ static int mcux_i3c_request_emit_start(I3C_Type *base, uint8_t addr, bool is_i2c
1000);
if (ret == 0) {
/* Check for NACK */
if (mcux_i3c_has_error() & I3C_MERRWARN_NACK_MASK) {
if (mcux_i3c_has_error(dev_data) & I3C_MERRWARN_NACK_MASK) {
ret = -ENODEV;
}
}
Expand All @@ -586,7 +589,8 @@ static int mcux_i3c_request_emit_start(I3C_Type *base, uint8_t addr, bool is_i2c
* @param wait_stop True if need to wait for controller to be
* no longer in NORMACT.
*/
static inline int mcux_i3c_do_request_emit_stop(I3C_Type *base, bool wait_stop)
static inline int mcux_i3c_do_request_emit_stop(struct mcux_i3c_data *dev_data, I3C_Type *base,
bool wait_stop)
{
uint32_t merrwarn;

Expand All @@ -607,7 +611,7 @@ static inline int mcux_i3c_do_request_emit_stop(I3C_Type *base, bool wait_stop)
*/
while (reg32_test_match(&base->MSTATUS, I3C_MSTATUS_STATE_MASK,
I3C_MSTATUS_STATE_NORMACT)) {
merrwarn = mcux_i3c_has_error();
merrwarn = mcux_i3c_has_error(dev_data);
if (merrwarn) {
/*
* A timeout error has been observed on
Expand Down Expand Up @@ -650,7 +654,7 @@ static inline void mcux_i3c_request_emit_stop(struct mcux_i3c_data *dev_data,
* it so any error as a result of emitting the stop
* itself doesn't get incorrectly mixed together.
*/
if (mcux_i3c_has_error()) {
if (mcux_i3c_has_error(dev_data)) {
mcux_i3c_errwarn_clear_all_nowait(base);
}

Expand All @@ -662,7 +666,7 @@ static inline void mcux_i3c_request_emit_stop(struct mcux_i3c_data *dev_data,

retries = 0;
while (1) {
int err = mcux_i3c_do_request_emit_stop(base, wait_stop);
int err = mcux_i3c_do_request_emit_stop(dev_data, base, wait_stop);

if (err) {
if ((err == -ETIMEDOUT) && (++retries <= I3C_MAX_STOP_RETRIES)) {
Expand Down Expand Up @@ -691,27 +695,23 @@ static inline void mcux_i3c_request_emit_stop(struct mcux_i3c_data *dev_data,
*
* @param base Pointer to controller registers.
*/
static inline void mcux_i3c_ibi_respond_nack(I3C_Type *base)
static inline int mcux_i3c_ibi_respond_nack(I3C_Type *base)
{
reg32_update(&base->MCTRL,
I3C_MCTRL_REQUEST_MASK | I3C_MCTRL_IBIRESP_MASK,
I3C_MCTRL_REQUEST_IBI_ACK_NACK | I3C_MCTRL_IBIRESP_NACK);
base->MCTRL = I3C_MCTRL_REQUEST_IBI_ACK_NACK | I3C_MCTRL_IBIRESP_NACK;

mcux_i3c_status_wait_clear(base, I3C_MSTATUS_MCTRLDONE_MASK);
return mcux_i3c_status_wait_clear_timeout(base, I3C_MSTATUS_MCTRLDONE_MASK, 1000);
}

/**
* @brief Tell controller to ACK the incoming IBI.
*
* @param base Pointer to controller registers.
*/
static inline void mcux_i3c_ibi_respond_ack(I3C_Type *base)
static inline int mcux_i3c_ibi_respond_ack(I3C_Type *base)
{
reg32_update(&base->MCTRL,
I3C_MCTRL_REQUEST_MASK | I3C_MCTRL_IBIRESP_MASK,
I3C_MCTRL_REQUEST_IBI_ACK_NACK | I3C_MCTRL_IBIRESP_ACK);
base->MCTRL = I3C_MCTRL_REQUEST_IBI_ACK_NACK | I3C_MCTRL_IBIRESP_ACK;

mcux_i3c_status_wait_clear(base, I3C_MSTATUS_MCTRLDONE_MASK);
return mcux_i3c_status_wait_clear_timeout(base, I3C_MSTATUS_MCTRLDONE_MASK, 1000);
}

/**
Expand Down Expand Up @@ -817,10 +817,7 @@ static int mcux_i3c_recover_bus(const struct device *dev)
/* Exhaust all target initiated IBI */
while (mcux_i3c_status_is_set(base, I3C_MSTATUS_SLVSTART_MASK)) {
/* Tell the controller to perform auto IBI. */
mcux_i3c_request_auto_ibi(base);

if (mcux_i3c_status_wait_clear_timeout(base, I3C_MSTATUS_COMPLETE_MASK,
1000) == -ETIMEDOUT) {
if (mcux_i3c_request_auto_ibi(base) == -ETIMEDOUT) {
break;
}

Expand Down Expand Up @@ -887,7 +884,7 @@ static int mcux_i3c_do_one_xfer_read(I3C_Type *base, struct mcux_i3c_data *data,
/*
* If timed out, we abort the transaction.
*/
if ((mcux_i3c_has_error() & I3C_MERRWARN_TIMEOUT_MASK) || ret) {
if ((mcux_i3c_has_error(data) & I3C_MERRWARN_TIMEOUT_MASK) || ret) {
ret = -ETIMEDOUT;

/* for ibi, ignore timeout err if any bytes were
Expand Down Expand Up @@ -992,7 +989,7 @@ static int mcux_i3c_do_one_xfer(I3C_Type *base, struct mcux_i3c_data *data,

/* Emit START if so desired */
if (emit_start) {
ret = mcux_i3c_request_emit_start(base, addr, is_i2c, is_read, buf_sz);
ret = mcux_i3c_request_emit_start(data, base, addr, is_i2c, is_read, buf_sz);
if (ret != 0) {
emit_stop = true;

Expand All @@ -1001,6 +998,7 @@ static int mcux_i3c_do_one_xfer(I3C_Type *base, struct mcux_i3c_data *data,
}

if ((buf == NULL) || (buf_sz == 0)) {
emit_stop = true;
goto out_one_xfer;
}

Expand All @@ -1011,6 +1009,7 @@ static int mcux_i3c_do_one_xfer(I3C_Type *base, struct mcux_i3c_data *data,
}

if (ret < 0) {
emit_stop = true;
goto out_one_xfer;
}

Expand All @@ -1029,7 +1028,8 @@ static int mcux_i3c_do_one_xfer(I3C_Type *base, struct mcux_i3c_data *data,
}
}

if (mcux_i3c_has_error()) {
if (mcux_i3c_has_error(data)) {
emit_stop = true;
ret = -EIO;
}

Expand Down Expand Up @@ -1114,8 +1114,8 @@ static int mcux_i3c_transfer(const struct device *dev,
*/
if (!(msgs[i].flags & I3C_MSG_NBCH) && (send_broadcast)) {
while (1) {
ret = mcux_i3c_request_emit_start(base, I3C_BROADCAST_ADDR,
false, false, 0);
ret = mcux_i3c_request_emit_start(
dev_data, base, I3C_BROADCAST_ADDR, false, false, 0);
if (ret == -ENODEV) {
LOG_WRN("emit start of broadcast addr got NACK, maybe IBI");
/* wait for idle then try again */
Expand Down Expand Up @@ -1202,7 +1202,7 @@ static int mcux_i3c_do_daa(const struct device *dev)
do {
/* Loop to grab data from devices (Provisioned ID, BCR and DCR) */
do {
if (mcux_i3c_has_error()) {
if (mcux_i3c_has_error(data)) {
LOG_ERR("DAA recv error");

ret = -EIO;
Expand Down Expand Up @@ -1335,7 +1335,7 @@ static int mcux_i3c_do_ccc(const struct device *dev,
LOG_DBG("CCC[0x%02x]", payload->ccc.id);

/* Emit START */
ret = mcux_i3c_request_emit_start(base, I3C_BROADCAST_ADDR, false, false, 0);
ret = mcux_i3c_request_emit_start(data, base, I3C_BROADCAST_ADDR, false, false, 0);
if (ret < 0) {
LOG_ERR("CCC[0x%02x] %s START error (%d)",
payload->ccc.id,
Expand Down Expand Up @@ -1456,8 +1456,16 @@ static void mcux_i3c_ibi_work(struct k_work *work)
goto out_ibi_work;
};

/* IBIWON maybe set before request auto IBI causing it to return immediately
* Thus clear IBIWON before requesting AUTO IBI
*/
base->MSTATUS = I3C_MSTATUS_IBIWON_MASK;

/* Use auto IBI to service the IBI */
mcux_i3c_request_auto_ibi(base);
if (mcux_i3c_request_auto_ibi(base) == -ETIMEDOUT) {
mcux_i3c_request_emit_stop(data, base, true);
goto out_ibi_work;
}

mstatus = sys_read32((mem_addr_t)&base->MSTATUS);
ibiaddr = (mstatus & I3C_MSTATUS_IBIADDR_MASK) >> I3C_MSTATUS_IBIADDR_SHIFT;
Expand Down Expand Up @@ -1527,7 +1535,7 @@ static void mcux_i3c_ibi_work(struct k_work *work)
break;
}

if (mcux_i3c_has_error()) {
if (mcux_i3c_has_error(data)) {
/*
* If the controller detects any errors, simply
* emit a STOP to abort the IBI. The target will
Expand Down Expand Up @@ -1556,6 +1564,7 @@ static void mcux_i3c_ibi_work(struct k_work *work)
}
break;
case I3C_MSTATUS_IBITYPE_MR:
mcux_i3c_request_emit_stop(data, base, true);
break;
default:
break;
Expand Down Expand Up @@ -1807,7 +1816,7 @@ static void mcux_i3c_isr(const struct device *dev)
err = i3c_ibi_work_enqueue_cb(dev, mcux_i3c_ibi_work);
if (err) {
LOG_ERR("Error enqueuing ibi work, err %d", err);
base->MINTSET = I3C_MINTCLR_SLVSTART_MASK;
base->MINTSET = I3C_MINTSET_SLVSTART_MASK;
}
}
#endif
Expand All @@ -1825,7 +1834,7 @@ static void mcux_i3c_isr(const struct device *dev)
}

if (interrupt_enable & I3C_MSTATUS_ERRWARN_MASK) {
merrwarn_reg = base->MERRWARN;
dev_data->merrwarn_reg = base->MERRWARN;
base->MERRWARN = base->MERRWARN;
}
}
Expand Down Expand Up @@ -1990,16 +1999,18 @@ static int mcux_i3c_init(const struct device *dev)
}

/* Disable all interrupts except error interrupt */
base->MINTCLR = I3C_MINTCLR_SLVSTART_MASK |
I3C_MINTCLR_MCTRLDONE_MASK |
base->MINTCLR = I3C_MINTCLR_MCTRLDONE_MASK |
I3C_MINTCLR_COMPLETE_MASK |
I3C_MINTCLR_RXPEND_MASK |
I3C_MINTCLR_TXNOTFULL_MASK |
I3C_MINTCLR_IBIWON_MASK |
I3C_MINTCLR_NOWMASTER_MASK;

/* Enable error interrupt */
base->MINTSET = I3C_MSTATUS_ERRWARN_MASK;
base->MINTSET = I3C_MSTATUS_ERRWARN_MASK | I3C_MSTATUS_SLVSTART_MASK;

/* Configure interrupt */
config->irq_config_func(dev);

/* Just in case the bus is not in idle. */
ret = mcux_i3c_recover_bus(dev);
Expand All @@ -2008,9 +2019,6 @@ static int mcux_i3c_init(const struct device *dev)
goto err_out;
}

/* Configure interrupt */
config->irq_config_func(dev);

/* Perform bus initialization */
ret = i3c_bus_init(dev, &config->common.dev_list);

Expand Down
Loading