On Tue, Jun 23, 2026 at 05:14:16PM +0530, Shubham Patil wrote:
> From: Manikanta Guntupalli <[email protected]>
>
> Add an I3C master driver and maintainers fragment for the AMD I3C bus
> controller.
>
> The driver currently supports the I3C bus operating in SDR mode,
> with features including Dynamic Address Assignment, private data
> transfers, and CCC transfers in both broadcast and direct modes. It
> also supports operation in I2C mode.
>
> The controller's data FIFOs are accessed big-endian; the driver performs
> this conversion locally using ioread32be()/iowrite32be() with the
> helpers, so it does not depend on any core FIFO-endianness helpers.
>
> Signed-off-by: Manikanta Guntupalli <[email protected]>
> Co-developed-by: Shubhrajyoti Datta <[email protected]>
> Signed-off-by: Shubhrajyoti Datta <[email protected]>
> Co-developed-by: Shubham Patil <[email protected]>
> Signed-off-by: Shubham Patil <[email protected]>
> ---
...
> +#define XI3C_REV_NUM_MASK                    GENMASK(15, 8)
> +#define XI3C_PID1_MASK                               GENMASK(15, 0)
> +#define XI3C_FIFO_LEVEL_MASK                 GENMASK(15, 0)
> +#define XI3C_RESP_CODE_MASK                  GENMASK(8, 5)
> +#define XI3C_RESP_CODE_SUCCESS                       0       /* Transfer 
> completed OK */
> +#define XI3C_RESP_CODE_NO_TARGET             2       /* 7E NACK: no target 
> on bus */
> +#define XI3C_RESP_CODE_NACK                  3       /* Target NACK / CE2 / 
> DAA end */
> +#define XI3C_ADDR_MASK                               GENMASK(6, 0)
> +#define XI3C_FIFOS_RST_MASK                  GENMASK(4, 1)
> +
> +/* Command FIFO word layout (bit ranges encoded in the GENMASK/BIT args) */
> +#define XI3C_CMD_TYPE                                GENMASK(3, 0)   /* 
> command type */
> +#define XI3C_CMD_TERMINATE                   BIT(4)          /* terminate 
> (last cmd of xfer) */
> +#define XI3C_CMD_ADDR                                GENMASK(15, 8)  /* 
> target address << 1 | RnW */
> +#define XI3C_CMD_LEN                         GENMASK(27, 16) /* payload 
> length in bytes */
> +#define XI3C_CMD_TID                         GENMASK(31, 28) /* transfer ID 
> */
> +
> +#define XI3C_OD_TLOW_NS                              500000
> +#define XI3C_OD_THIGH_NS                     41000
> +#define XI3C_I2C_TCASMIN_NS                  600000
> +#define XI3C_TCASMIN_NS                              260000
> +#define XI3C_MAXDATA_LENGTH                  4095
> +#define XI3C_MAX_DEVS                                32
> +#define XI3C_DAA_SLAVEINFO_READ_BYTECOUNT    8
> +
> +#define XI3C_THOLD_MIN_REV0                  5       /* Min SDA hold cycles, 
> rev 0 IP */
> +#define XI3C_THOLD_MIN_REV1                  6       /* Min SDA hold cycles, 
> rev >= 1 IP */
> +#define XI3C_CYCLE_ADJUST                    2       /* SCL/SDA pre-bias for 
> HW pipeline */
> +#define XI3C_FIFO_RESET_DELAY_US             10      /* HW settling time 
> after FIFO reset */
> +#define XI3C_POLL_INTERVAL_US                        10      /* 
> readl_poll_timeout() sleep slice */

Can you provide comment where these value come from, spec, datasheet ...?

> +
> +#define XI3C_I2C_MODE                                0
> +#define XI3C_I2C_TID                         0
> +#define XI3C_SDR_MODE                                1
> +#define XI3C_SDR_TID                         1
> +
> +#define XI3C_WORD_LEN                                4
> +
> +/*
> + * XI3C_RESP_TIMEOUT_US is in microseconds because it is passed as the
> + * timeout_us argument of readl_poll_timeout(). XI3C_XFER_TIMEOUT_MS is in
> + * milliseconds because it feeds msecs_to_jiffies(). Keep the two units
> + * distinct in the names so callers cannot mix them up.
> + */
> +#define XI3C_RESP_TIMEOUT_US                 500000
> +#define XI3C_XFER_TIMEOUT_MS                 1000

the same here.

