Skip to content

Use small lock to protect resources related to i2c. #15648

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

Merged
xiaoxiang781216 merged 1 commit into from
Jan 23, 2025
Merged

Use small lock to protect resources related to i2c. #15648

Show file tree
Hide file tree
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
18 changes: 13 additions & 5 deletions arch/arm/src/at32/at32_i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
#include <debug.h>

#include <nuttx/arch.h>
#include <nuttx/irq.h>
#include <nuttx/spinlock.h>
#include <nuttx/mutex.h>
#include <nuttx/semaphore.h>
#include <nuttx/kmalloc.h>
Expand Down Expand Up @@ -316,6 +316,7 @@ struct at32_i2c_priv_s

int refs; /* Reference count */
mutex_t lock; /* Mutual exclusion mutex */
spinlock_t spinlock; /* Spinlock */
#ifndef CONFIG_I2C_POLLED
sem_t sem_isr; /* Interrupt wait semaphore */
#endif
Expand Down Expand Up @@ -429,6 +430,7 @@ static struct at32_i2c_priv_s at32_i2c1_priv =
.config = &at32_i2c1_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -465,6 +467,7 @@ static struct at32_i2c_priv_s at32_i2c2_priv =
.config = &at32_i2c2_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -501,6 +504,7 @@ static struct at32_i2c_priv_s at32_i2c3_priv =
.config = &at32_i2c3_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -666,7 +670,7 @@ static inline int at32_i2c_sem_waitdone(struct at32_i2c_priv_s *priv)
irqstate_t flags;
int ret;

flags = enter_critical_section();
flags = spin_lock_irqsave(&priv->spinlock);

/* Enable I2C interrupts */

Expand All @@ -683,6 +687,8 @@ static inline int at32_i2c_sem_waitdone(struct at32_i2c_priv_s *priv)
priv->intstate = INTSTATE_WAITING;
do
{
spin_unlock_irqrestore(&priv->spinlock, flags);

/* Wait until either the transfer is complete or the timeout expires */

#ifdef CONFIG_AT32_I2C_DYNTIMEO
Expand All @@ -701,6 +707,8 @@ static inline int at32_i2c_sem_waitdone(struct at32_i2c_priv_s *priv)

break;
}

flags = spin_lock_irqsave(&priv->spinlock);
}

/* Loop until the interrupt level transfer is complete. */
Expand All @@ -715,7 +723,7 @@ static inline int at32_i2c_sem_waitdone(struct at32_i2c_priv_s *priv)

at32_i2c_modifyreg32(priv, AT32_I2C_CR1_OFFSET, I2C_CR1_ALLINTS, 0);

leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);
return ret;
}
#else
Expand Down Expand Up @@ -1616,7 +1624,7 @@ static int at32_i2c_isr_process(struct at32_i2c_priv_s *priv)
*/

#ifdef CONFIG_I2C_POLLED
irqstate_t state = enter_critical_section();
irqstate_t state = spin_lock_irqsave(&priv->spinlock);
#endif
/* Receive a byte */

Expand All @@ -1633,7 +1641,7 @@ static int at32_i2c_isr_process(struct at32_i2c_priv_s *priv)
priv->dcnt--;

#ifdef CONFIG_I2C_POLLED
leave_critical_section(state);
spin_unlock_irqrestore(&priv->spinlock, state);
#endif
}
else
Expand Down
18 changes: 11 additions & 7 deletions arch/arm/src/cxd56xx/cxd56_i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include <nuttx/mutex.h>
#include <nuttx/i2c/i2c_master.h>

#include <nuttx/irq.h>
#include <nuttx/spinlock.h>
#include <arch/board/board.h>

#include "chip.h"
Expand Down Expand Up @@ -84,6 +84,7 @@ struct cxd56_i2cdev_s
uint32_t base_freq; /* branch frequency */

mutex_t lock; /* Only one thread can access at a time */
spinlock_t spinlock; /* Spinlock */
sem_t wait; /* Place to wait for transfer completion */
struct wdog_s timeout; /* watchdog to timeout when bus hung */
uint32_t frequency; /* Current I2C frequency */
Expand All @@ -108,6 +109,7 @@ static struct cxd56_i2cdev_s g_i2c0dev =
.base = CXD56_SCU_I2C0_BASE,
.irqid = CXD56_IRQ_SCU_I2C0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.refs = 0,
};
Expand All @@ -119,6 +121,7 @@ static struct cxd56_i2cdev_s g_i2c1dev =
.base = CXD56_SCU_I2C1_BASE,
.irqid = CXD56_IRQ_SCU_I2C1,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.refs = 0,
};
Expand All @@ -130,6 +133,7 @@ static struct cxd56_i2cdev_s g_i2c2dev =
.base = CXD56_I2CM_BASE,
.irqid = CXD56_IRQ_I2CM,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.refs = 0,
};
Expand Down Expand Up @@ -358,11 +362,11 @@ static void cxd56_i2c_setfrequency(struct cxd56_i2cdev_s *priv,
static void cxd56_i2c_timeout(wdparm_t arg)
{
struct cxd56_i2cdev_s *priv = (struct cxd56_i2cdev_s *)arg;
irqstate_t flags = enter_critical_section();
irqstate_t flags = spin_lock_irqsave(&priv->spinlock);

priv->error = -ENODEV;
nxsem_post(&priv->wait);
leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);
}

