[PATCH v5 2/2] i2c: spacemit: introduce pio for k1
Troy Mitchell
troy.mitchell at linux.spacemit.com
Wed Jan 7 23:43:35 PST 2026
On Mon, Dec 29, 2025 at 04:07:15PM +0100, Aurelien Jarno wrote:
> Hi,
>
> On 2025-12-26 11:31, Troy Mitchell wrote: This patch introduces I2C PIO functionality for the Spacemit K1 SoC,
> > enabling the use of I2C in atomic context.
> >
> > When i2c xfer_atomic is invoked, use_pio is set accordingly.
> >
> > Since an atomic context is required, all interrupts are disabled when
> > operating in PIO mode. Even with interrupts disabled, the bits in the
> > ISR (Interrupt Status Register) will still be set, so error handling can
> > be performed by polling the relevant status bits in the ISR.
> >
> > Signed-off-by: Troy Mitchell <troy.mitchell at linux.spacemit.com>
> > ---
> > Changes in v5:
> > - optimize code logic
> > - refactor delay handling into spacemit_i2c_delay() helper
> > - introduce spacemit_i2c_complete() to centralize transfer completion
> > - rework PIO transfer wait logic for clarity and correctness
> > - modify and add some comments
> > - modify commit message
> > - Link to v4: https://lore.kernel.org/all/20251009-k1-i2c-atomic-v4-1-a89367870286@linux.spacemit.com/
> >
> > Changes in v4:
> > - refactor for better readability: simplify condition check and moving if/else (timeout/
> > wait_xfer_complete) logic into a function
> > - remove irrelevant changes
> > - remove the status clear call in spacemit_i2c_xfer_common()
> > - sort functions to avoid forward declarations,
> > move unavoidable ones above function definitions
> > - use udelay() in atomic context to avoid sleeping
> > - wait for MSD on the last byte in wait_pio_xfer()
> > - Link to v3: https://lore.kernel.org/r/20250929-k1-i2c-atomic-v3-1-f7e660c138b6@linux.spacemit.com
> >
> > Changes in v3:
> > - drop 1-5 patches (have been merged)
> > - modify commit message
> > - use readl_poll_timeout_atomic() in wait_pio_xfer()
> > - use msecs_to_jiffies() when get PIO mode timeout value
> > - factor out transfer state handling into spacemit_i2c_handle_state().
> > - do not disable/enable the controller IRQ around PIO transfers.
> > - consolidate spacemit_i2c_init() interrupt setup
> > - rename is_pio -> use_pio
> > - rename spacemit_i2c_xfer() -> spacemit_i2c_xfer_common()
> > - rename spacemit_i2c_int_xfer() -> spacemit_i2c_xfer()
> > - rename spacemit_i2c_pio_xfer() -> spacemit_i2c_pio_xfer_atomic()
> > - call spacemit_i2c_err_check() in wait_pio_xfer() when write last byte
> > - Link to v2: https://lore.kernel.org/r/20250925-k1-i2c-atomic-v2-0-46dc13311cda@linux.spacemit.com
> >
> > Changes in v2:
> > - add is_pio judgement in irq_handler()
> > - use a fixed timeout value when PIO
> > - use readl_poll_timeout() in spacemit_i2c_wait_bus_idle() when PIO
> > - Link to v1: https://lore.kernel.org/r/20250827-k1-i2c-atomic-v1-0-e59bea02d680@linux.spacemit.com
> > ---
> > drivers/i2c/busses/i2c-k1.c | 297 +++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 225 insertions(+), 72 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> > index accef6653b56bd3505770328af17e441fad613a7..78a2de2c517e51e6ff997cc21402eb8f85054f85 100644
> > --- a/drivers/i2c/busses/i2c-k1.c
> > +++ b/drivers/i2c/busses/i2c-k1.c
>
> ...
>
> > @@ -383,8 +424,134 @@ static void spacemit_i2c_err_check(struct spacemit_i2c_dev *i2c)
> >
> > spacemit_i2c_clear_int_status(i2c, SPACEMIT_I2C_INT_STATUS_MASK);
> >
> > - i2c->state = SPACEMIT_STATE_IDLE;
> > - complete(&i2c->complete);
> > + spacemit_i2c_complete(i2c);
> > +}
> > +
> > +static void spacemit_i2c_handle_state(struct spacemit_i2c_dev *i2c)
> > +{
> > + u32 val;
> > +
> > + if (i2c->status & SPACEMIT_SR_ERR)
> > + goto err_out;
> > +
> > + val = readl(i2c->base + SPACEMIT_ICR);
> > + val &= ~(SPACEMIT_CR_TB | SPACEMIT_CR_ACKNAK | SPACEMIT_CR_STOP | SPACEMIT_CR_START);
> > +
> > + switch (i2c->state) {
> > + case SPACEMIT_STATE_START:
> > + spacemit_i2c_handle_start(i2c);
> > + break;
> > + case SPACEMIT_STATE_READ:
> > + spacemit_i2c_handle_read(i2c);
> > + break;
> > + case SPACEMIT_STATE_WRITE:
> > + spacemit_i2c_handle_write(i2c);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + if (i2c->state != SPACEMIT_STATE_IDLE) {
> > + val |= SPACEMIT_CR_TB;
> > + if (i2c->use_pio)
> > + val |= SPACEMIT_CR_ALDIE;
> > +
> > +
> > + if (spacemit_i2c_is_last_msg(i2c)) {
> > + /* trigger next byte with stop */
> > + val |= SPACEMIT_CR_STOP;
> > +
> > + if (i2c->read)
> > + val |= SPACEMIT_CR_ACKNAK;
> > + }
> > + writel(val, i2c->base + SPACEMIT_ICR);
> > + }
> > +
> > +err_out:
> > + spacemit_i2c_err_check(i2c);
> > +}
> > +
> > +/*
> > + * In PIO mode, this function is used as a replacement for
> > + * wait_for_completion_timeout(), whose return value indicates
> > + * the remaining time.
> > + *
> > + * We do not have a meaningful remaining-time value here, so
> > + * return a non-zero value on success to indicate "not timed out".
> > + * Returning 1 ensures callers treating the return value as
> > + * time_left will not incorrectly report a timeout.
> > + */
> > +static int spacemit_i2c_wait_pio_xfer(struct spacemit_i2c_dev *i2c)
> > +{
> > + u32 mask, msec = jiffies_to_msecs(i2c->adapt.timeout);
> > + ktime_t timeout = ktime_add_ms(ktime_get(), msec);
> > + int ret;
> > +
> > + mask = SPACEMIT_SR_IRF | SPACEMIT_SR_ITE;
> > +
> > + do {
> > + i2c->status = readl(i2c->base + SPACEMIT_ISR);
> > +
> > + spacemit_i2c_clear_int_status(i2c, i2c->status);
>
> Do we actually need to clear the interrupt status even if none of above
> bits are set? Said otherwise, can we move this line after the if and
> before the handle_state?
No, if other bits is pending, we need to clear them here.
>
> > +
> > + if (!(i2c->status & mask)) {
> > + udelay(10);
>
> It seems that the poll delay elsewhere in this patch is 30 µs.
> Maybe use a consistent value.
>
> > + continue;
> > + }
> > +
> > + spacemit_i2c_handle_state(i2c);
> > +
> > +
>
> Please delete the extra blank lines here.
>
> > + } while (i2c->unprocessed && ktime_compare(ktime_get(), timeout) < 0);
> > +
>
> Otherwise it sounds good, thanks for the changes.
Thanks.
- Troy
>
> Regards
> Aurelien
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurelien at aurel32.net http://aurel32.net
>
More information about the linux-riscv
mailing list