> +
> +struct xi3c_cmd {
> +     const void *tx_buf;
> +     void *rx_buf;
> +     u16 tx_len;
> +     u16 rx_len;
> +     u8 addr;
> +     u8 type;
> +     u8 tid;
> +     bool rnw;
> +     bool is_daa;
> +     bool continued;
> +     enum i3c_error_code err;
> +};
> +
...
> +
> +static void xi3c_master_reset_fifos(struct xi3c_master *master)
> +{
> +     u32 data;
> +
> +     /* Assert FIFO reset. */
> +     data = ioread32(master->membase + XI3C_RESET_OFFSET);
> +     data |= XI3C_FIFOS_RST_MASK;
> +     iowrite32(data, master->membase + XI3C_RESET_OFFSET);
> +     /* Read-back flushes the posted write before the settling delay below. 
> */
> +     ioread32(master->membase + XI3C_RESET_OFFSET);
> +     udelay(XI3C_FIFO_RESET_DELAY_US);

now suggest use fsleep()

> +
> +     /* De-assert FIFO reset, then wait for the FIFOs to come back up. */
> +     data &= ~XI3C_FIFOS_RST_MASK;
> +     iowrite32(data, master->membase + XI3C_RESET_OFFSET);
> +     ioread32(master->membase + XI3C_RESET_OFFSET);
> +     udelay(XI3C_FIFO_RESET_DELAY_US);
> +}
> +
> +static inline void xi3c_master_init(struct xi3c_master *master)
> +{
> +     /* Reset fifos */
> +     xi3c_master_reset_fifos(master);
> +
> +     /* Enable controller */
> +     xi3c_master_enable(master);
> +}
> +
> +static inline void xi3c_master_reinit(struct xi3c_master *master)
> +{
> +     /* Reset fifos */
> +     xi3c_master_reset_fifos(master);
> +
> +     /* Resume controller */
> +     xi3c_master_resume(master);
> +}
> +
> +static struct xi3c_xfer *xi3c_master_alloc_xfer(unsigned int ncmds)
> +{
> +     struct xi3c_xfer *xfer;
> +
> +     xfer = kzalloc(struct_size(xfer, cmds, ncmds), GFP_KERNEL);

ues new API,  kzalloc_flex()

> +     if (!xfer)
> +             return NULL;
> +
> +     xfer->ncmds = ncmds;
> +
> +     return xfer;
> +}
> +
> +static void xi3c_master_rd_from_rx_fifo(struct xi3c_master *master,
> +                                     struct xi3c_cmd *cmd)
> +{
> +     u16 rx_data_available;
> +     u16 copy_len;
> +     u16 len;
> +
> +     rx_data_available = xi3c_rd_fifo_level(master);
> +     len = rx_data_available * XI3C_WORD_LEN;
> +
> +     if (!len)
> +             return;
> +
> +     copy_len = min_t(u16, len, cmd->rx_len);

now need't min_t, just min

> +     xi3c_readl_fifo(master->membase + XI3C_RD_FIFO_OFFSET,
> +                     (u8 *)cmd->rx_buf, copy_len);
> +
> +     cmd->rx_buf = (u8 *)cmd->rx_buf + copy_len;
> +     cmd->rx_len -= copy_len;
> +}
> +
...
> +
> +     timeout = jiffies + msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS);
> +
> +     /* Read data from rx fifo */
> +     while (cmd->rx_len > 0 && !xi3c_is_resp_available(master)) {
> +             if (time_after(jiffies, timeout)) {
> +                     dev_err(master->dev, "XI3C read timeout\n");
> +                     return -EIO;
> +             }
> +             xi3c_master_rd_from_rx_fifo(master, cmd);
> +             usleep_range(XI3C_POLL_INTERVAL_US, 2 * XI3C_POLL_INTERVAL_US);
> +     }

can you use read_poll_timeout macro?

> +
> +     /* Read remaining data */
> +     xi3c_master_rd_from_rx_fifo(master, cmd);
> +
> +     return 0;
> +}
> +
...
> +
> +     for (i = 0; i < master->daa.index; i++) {
> +             u64 pid;
> +
> +             ret = i3c_master_add_i3c_dev_locked(m, master->daa.addrs[i]);
> +             if (ret)
> +                     goto err_daa;


https://lore.kernel.org/linux-i3c/[email protected]/T/#u
which defer add i3c device.

And don't check error here, because one device add failure should not impact
other following devices.

Frank
> +
> +             pid = FIELD_GET(XI3C_PID_MASK,
> +                             get_unaligned_be64(pid_bufs[i]));
> +             dev_dbg(master->dev, "Client %d: PID: 0x%llx\n", i, pid);
> +     }
> +
> +     return 0;
> +
> +err_daa:
> +     xi3c_master_reinit(master);
> +     return ret;
> +}
> +
...
> +static int xi3c_master_send_bdcast_ccc_cmd(struct xi3c_master *master,
> +                                        struct i3c_ccc_cmd *ccc)
> +{
> +     struct xi3c_xfer *xfer __free(kfree) = NULL;
> +     u8 *buf __free(kfree) = NULL;
> +     struct xi3c_cmd *cmd;
> +     u16 xfer_len;
> +     int ret;
> +
> +     if (ccc->dests[0].payload.len >= XI3C_MAXDATA_LENGTH)
> +             return -EINVAL;
> +
> +     xfer_len = ccc->dests[0].payload.len + 1;
> +
> +     xfer = xi3c_master_alloc_xfer(1);
> +     if (!xfer)
> +             return -ENOMEM;
> +
> +     buf = kmalloc(xfer_len, GFP_KERNEL);

kmalloc_obj

Frank
> +     if (!buf)
> +             return -ENOMEM;
> +
> +     buf[0] = ccc->id;
> +     memcpy(&buf[1], ccc->dests[0].payload.data, ccc->dests[0].payload.len);
> +
> +     cmd = &xfer->cmds[0];
> +     cmd->addr = ccc->dests[0].addr;
> +     cmd->rnw = ccc->rnw;
> +     cmd->tx_buf = buf;
> +     cmd->tx_len = xfer_len;
> +     cmd->type = XI3C_SDR_MODE;
> +     cmd->tid = XI3C_SDR_TID;
> +     cmd->continued = false;
> +
> +     ret = xi3c_master_common_xfer(master, xfer);
> +     ccc->err = cmd->err;
> +
> +     return ret;
> +}

Reply via email to