/****************************************************************************
Expand Down Expand Up @@ -521,7 +525,7 @@ static int cxd56_i2c_receive(struct cxd56_i2cdev_s *priv, int last)
i2c_reg_write(priv, CXD56_IC_DATA_CMD, CMD_READ);
}

flags = enter_critical_section();
flags = spin_lock_irqsave(&priv->spinlock);
wd_start(&priv->timeout, I2C_TIMEOUT,
cxd56_i2c_timeout, (wdparm_t)priv);

Expand All @@ -530,7 +534,7 @@ static int cxd56_i2c_receive(struct cxd56_i2cdev_s *priv, int last)
i2c_reg_write(priv, CXD56_IC_DATA_CMD, CMD_READ | (en ? CMD_STOP : 0));

i2c_reg_rmw(priv, CXD56_IC_INTR_MASK, INTR_RX_FULL, INTR_RX_FULL);
leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);
nxsem_wait_uninterruptible(&priv->wait);

if (priv->error != OK)
Expand Down Expand Up @@ -567,7 +571,7 @@ static int cxd56_i2c_send(struct cxd56_i2cdev_s *priv, int last)

while (!(i2c_reg_read(priv, CXD56_IC_STATUS) & STATUS_TFNF));

flags = enter_critical_section();
flags = spin_lock_irqsave(&priv->spinlock);
wd_start(&priv->timeout, I2C_TIMEOUT,
cxd56_i2c_timeout, (wdparm_t)priv);
i2c_reg_write(priv, CXD56_IC_DATA_CMD,
Expand All @@ -576,7 +580,7 @@ static int cxd56_i2c_send(struct cxd56_i2cdev_s *priv, int last)
/* Enable TX_EMPTY interrupt for determine transfer done. */

i2c_reg_rmw(priv, CXD56_IC_INTR_MASK, INTR_TX_EMPTY, INTR_TX_EMPTY);
leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);

nxsem_wait_uninterruptible(&priv->wait);
return 0;
Expand Down
15 changes: 12 additions & 3 deletions arch/arm/src/gd32f4/gd32f4xx_i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
#include <debug.h>

#include <nuttx/arch.h>
#include <nuttx/irq.h>
#include <nuttx/spinlock.h>
#include <nuttx/clock.h>
#include <nuttx/mutex.h>
#include <nuttx/semaphore.h>
Expand Down Expand Up @@ -243,6 +243,7 @@ struct gd32_i2c_priv_s

int refs; /* Reference count */
mutex_t lock; /* Mutual exclusion mutex */
spinlock_t spinlock; /* Spinlock */
#ifndef CONFIG_I2C_POLLED
sem_t sem_isr; /* Interrupt wait semaphore */
#endif
Expand Down Expand Up @@ -378,6 +379,7 @@ static struct gd32_i2c_priv_s gd32_i2c0_priv =
.config = &gd32_i2c0_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -413,6 +415,7 @@ static struct gd32_i2c_priv_s gd32_i2c1_priv =
.config = &gd32_i2c1_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -448,6 +451,7 @@ static struct gd32_i2c_priv_s gd32_i2c2_priv =
.config = &gd32_i2c2_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -691,7 +695,7 @@ static inline int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv)
uint32_t regval;
int ret;

flags = enter_critical_section();
flags = spin_lock_irqsave(&priv->spinlock);

/* Enable I2C interrupts */

Expand All @@ -707,6 +711,8 @@ static inline int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv)
priv->intstate = INTSTATE_WAITING;
do
{
spin_unlock_irqrestore(&priv->spinlock, flags);

/* Wait until either the transfer is complete or the timeout expires */

#ifdef CONFIG_GD32F4_I2C_DYNTIMEO
Expand All @@ -725,6 +731,8 @@ static inline int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv)

break;
}

flags = spin_lock_irqsave(&priv->spinlock);
}

/* Loop until the interrupt level transfer is complete. */
Expand All @@ -741,7 +749,8 @@ static inline int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv)
regval &= ~I2C_CTL1_INTS_MASK;
gd32_i2c_putreg(priv, GD32_I2C_CTL1_OFFSET, regval);

leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);

return ret;
}
#else
Expand Down
11 changes: 8 additions & 3 deletions arch/arm/src/kinetis/kinetis_i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#include <nuttx/semaphore.h>
#include <nuttx/i2c/i2c_master.h>

#include <nuttx/irq.h>
#include <nuttx/spinlock.h>
#include <arch/board/board.h>

#include "chip.h"
Expand Down Expand Up @@ -116,6 +116,7 @@ struct kinetis_i2cdev_s
volatile uint8_t state; /* State of state machine */
bool restart; /* Should next transfer restart or not */
mutex_t lock; /* Only one thread can access at a time */
spinlock_t spinlock; /* Spinlock */
sem_t wait; /* Place to wait for state machine completion */
struct wdog_s timeout; /* watchdog to timeout when bus hung */
struct i2c_msg_s *msgs; /* Remaining transfers - first one is in
Expand Down Expand Up @@ -193,6 +194,7 @@ static struct kinetis_i2cdev_s g_i2c0_dev =
.config = &kinetis_i2c0_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.state = STATE_OK,
.msgs = NULL,
Expand All @@ -216,6 +218,7 @@ static struct kinetis_i2cdev_s g_i2c1_dev =
.config = &kinetis_i2c1_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.state = STATE_OK,
.msgs = NULL,
Expand All @@ -239,6 +242,7 @@ static struct kinetis_i2cdev_s g_i2c2_dev =
.config = &kinetis_i2c2_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.state = STATE_OK,
.msgs = NULL,
Expand All @@ -262,6 +266,7 @@ static struct kinetis_i2cdev_s g_i2c3_dev =
.config = &kinetis_i2c3_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
.wait = SEM_INITIALIZER(0),
.state = STATE_OK,
.msgs = NULL,
Expand Down Expand Up @@ -818,10 +823,10 @@ static void kinetis_i2c_timeout(wdparm_t arg)
DEBUGASSERT(priv != NULL);
i2cinfo("Timeout msg=%p\n", priv->msgs);

irqstate_t flags = enter_critical_section();
irqstate_t flags = spin_lock_irqsave(&priv->spinlock);
priv->state = STATE_TIMEOUT;
kinetis_i2c_endwait(priv);
leave_critical_section(flags);
spin_unlock_irqrestore(&priv->spinlock, flags);
}

/****************************************************************************
Expand Down
12 changes: 7 additions & 5 deletions arch/arm/src/lc823450/lc823450_i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#include <stddef.h>

#include <nuttx/arch.h>
#include <nuttx/irq.h>
#include <nuttx/spinlock.h>
#include <nuttx/kmalloc.h>
#include <nuttx/clock.h>
#include <nuttx/signal.h>
Expand Down Expand Up @@ -121,6 +121,7 @@ struct lc823450_i2c_priv_s

int refs; /* Reference count */
mutex_t lock; /* Mutual exclusion mutex */
spinlock_t spinlock; /* Spinlock */
#ifndef CONFIG_I2C_POLLED
sem_t sem_isr; /* Interrupt wait semaphore */
#endif
Expand Down Expand Up @@ -209,6 +210,7 @@ static struct lc823450_i2c_priv_s lc823450_i2c0_priv =
.config = &lc823450_i2c0_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -239,6 +241,7 @@ static struct lc823450_i2c_priv_s lc823450_i2c1_priv =
.config = &lc823450_i2c1_config,
.refs = 0,
.lock = NXMUTEX_INITIALIZER,
.spinlock = SP_UNLOCKED,
#ifndef CONFIG_I2C_POLLED
.sem_isr = SEM_INITIALIZER(0),
#endif
Expand Down Expand Up @@ -1005,7 +1008,7 @@ static int lc823450_i2c_transfer(struct i2c_master_s *dev,

if (lc823450_i2c_sem_waitdone(priv) < 0)
{
irqs = enter_critical_section();
irqs = spin_lock_irqsave(&priv->spinlock);

ret = -ETIMEDOUT;

Expand All @@ -1017,7 +1020,7 @@ static int lc823450_i2c_transfer(struct i2c_master_s *dev,

priv->timedout = true;

leave_critical_section(irqs);
spin_unlock_irqrestore(&priv->spinlock, irqs);

/* Wait for irq handler completion. 10msec wait is probably enough
* to terminate i2c transaction, NACK and STOP contition for read
Expand All @@ -1028,10 +1031,9 @@ static int lc823450_i2c_transfer(struct i2c_master_s *dev,
}
else
{
spin_unlock_irqrestore(&priv->spinlock, irqs);
i2cerr("No need of timeout handling. "
"It may be done in irq handler\n");

leave_critical_section(irqs);
}

#ifndef CONFIG_LC823450_IPL2
Expand Down
Loading