lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <IA1PR12MB6092043688B7D23925AE34978C00A@IA1PR12MB6092.namprd12.prod.outlook.com>
Date: Thu, 4 Sep 2025 10:09:24 +0000
From: "Guntupalli, Manikanta" <manikanta.guntupalli@....com>
To: Frank Li <Frank.li@....com>
CC: "git (AMD-Xilinx)" <git@....com>, "Simek, Michal" <michal.simek@....com>,
	"alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"kees@...nel.org" <kees@...nel.org>, "gustavoars@...nel.org"
	<gustavoars@...nel.org>, "jarkko.nikula@...ux.intel.com"
	<jarkko.nikula@...ux.intel.com>, "linux-i3c@...ts.infradead.org"
	<linux-i3c@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "Pandey, Radhey Shyam"
	<radhey.shyam.pandey@....com>, "Goud, Srinivas" <srinivas.goud@....com>,
	"Datta, Shubhrajyoti" <shubhrajyoti.datta@....com>, "manion05gk@...il.com"
	<manion05gk@...il.com>
Subject: RE: [PATCH V2 2/2] i3c: master: Add AMD I3C bus controller driver

[Public]

Hi,

> -----Original Message-----
> From: Frank Li <Frank.li@....com>
> Sent: Wednesday, September 3, 2025 7:58 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@....com>
> Cc: git (AMD-Xilinx) <git@....com>; Simek, Michal <michal.simek@....com>;
> alexandre.belloni@...tlin.com; robh@...nel.org; krzk+dt@...nel.org;
> conor+dt@...nel.org; kees@...nel.org; gustavoars@...nel.org;
> jarkko.nikula@...ux.intel.com; linux-i3c@...ts.infradead.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> hardening@...r.kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@....com>; Goud, Srinivas <srinivas.goud@....com>;
> Datta, Shubhrajyoti <shubhrajyoti.datta@....com>; manion05gk@...il.com
> Subject: Re: [PATCH V2 2/2] i3c: master: Add AMD I3C bus controller driver
>
> On Fri, Aug 29, 2025 at 10:43:27PM +0530, Manikanta Guntupalli wrote:
> > Add an I3C master driver and maintainers fragment for the AMD I3C bus
> > controller.
> >
> > The driver currently supports the I3C bus operating in SDR i3c 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.
> >
> > Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@....com>
> > ---
> > Changes for V2:
> > Updated commit description.
> > Added mixed mode support with clock configuration.
> > Converted smaller functions into inline functions.
> > Used FIELD_GET() in xi3c_get_response().
> > Updated xi3c_master_rd_from_rx_fifo() to use cmd->rx_buf.
> > Used parity8() for address parity calculation.
> > Added guards for locks.
> > Dropped num_targets and updated xi3c_master_do_daa().
> > Used __free(kfree) in xi3c_master_send_bdcast_ccc_cmd().
> > Dropped PM runtime support.
> > Updated xi3c_master_read() and xi3c_master_write() with
> > xi3c_is_resp_available() check.
> > Created separate functions: xi3c_master_init() and xi3c_master_reinit().
> > Used xi3c_master_init() in bus initialization and xi3c_master_reinit()
> > in error paths.
> > Added DAA structure to xi3c_master structure.
> > ---
> >  MAINTAINERS                         |    7 +
> >  drivers/i3c/master/Kconfig          |   16 +
> >  drivers/i3c/master/Makefile         |    1 +
> >  drivers/i3c/master/amd-i3c-master.c | 1009 +++++++++++++++++++++++++++
> >  4 files changed, 1033 insertions(+)
> >  create mode 100644 drivers/i3c/master/amd-i3c-master.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0cb9e55021cb..28ee1554cb0b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11416,6 +11416,13 @@ L: linux-i2c@...r.kernel.org
> >  S: Maintained
> >  F: drivers/i2c/i2c-stub.c
> >
> > +I3C DRIVER FOR AMD AXI I3C MASTER
> > +M: Manikanta Guntupalli <manikanta.guntupalli@....com>
> > +R: Michal Simek <michal.simek@....com>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/i3c/xlnx,axi-i3c.yaml
> > +F: drivers/i3c/master/amd-i3c-master.c
> > +
> >  I3C DRIVER FOR ASPEED AST2600
> >  M: Jeremy Kerr <jk@...econstruct.com.au>
> >  S: Maintained
> > diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> > index 77da199c7413..a77d288d2464 100644
> > --- a/drivers/i3c/master/Kconfig
> > +++ b/drivers/i3c/master/Kconfig
> > @@ -1,4 +1,20 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > +
> > +config AMD_I3C_MASTER
> > +   tristate "AMD I3C Master driver"
> > +   depends on I3C
> > +   depends on HAS_IOMEM
> > +   help
> > +     Support for AMD I3C Master Controller.
> > +
> > +     This driver allows communication with I3C devices using the AMD
> > +     I3C master, currently supporting Standard Data Rate (SDR) mode.
> > +     Features include Dynamic Address Assignment, private transfers,
> > +     and CCC transfers in both broadcast and direct modes.
> > +
> > +     This driver can also be built as a module.  If so, the module
> > +     will be called amd-i3c-master.
> > +
> >  config CDNS_I3C_MASTER
> >     tristate "Cadence I3C master driver"
> >     depends on I3C
> > diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> > index 3e97960160bc..c0167aa1f1b3 100644
> > --- a/drivers/i3c/master/Makefile
> > +++ b/drivers/i3c/master/Makefile
> > @@ -1,4 +1,5 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > +obj-$(CONFIG_AMD_I3C_MASTER)               += amd-i3c-master.o
> >  obj-$(CONFIG_CDNS_I3C_MASTER)              += i3c-master-cdns.o
> >  obj-$(CONFIG_DW_I3C_MASTER)                += dw-i3c-master.o
> >  obj-$(CONFIG_AST2600_I3C_MASTER)   += ast2600-i3c-master.o
> > diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-
> master.c
> > new file mode 100644
> > index 000000000000..f74febe4d17d
> > --- /dev/null
> > +++ b/drivers/i3c/master/amd-i3c-master.c
> > @@ -0,0 +1,1009 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * I3C master driver for the AMD I3C controller.
> > + *
> > + * Copyright (C) 2025, Advanced Micro Devices, Inc.
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/i3c/master.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/ioport.h>
> > +#include <linux/kernel.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/unaligned.h>
> > +
> > +#define XI3C_VERSION_OFFSET                        0x00    /* Version
> Register */
> > +#define XI3C_RESET_OFFSET                  0x04    /* Soft Reset Register */
> > +#define XI3C_CR_OFFSET                             0x08    /* Control
> Register */
> > +#define XI3C_ADDRESS_OFFSET                        0x0C    /* Target Address
> Register */
> > +#define XI3C_SR_OFFSET                             0x10    /* Status
> Register */
> > +#define XI3C_CMD_FIFO_OFFSET                       0x20    /* I3C Command
> FIFO Register */
> > +#define XI3C_WR_FIFO_OFFSET                        0x24    /* I3C Write Data
> FIFO Register */
> > +#define XI3C_RD_FIFO_OFFSET                        0x28    /* I3C Read Data
> FIFO Register */
> > +#define XI3C_RESP_STATUS_FIFO_OFFSET               0x2C    /* I3C Response
> status FIFO Register */
> > +#define XI3C_FIFO_LVL_STATUS_OFFSET                0x30    /* I3C CMD &
> WR FIFO LVL Register */
> > +#define XI3C_FIFO_LVL_STATUS_1_OFFSET              0x34    /* I3C RESP &
> RD FIFO LVL Register */
> > +#define XI3C_SCL_HIGH_TIME_OFFSET          0x38    /* I3C SCL HIGH
> Register */
> > +#define XI3C_SCL_LOW_TIME_OFFSET           0x3C    /* I3C SCL LOW
> Register */
> > +#define XI3C_SDA_HOLD_TIME_OFFSET          0x40    /* I3C SDA
> HOLD Register */
> > +#define XI3C_TSU_START_OFFSET                      0x48    /* I3C START
> SETUP Register  */
> > +#define XI3C_THD_START_OFFSET                      0x4C    /* I3C START
> HOLD Register */
> > +#define XI3C_TSU_STOP_OFFSET                       0x50    /* I3C STOP
> Setup Register  */
> > +#define XI3C_OD_SCL_HIGH_TIME_OFFSET               0x54    /* I3C OD SCL
> HIGH Register */
> > +#define XI3C_OD_SCL_LOW_TIME_OFFSET                0x58    /* I3C OD SCL
> LOW  Register */
> > +#define XI3C_PID0_OFFSET                   0x6C    /* LSB 4 bytes of the
> PID */
> > +#define XI3C_PID1_BCR_DCR                  0x70    /* MSB 2 bytes of the
> PID, BCR and DCR */
> > +
> > +#define XI3C_CR_EN_MASK                            BIT(0)  /* Core Enable */
> > +#define XI3C_CR_RESUME_MASK                        BIT(2)  /* Core Resume
> */
> > +#define XI3C_SR_RESP_NOT_EMPTY_MASK                BIT(4)  /* Resp Fifo not
> empty status mask */
> > +
> > +#define XI3C_BCR_MASK                              GENMASK(23, 16)
> > +#define XI3C_DCR_MASK                              GENMASK(31, 24)
> > +#define XI3C_PID_MASK                              GENMASK(63, 16)
> > +#define XI3C_SCL_HIGH_TIME_MASK                    GENMASK(17, 0)
> > +#define XI3C_SCL_LOW_TIME_MASK                     GENMASK(17, 0)
> > +#define XI3C_SDA_HOLD_TIME_MASK                    GENMASK(17, 0)
> > +#define XI3C_TSU_START_MASK                        GENMASK(17, 0)
> > +#define XI3C_THD_START_MASK                        GENMASK(17, 0)
> > +#define XI3C_TSU_STOP_MASK                 GENMASK(17, 0)
> > +#define XI3C_REV_NUM_MASK                  GENMASK(15, 8)
> > +#define XI3C_PID1_MASK                             GENMASK(15, 0)
> > +#define XI3C_WR_FIFO_LEVEL_MASK                    GENMASK(15, 0)
> > +#define XI3C_CMD_LEN_MASK                  GENMASK(11, 0)
> > +#define XI3C_RESP_CODE_MASK                        GENMASK(8, 5)
> > +#define XI3C_ADDR_MASK                             GENMASK(6, 0)
> > +#define XI3C_CMD_TYPE_MASK                 GENMASK(3, 0)
> > +#define XI3C_CMD_TID_MASK                  GENMASK(3, 0)
> > +#define XI3C_FIFOS_RST_MASK                        GENMASK(4, 1)
> > +
> > +#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_I2C_MODE                              0
> > +#define XI3C_I2C_TID                               0
> > +#define XI3C_SDR_MODE                              1
> > +#define XI3C_SDR_TID                               1
> > +
> > +#define XI3C_WORD_LEN                              4
> > +
> > +/* timeout waiting for the controller finish transfers */
> > +#define XI3C_XFER_TIMEOUT                  (msecs_to_jiffies(10000))
> > +#define XI3C_XFER_TIMEOUT_MS                       10000
>
> Only define one TIMEOUT should be enough
We will fix.
>
> s/XI3C_XFER_TIMEOUT/(msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS))
>
> > +
> > +#define xi3c_getrevisionnumber(master)                                             \
> > +   ((u8)(FIELD_GET(XI3C_REV_NUM_MASK,                                      \
> > +                   readl((master)->membase + XI3C_VERSION_OFFSET))))
> > +
> > +#define xi3c_wrfifolevel(master)                                           \
> > +   ((u16)(readl((master)->membase + XI3C_FIFO_LVL_STATUS_OFFSET) &
>               \
> > +          XI3C_WR_FIFO_LEVEL_MASK))
> > +
> > +#define xi3c_rdfifolevel(master)                                           \
> > +   ((u16)(readl((master)->membase + XI3C_FIFO_LVL_STATUS_1_OFFSET)
> &     \
> > +          XI3C_WR_FIFO_LEVEL_MASK))
> > +
> > +#define xi3c_is_resp_available(master)                                             \
> > +   ((u8)(FIELD_GET(XI3C_SR_RESP_NOT_EMPTY_MASK,
>       \
> > +                   readl((master)->membase + XI3C_SR_OFFSET))))
>
> Are you sure need type force convert at above macro?
Not needed, we will fix.
>
> > +
> > +struct xi3c_cmd {
> > +   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;
> > +};
> > +
> > +struct xi3c_xfer {
> > +   struct list_head node;
> > +   struct completion comp;
> > +   int ret;
> > +   unsigned int ncmds;
> > +   struct xi3c_cmd cmds[] __counted_by(ncmds);
> > +};
> > +
> > +/**
> > + * struct xi3c_master - I3C Master structure
> > + * @base: I3C master controller
> > + * @dev: Pointer to device structure
> > + * @xferqueue: Transfer queue structure
> > + * @xferqueue.list: List member
> > + * @xferqueue.cur: Current ongoing transfer
> > + * @xferqueue.lock: Queue lock
> > + * @membase: Memory base of the HW registers
> > + * @pclk: Input clock
> > + * @lock: Transfer lock
> > + * @daa.addrs: Slave addresses array
> > + * @daa.index: Slave device index
> > + */
> > +struct xi3c_master {
> > +   struct i3c_master_controller base;
> > +   struct device *dev;
> > +   struct {
> > +           struct list_head list;
> > +           struct xi3c_xfer *cur;
> > +           /* Queue lock */
> > +           spinlock_t lock;
> > +   } xferqueue;
> > +   void __iomem *membase;
> > +   struct clk *pclk;
> > +   /* Transfer lock */
> > +   struct mutex lock;
> > +   struct {
> > +           u8 addrs[XI3C_MAX_DEVS];
> > +           u8 index;
> > +   } daa;
> > +};
> > +
> > +static inline struct xi3c_master *
> > +to_xi3c_master(struct i3c_master_controller *master)
> > +{
> > +   return container_of(master, struct xi3c_master, base);
> > +}
> > +
> > +static int xi3c_get_response(struct xi3c_master *master)
> > +{
> > +   u32 resp_reg, response_data;
> > +   int ret;
> > +
> > +   ret = readl_poll_timeout(master->membase + XI3C_SR_OFFSET,
> > +                            resp_reg,
> > +                            resp_reg & XI3C_SR_RESP_NOT_EMPTY_MASK,
> > +                            0, XI3C_XFER_TIMEOUT_MS);
> > +   if (ret) {
> > +           dev_err(master->dev, "AXI I3C response timeout\n");
> > +           return ret;
> > +   }
> > +
> > +   response_data = readl(master->membase +
> XI3C_RESP_STATUS_FIFO_OFFSET);
> > +
> > +   /* Return response code */
> > +   return FIELD_GET(XI3C_RESP_CODE_MASK, response_data);
> > +}
> > +
> > +static void xi3c_master_wr_to_tx_fifo(struct xi3c_master *master,
> > +                                 struct xi3c_cmd *cmd)
> > +{
> > +   u8 *tx_buf = (u8 *)cmd->tx_buf;
> > +   u32 data = 0;
> > +   u16 len;
> > +
> > +   len = cmd->tx_len;
> > +   if (len > 0) {
> > +           len = (len >= XI3C_WORD_LEN) ? XI3C_WORD_LEN : len;
> > +           memcpy(&data, tx_buf, len);
> > +           tx_buf += len;
> > +           cmd->tx_len -= len;
> > +   }
> > +   cmd->tx_buf = tx_buf;
> > +
> > +   /* Write the 32-bit value to the Tx FIFO */
> > +   iowrite32be(data, master->membase + XI3C_WR_FIFO_OFFSET);
> > +}
> > +
> > +static void xi3c_master_rd_from_rx_fifo(struct xi3c_master *master,
> > +                                   struct xi3c_cmd *cmd)
> > +{
> > +   u32 data;
> > +   u16 len;
> > +
> > +   /* Read from Rx FIFO */
> > +   data = ioread32be(master->membase + XI3C_RD_FIFO_OFFSET);
> > +
> > +   /* Data extraction to rx buffer */
> > +   len = cmd->rx_len;
> > +   if (len > 0) {
> > +           len = (len >= XI3C_WORD_LEN) ? XI3C_WORD_LEN : len;
> > +           memcpy((u8 *)cmd->rx_buf, &data, len);
> > +           cmd->rx_buf = (u8 *)cmd->rx_buf + len;
> > +           cmd->rx_len -= len;
> > +   }
> > +}
>
> Check i3c_writel_fifo() and i3c_readl_fifo(), which do the similar thing.
i3c_writel_fifo() and i3c_readl_fifo() follow the native endianness of the CPU.
However, in this case the FIFO should always be handled in big-endian format.
Therefore, we cannot use i3c_writel_fifo() and i3c_readl_fifo() directly.
>
> > +
> > +static void xi3c_master_write_to_cmdfifo(struct xi3c_master *master,
> > +                                    struct xi3c_cmd *cmd, u16 len)
> > +{
> > +   u32 transfer_cmd = 0;
> > +   u8 addr;
> > +
> > +   addr = ((cmd->addr & XI3C_ADDR_MASK) << 1) | (cmd->rnw & BIT(0));
> > +
> > +   transfer_cmd = cmd->type & XI3C_CMD_TYPE_MASK;
> > +   transfer_cmd |= (u32)(!cmd->continued) << 4;
> > +   transfer_cmd |= (u32)(addr) << 8;
> > +   transfer_cmd |= (u32)(cmd->tid & XI3C_CMD_TID_MASK) << 28;
>
> Can you use GEN_MASK to avoid hardcode 4, 8, 28
We found the GEN_MASK macro and its usage only under arch/alpha/.
It does not appear to be a generic API available for other architectures.
>
> > +
> > +   /*
> > +    * For dynamic addressing, an additional 1-byte length must be added
> > +    * to the command FIFO to account for the address present in the TX FIFO
> > +    */
> > +   if (cmd->is_daa) {
> > +           xi3c_master_wr_to_tx_fifo(master, cmd);
> > +
> > +           len++;
> > +           master->daa.index++;
> > +           cmd->is_daa = false;
> > +   }
> > +
> > +   transfer_cmd |= (u32)(len & XI3C_CMD_LEN_MASK) << 16;
>
> And 16 here.
Same here.
>
> > +   writel(transfer_cmd, master->membase + XI3C_CMD_FIFO_OFFSET);
> > +}
> > +
> > +static inline void xi3c_master_enable(struct xi3c_master *master)
> > +{
> > +   writel(readl(master->membase + XI3C_CR_OFFSET) |
> XI3C_CR_EN_MASK,
> > +          master->membase + XI3C_CR_OFFSET);
> > +}
> > +
> > +static inline void xi3c_master_disable(struct xi3c_master *master)
> > +{
> > +   writel(readl(master->membase + XI3C_CR_OFFSET) &
> (~XI3C_CR_EN_MASK),
> > +          master->membase + XI3C_CR_OFFSET);
> > +}
> > +
> > +static inline void xi3c_master_resume(struct xi3c_master *master)
> > +{
> > +   writel(readl(master->membase + XI3C_CR_OFFSET) |
> XI3C_CR_RESUME_MASK,
> > +          master->membase + XI3C_CR_OFFSET);
> > +}
> > +
> > +static void xi3c_master_reset_fifos(struct xi3c_master *master)
> > +{
> > +   u32 data;
> > +
> > +   /* Reset fifos */
> > +   data = readl(master->membase + XI3C_RESET_OFFSET);
>
> you mix use readl and ioread32, can you keep consistence?
We will fix.
>
> > +   data |= XI3C_FIFOS_RST_MASK;
> > +   writel(data, master->membase + XI3C_RESET_OFFSET);
> > +   udelay(10);
>
> udelay() may not include MMIO operation, it may delay less than 10us.
> suggest readl() before udelay() to make sure value reach XI3C_RESET_OFFSET.
We will fix.
>
> > +   data &= ~XI3C_FIFOS_RST_MASK;
> > +   writel(data, master->membase + XI3C_RESET_OFFSET);
> > +   udelay(10);
> > +}
> > +
> > +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(struct xi3c_master *master, unsigned int ncmds)
> > +{
> > +   struct xi3c_xfer *xfer;
> > +
> > +   xfer = kzalloc(struct_size(xfer, cmds, ncmds), GFP_KERNEL);
> > +   if (!xfer)
> > +           return NULL;
> > +
> > +   INIT_LIST_HEAD(&xfer->node);
> > +   xfer->ncmds = ncmds;
> > +   xfer->ret = -ETIMEDOUT;
> > +
> > +   return xfer;
> > +}
> > +
> > +static inline void xi3c_master_free_xfer(struct xi3c_xfer *xfer)
> > +{
> > +   kfree(xfer);
> > +}
>
> why not direct call kfree()?
We will fix.
>
> > +
> > +static int xi3c_master_read(struct xi3c_master *master, struct xi3c_cmd *cmd)
> > +{
> > +   unsigned long timeout;
> > +   u16 rx_data_available;
> > +   u16 data_index;
> > +
> > +   if (!cmd->rx_buf || cmd->rx_len > XI3C_MAXDATA_LENGTH)
> > +           return -EINVAL;
> > +
> > +   /* Fill command fifo */
> > +   xi3c_master_write_to_cmdfifo(master, cmd, cmd->rx_len);
> > +
> > +   timeout = jiffies + XI3C_XFER_TIMEOUT;
> > +   /* 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;
> > +           }
> > +
> > +           rx_data_available = xi3c_rdfifolevel(master);
> > +           for (data_index = 0;
> > +                data_index < rx_data_available && cmd->rx_len > 0;
> > +                data_index++)
> > +                   xi3c_master_rd_from_rx_fifo(master, cmd);
> > +   }
> > +
> > +   /* Read remaining data */
> > +   rx_data_available = xi3c_rdfifolevel(master);
> > +   for (data_index = 0;
> > +        data_index < rx_data_available && cmd->rx_len > 0;
> > +        data_index++)
> > +           xi3c_master_rd_from_rx_fifo(master, cmd);
> > +   return 0;
> > +}
> > +
> > +static int xi3c_master_write(struct xi3c_master *master, struct xi3c_cmd *cmd)
> > +{
> > +   unsigned long timeout;
> > +   u16 wrfifo_space;
> > +   u16 space_index;
> > +   u16 len;
> > +
> > +   len = cmd->tx_len;
> > +   if (!cmd->tx_buf || cmd->tx_len > XI3C_MAXDATA_LENGTH)
> > +           return -EINVAL;
> > +
> > +   /* Fill Tx fifo */
> > +   wrfifo_space = xi3c_wrfifolevel(master);
> > +   for (space_index = 0; space_index < wrfifo_space && cmd->tx_len > 0;
> > +        space_index++)
> > +           xi3c_master_wr_to_tx_fifo(master, cmd);
>
> Does existed helper function i3c_writel_fifo() handle this?
As mentioned above, due to the endianness conflict with i3c_writel_fifo(),
it doesn't fit this case directly.
>
> > +
> > +   /* Write to command fifo */
> > +   xi3c_master_write_to_cmdfifo(master, cmd, len);
> > +
> > +   timeout = jiffies + XI3C_XFER_TIMEOUT;
> > +   /* Fill if any remaining data to tx fifo */
> > +   while (cmd->tx_len > 0 && !xi3c_is_resp_available(master)) {
> > +           if (time_after(jiffies, timeout)) {
> > +                   dev_err(master->dev, "XI3C write timeout\n");
> > +                   return -EIO;
> > +           }
> > +
> > +           wrfifo_space = xi3c_wrfifolevel(master);
> > +           for (space_index = 0; space_index < wrfifo_space && cmd->tx_len >
> 0;
> > +                space_index++)
> > +                   xi3c_master_wr_to_tx_fifo(master, cmd);
> > +   }
> > +   return 0;
> > +}
> > +
> > +static int xi3c_master_xfer(struct xi3c_master *master, struct xi3c_cmd *cmd)
> > +{
> > +   int ret;
> > +
> > +   if (cmd->rnw)
> > +           ret = xi3c_master_read(master, cmd);
> > +   else
> > +           ret = xi3c_master_write(master, cmd);
> > +
> > +   if (ret < 0)
> > +           goto err_xfer_out;
> > +
> > +   ret = xi3c_get_response(master);
> > +   if (ret)
> > +           goto err_xfer_out;
> > +
> > +   return 0;
> > +
> > +err_xfer_out:
> > +   xi3c_master_reinit(master);
> > +   return ret;
> > +}
> > +
> > +static void xi3c_master_dequeue_xfer_locked(struct xi3c_master *master,
> > +                                       struct xi3c_xfer *xfer)
> > +{
> > +   if (master->xferqueue.cur == xfer)
> > +           master->xferqueue.cur = NULL;
> > +   else
> > +           list_del_init(&xfer->node);
> > +}
> > +
> > +static void xi3c_master_dequeue_xfer(struct xi3c_master *master,
> > +                                struct xi3c_xfer *xfer)
> > +{
> > +   guard(spinlock_irqsave)(&master->xferqueue.lock);
>
> Add empty line here.
We will fix.
>
> > +   xi3c_master_dequeue_xfer_locked(master, xfer);
> > +}
> > +
> > +static void xi3c_master_start_xfer_locked(struct xi3c_master *master)
> > +{
> > +   struct xi3c_xfer *xfer = master->xferqueue.cur;
> > +   int ret = 0, i;
> > +
> > +   if (!xfer)
> > +           return;
> > +
> > +   for (i = 0; i < xfer->ncmds; i++) {
> > +           struct xi3c_cmd *cmd = &xfer->cmds[i];
> > +
> > +           ret = xi3c_master_xfer(master, cmd);
> > +           if (ret)
> > +                   break;
> > +   }
> > +
> > +   xfer->ret = ret;
> > +   complete(&xfer->comp);
> > +
> > +   xfer = list_first_entry_or_null(&master->xferqueue.list,
> > +                                   struct xi3c_xfer,
> > +                                   node);
> > +   if (xfer)
> > +           list_del_init(&xfer->node);
> > +
> > +   master->xferqueue.cur = xfer;
> > +   xi3c_master_start_xfer_locked(master);
> > +}
> > +
> > +static inline void xi3c_master_enqueue_xfer(struct xi3c_master *master,
> > +                                       struct xi3c_xfer *xfer)
> > +{
> > +   init_completion(&xfer->comp);
> > +   guard(spinlock_irqsave)(&master->xferqueue.lock);
> > +   if (master->xferqueue.cur) {
> > +           list_add_tail(&xfer->node, &master->xferqueue.list);
> > +   } else {
> > +           master->xferqueue.cur = xfer;
> > +           xi3c_master_start_xfer_locked(master);
> > +   }
> > +}
> > +
> > +static inline int xi3c_master_common_xfer(struct xi3c_master *master,
> > +                                     struct xi3c_xfer *xfer)
> > +{
> > +   int ret, timeout;
> > +
> > +   guard(mutex)(&master->lock);
> > +   xi3c_master_enqueue_xfer(master, xfer);
> > +   timeout = wait_for_completion_timeout(&xfer->comp,
> > +
> msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS));
> > +   if (!timeout)
> > +           ret = -ETIMEDOUT;
> > +   else
> > +           ret = xfer->ret;
> > +
> > +   if (ret)
> > +           xi3c_master_dequeue_xfer(master, xfer);
> > +
> > +   return ret;
> > +}
> > +
> > +static int xi3c_master_do_daa(struct i3c_master_controller *m)
> > +{
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +   struct xi3c_cmd *daa_cmd;
> > +   struct xi3c_xfer *xfer;
> > +   u8 pid_bufs[XI3C_MAX_DEVS][8];
> > +   u8 data, last_addr = 0;
> > +   int addr, ret, i;
> > +   u8 *pid_buf;
> > +
> > +   u64 *pid_bcr_dcr __free(kfree) = kcalloc(XI3C_MAX_DEVS, sizeof(u64),
> > +                                            GFP_KERNEL);
> > +   if (!pid_bcr_dcr)
> > +           return -ENOMEM;
> > +
> > +   xfer = xi3c_master_alloc_xfer(master, 1);
> > +   if (!xfer) {
> > +           ret = -ENOMEM;
> > +           goto err_daa_mem;
> > +   }
> > +
> > +   for (i = 0; i < XI3C_MAX_DEVS; i++) {
> > +           addr = i3c_master_get_free_addr(m, last_addr + 1);
> > +           if (addr < 0) {
> > +                   ret = -ENOSPC;
> > +                   goto err_daa;
> > +           }
> > +           master->daa.addrs[i] = (u8)addr;
> > +           last_addr = (u8)addr;
> > +   }
> > +
> > +   /* Fill ENTDAA CCC */
> > +   data = I3C_CCC_ENTDAA;
> > +   daa_cmd = &xfer->cmds[0];
> > +   daa_cmd->addr = I3C_BROADCAST_ADDR;
> > +   daa_cmd->rnw = 0;
> > +   daa_cmd->tx_buf = &data;
> > +   daa_cmd->tx_len = 1;
> > +   daa_cmd->type = XI3C_SDR_MODE;
> > +   daa_cmd->tid = XI3C_SDR_TID;
> > +   daa_cmd->continued = true;
> > +
> > +   ret = xi3c_master_common_xfer(master, xfer);
> > +   /* DAA always finishes with CE2_ERROR or NACK_RESP */
> > +   if (ret && ret != I3C_ERROR_M2) {
> > +           goto err_daa;
> > +   } else {
> > +           if (ret && ret == I3C_ERROR_M2) {
> > +                   ret = 0;
> > +                   goto err_daa;
> > +           }
> > +   }
> > +
> > +   master->daa.index = 0;
> > +
> > +   while (true) {
> > +           struct xi3c_cmd *cmd = &xfer->cmds[0];
> > +
> > +           pid_buf = pid_bufs[master->daa.index];
> > +           addr = (master->daa.addrs[master->daa.index] << 1) |
> > +                  (!parity8(master->daa.addrs[master->daa.index]));
> > +
> > +           cmd->tx_buf = (u8 *)&addr;
> > +           cmd->tx_len = 1;
> > +           cmd->addr = I3C_BROADCAST_ADDR;
> > +           cmd->rnw = 1;
> > +           cmd->rx_buf = pid_buf;
> > +           cmd->rx_len = XI3C_DAA_SLAVEINFO_READ_BYTECOUNT;
> > +           cmd->is_daa = true;
> > +           cmd->type = XI3C_SDR_MODE;
> > +           cmd->tid = XI3C_SDR_TID;
> > +           cmd->continued = true;
> > +
> > +           ret = xi3c_master_common_xfer(master, xfer);
> > +
> > +           /* DAA always finishes with CE2_ERROR or NACK_RESP */
> > +           if (ret && ret != I3C_ERROR_M2) {
> > +                   goto err_daa;
> > +           } else {
> > +                   if (ret && ret == I3C_ERROR_M2) {
> > +                           xi3c_master_resume(master);
> > +                           master->daa.index--;
> > +                           ret = 0;
> > +                           break;
> > +                   }
> > +           }
> > +   }
> > +
> > +   xi3c_master_free_xfer(xfer);
> > +
> > +   for (i = 0; i < master->daa.index; i++) {
> > +           ret = i3c_master_add_i3c_dev_locked(m, master->daa.addrs[i]);
> > +           if (ret)
> > +                   goto err_daa_mem;
>
> see comments in svc-master-i3c.c
>
> /*
>          * Register all devices who participated to the core
>          *
>          * If two devices (A and B) are detected in DAA and address 0xa is assigned to
>          * device A and 0xb to device B, a failure in i3c_master_add_i3c_dev_locked()
>          * for device A (addr: 0xa) could prevent device B (addr: 0xb) from being
>          * registered on the bus. The I3C stack might still consider 0xb a free
>          * address. If a subsequent Hotjoin occurs, 0xb might be assigned to Device A,
>          * causing both devices A and B to use the same address 0xb, violating the I3C
>          * specification.
>          *
>          * The return value for i3c_master_add_i3c_dev_locked() should not be
> checked
>          * because subsequent steps will scan the entire I3C bus, independent of
>          * whether i3c_master_add_i3c_dev_locked() returns success.
>          *
>          * If device A registration fails, there is still a chance to register device
>          * B. i3c_master_add_i3c_dev_locked() can reset DAA if a failure occurs while
>          * retrieving device information.
>          */
>
> I think it also fits your case.
Yes, that makes sense. We will skip checking the return value of i3c_master_add_i3c_dev_locked(),
since subsequent steps will rescan the entire bus regardless of the return status.
>
> > +
> > +           pid_bcr_dcr[i] = FIELD_GET(XI3C_PID_MASK,
> > +                                      get_unaligned_be64(pid_bufs[i]));
> > +           dev_info(master->dev, "Client %d: PID: 0x%llx\n", i, pid_bcr_dcr[i]);
> > +   }
> > +
> > +   return 0;
> > +
> > +err_daa:
> > +   xi3c_master_free_xfer(xfer);
> > +err_daa_mem:
> > +   xi3c_master_reinit(master);
> > +   return ret;
> > +}
> > +
> > +static bool
> > +xi3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
> > +                        const struct i3c_ccc_cmd *cmd)
> > +{
> > +   if (cmd->ndests > 1)
> > +           return false;
> > +
> > +   switch (cmd->id) {
> > +   case I3C_CCC_ENEC(true):
> > +   case I3C_CCC_ENEC(false):
> > +   case I3C_CCC_DISEC(true):
> > +   case I3C_CCC_DISEC(false):
> > +   case I3C_CCC_ENTAS(0, true):
> > +   case I3C_CCC_ENTAS(0, false):
> > +   case I3C_CCC_RSTDAA(true):
> > +   case I3C_CCC_RSTDAA(false):
> > +   case I3C_CCC_ENTDAA:
> > +   case I3C_CCC_SETMWL(true):
> > +   case I3C_CCC_SETMWL(false):
> > +   case I3C_CCC_SETMRL(true):
> > +   case I3C_CCC_SETMRL(false):
> > +   case I3C_CCC_ENTHDR(0):
> > +   case I3C_CCC_SETDASA:
> > +   case I3C_CCC_SETNEWDA:
> > +   case I3C_CCC_GETMWL:
> > +   case I3C_CCC_GETMRL:
> > +   case I3C_CCC_GETPID:
> > +   case I3C_CCC_GETBCR:
> > +   case I3C_CCC_GETDCR:
> > +   case I3C_CCC_GETSTATUS:
> > +   case I3C_CCC_GETMXDS:
> > +           return true;
> > +   default:
> > +           return false;
> > +   }
> > +}
> > +
> > +static int xi3c_master_send_bdcast_ccc_cmd(struct xi3c_master *master,
> > +                                      struct i3c_ccc_cmd *ccc)
> > +{
> > +   u16 xfer_len = ccc->dests[0].payload.len + 1;
> > +   struct xi3c_xfer *xfer;
> > +   struct xi3c_cmd *cmd;
> > +   int ret;
> > +
> > +   xfer = xi3c_master_alloc_xfer(master, 1);
> > +   if (!xfer)
> > +           return -ENOMEM;
> > +
> > +   u8 *buf __free(kfree) = kmalloc(xfer_len, GFP_KERNEL);
> > +   if (!buf) {
> > +           xi3c_master_free_xfer(xfer);
> > +           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);
> > +   xi3c_master_free_xfer(xfer);
> > +
> > +   return ret;
> > +}
> > +
> > +static int xi3c_master_send_direct_ccc_cmd(struct xi3c_master *master,
> > +                                      struct i3c_ccc_cmd *ccc)
> > +{
> > +   struct xi3c_xfer *xfer;
> > +   struct xi3c_cmd *cmd;
> > +   int ret;
> > +
> > +   xfer = xi3c_master_alloc_xfer(master, 2);
> > +   if (!xfer)
> > +           return -ENOMEM;
> > +
> > +   /* Broadcasted message */
> > +   cmd = &xfer->cmds[0];
> > +   cmd->addr = I3C_BROADCAST_ADDR;
> > +   cmd->rnw = 0;
> > +   cmd->tx_buf = &ccc->id;
> > +   cmd->tx_len = 1;
> > +   cmd->type = XI3C_SDR_MODE;
> > +   cmd->tid = XI3C_SDR_TID;
> > +   cmd->continued = true;
> > +
> > +   /* Directed message */
> > +   cmd = &xfer->cmds[1];
> > +   cmd->addr = ccc->dests[0].addr;
> > +   cmd->rnw = ccc->rnw;
> > +   if (cmd->rnw) {
> > +           cmd->rx_buf = ccc->dests[0].payload.data;
> > +           cmd->rx_len = ccc->dests[0].payload.len;
> > +   } else {
> > +           cmd->tx_buf = ccc->dests[0].payload.data;
> > +           cmd->tx_len = ccc->dests[0].payload.len;
> > +   }
> > +   cmd->type = XI3C_SDR_MODE;
> > +   cmd->tid = XI3C_SDR_TID;
> > +   cmd->continued = false;
> > +
> > +   ret = xi3c_master_common_xfer(master, xfer);
> > +   xi3c_master_free_xfer(xfer);
> > +   return ret;
> > +}
> > +
> > +static int xi3c_master_send_ccc_cmd(struct i3c_master_controller *m,
> > +                               struct i3c_ccc_cmd *cmd)
> > +{
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +   bool broadcast = cmd->id < 0x80;
> > +
> > +   if (broadcast)
> > +           return xi3c_master_send_bdcast_ccc_cmd(master, cmd);
> > +
> > +   return xi3c_master_send_direct_ccc_cmd(master, cmd);
> > +}
> > +
> > +static int xi3c_master_priv_xfers(struct i3c_dev_desc *dev,
> > +                             struct i3c_priv_xfer *xfers,
> > +                             int nxfers)
> > +{
> > +   struct i3c_master_controller *m = i3c_dev_get_master(dev);
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +   struct xi3c_xfer *xfer;
> > +   int i, ret;
> > +
> > +   if (!nxfers)
> > +           return 0;
> > +
> > +   xfer = xi3c_master_alloc_xfer(master, nxfers);
> > +   if (!xfer)
> > +           return -ENOMEM;
> > +
> > +   for (i = 0; i < nxfers; i++) {
> > +           struct xi3c_cmd *cmd = &xfer->cmds[i];
> > +
> > +           cmd->addr = dev->info.dyn_addr;
> > +           cmd->rnw = xfers[i].rnw;
> > +
> > +           if (cmd->rnw) {
> > +                   cmd->rx_buf = xfers[i].data.in;
> > +                   cmd->rx_len = xfers[i].len;
> > +           } else {
> > +                   cmd->tx_buf = (void *)xfers[i].data.out;
> > +                   cmd->tx_len = xfers[i].len;
> > +           }
> > +
> > +           cmd->type = XI3C_SDR_MODE;
> > +           cmd->tid = XI3C_SDR_TID;
> > +           cmd->continued = (i + 1) < nxfers;
> > +   }
> > +
> > +   ret = xi3c_master_common_xfer(master, xfer);
> > +   xi3c_master_free_xfer(xfer);
> > +   return ret;
> > +}
> > +
> > +static int xi3c_master_i2c_xfers(struct i2c_dev_desc *dev,
> > +                            struct i2c_msg *xfers,
> > +                            int nxfers)
> > +{
> > +   struct i3c_master_controller *m = i2c_dev_get_master(dev);
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +   struct xi3c_xfer *xfer;
> > +   int i, ret;
> > +
> > +   if (!nxfers)
> > +           return 0;
> > +
> > +   xfer = xi3c_master_alloc_xfer(master, nxfers);
> > +   if (!xfer)
> > +           return -ENOMEM;
> > +
> > +   for (i = 0; i < nxfers; i++) {
> > +           struct xi3c_cmd *cmd = &xfer->cmds[i];
> > +
> > +           cmd->addr = xfers[i].addr & XI3C_ADDR_MASK;
> > +           cmd->rnw = xfers[i].flags & I2C_M_RD;
> > +
> > +           if (cmd->rnw) {
> > +                   cmd->rx_buf = xfers[i].buf;
> > +                   cmd->rx_len = xfers[i].len;
> > +           } else {
> > +                   cmd->tx_buf = (void *)xfers[i].buf;
> > +                   cmd->tx_len = xfers[i].len;
> > +           }
> > +
> > +           cmd->type = XI3C_I2C_MODE;
> > +           cmd->tid = XI3C_I2C_TID;
> > +           cmd->continued = (i + 1) < nxfers;
> > +   }
> > +
> > +   ret = xi3c_master_common_xfer(master, xfer);
> > +   xi3c_master_free_xfer(xfer);
> > +   return ret;
> > +}
> > +
> > +static int xi3c_clk_cfg(struct xi3c_master *master, unsigned long sclhz, u8 mode)
> > +{
> > +   unsigned long core_rate, core_periodns;
> > +   u32 thigh, tlow, thold, odthigh, odtlow, tcasmin, tsustart, tsustop, thdstart;
> > +
> > +   core_rate = clk_get_rate(master->pclk);
> > +   if (!core_rate)
> > +           return -EINVAL;
> > +
> > +   core_periodns = DIV_ROUND_UP(1000000000, core_rate);
> > +
> > +   thigh = DIV_ROUND_UP(core_rate, sclhz) >> 1;
> > +   tlow = thigh;
> > +
> > +   /* Hold time : 40% of tlow time */
> > +   thold = (tlow * 4) / 10;
> > +
> > +   /*
> > +    * For initial IP (revision number == 0), minimum data hold time is 5.
> > +    * For updated IP (revision number > 0), minimum data hold time is 6.
> > +    * Updated IP supports achieving high data rate with low reference
> > +    * frequency.
> > +    */
> > +   if (xi3c_getrevisionnumber(master) == 0)
> > +           thold = (thold < 5) ? 5 : thold;
> > +   else
> > +           thold = (thold < 6) ? 6 : thold;
> > +
> > +   writel((thigh - 2) & XI3C_SCL_HIGH_TIME_MASK,
> > +          master->membase + XI3C_SCL_HIGH_TIME_OFFSET);
> > +   writel((tlow - 2) & XI3C_SCL_LOW_TIME_MASK,
> > +          master->membase + XI3C_SCL_LOW_TIME_OFFSET);
> > +   writel((thold - 2) & XI3C_SDA_HOLD_TIME_MASK,
> > +          master->membase + XI3C_SDA_HOLD_TIME_OFFSET);
> > +
> > +   if (!mode) {
> > +           /* I2C */
> > +           writel((thigh - 2) & XI3C_SCL_HIGH_TIME_MASK,
> > +                  master->membase + XI3C_OD_SCL_HIGH_TIME_OFFSET);
> > +           writel((tlow - 2) & XI3C_SCL_LOW_TIME_MASK,
> > +                  master->membase + XI3C_OD_SCL_LOW_TIME_OFFSET);
> > +
> > +           tcasmin = DIV_ROUND_UP(XI3C_I2C_TCASMIN_NS,
> core_periodns);
> > +   } else {
> > +           /* I3C */
> > +           odtlow = DIV_ROUND_UP(XI3C_OD_TLOW_NS, core_periodns);
> > +           odthigh = DIV_ROUND_UP(XI3C_OD_THIGH_NS, core_periodns);
> > +
> > +           odtlow = (tlow < odtlow) ? odtlow : tlow;
> > +           odthigh = (thigh > odthigh) ? odthigh : thigh;
> > +
> > +           writel((odthigh - 2) & XI3C_SCL_HIGH_TIME_MASK,
> > +                  master->membase + XI3C_OD_SCL_HIGH_TIME_OFFSET);
> > +           writel((odtlow - 2) & XI3C_SCL_LOW_TIME_MASK,
> > +                  master->membase + XI3C_OD_SCL_LOW_TIME_OFFSET);
> > +
> > +           tcasmin = DIV_ROUND_UP(XI3C_TCASMIN_NS, core_periodns);
> > +   }
> > +
> > +   thdstart = (thigh > tcasmin) ? thigh : tcasmin;
> > +   tsustart = (tlow > tcasmin) ? tlow : tcasmin;
> > +   tsustop = (tlow > tcasmin) ? tlow : tcasmin;
> > +
> > +   writel((tsustart - 2) & XI3C_TSU_START_MASK,
> > +          master->membase + XI3C_TSU_START_OFFSET);
> > +   writel((thdstart - 2) & XI3C_THD_START_MASK,
> > +          master->membase + XI3C_THD_START_OFFSET);
> > +   writel((tsustop - 2) & XI3C_TSU_STOP_MASK,
> > +          master->membase + XI3C_TSU_STOP_OFFSET);
> > +
> > +   return 0;
> > +}
> > +
> > +static int xi3c_master_bus_init(struct i3c_master_controller *m)
> > +{
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +   struct i3c_bus *bus = i3c_master_get_bus(m);
> > +   struct i3c_device_info info = { };
> > +   unsigned long sclhz;
> > +   u64 pid1_bcr_dcr;
> > +   u8 mode;
> > +   int ret;
> > +
> > +   switch (bus->mode) {
> > +   case I3C_BUS_MODE_MIXED_FAST:
> > +   case I3C_BUS_MODE_MIXED_LIMITED:
> > +           mode = XI3C_I2C_MODE;
> > +           sclhz = bus->scl_rate.i2c;
> > +           break;
> > +   case I3C_BUS_MODE_PURE:
> > +           mode = XI3C_SDR_MODE;
> > +           sclhz = bus->scl_rate.i3c;
> > +           break;
> > +   default:
> > +           return -EINVAL;
> > +   }
> > +
> > +   ret = xi3c_clk_cfg(master, sclhz, mode);
> > +   if (ret)
> > +           return ret;
> > +
> > +   /* Get an address for the master. */
> > +   ret = i3c_master_get_free_addr(m, 0);
> > +   if (ret < 0)
> > +           return ret;
> > +
> > +   info.dyn_addr = (u8)ret;
> > +
> > +   /* Write the dynamic address value to the address register. */
> > +   writel(info.dyn_addr, master->membase + XI3C_ADDRESS_OFFSET);
> > +
> > +   /* Read PID, BCR and DCR values, and assign to i3c device info. */
> > +   pid1_bcr_dcr = readl(master->membase + XI3C_PID1_BCR_DCR);
> > +   info.pid = ((FIELD_GET(XI3C_PID1_MASK, pid1_bcr_dcr) << 32) |
> > +               readl(master->membase + XI3C_PID0_OFFSET));
> > +   info.bcr = (u8)FIELD_GET(XI3C_BCR_MASK, pid1_bcr_dcr);
> > +   info.dcr = (u8)FIELD_GET(XI3C_DCR_MASK, pid1_bcr_dcr);
> > +
> > +   ret = i3c_master_set_info(&master->base, &info);
> > +   if (ret)
> > +           return ret;
> > +
> > +   xi3c_master_init(master);
> > +
> > +   return ret;
> > +}
> > +
> > +static void xi3c_master_bus_cleanup(struct i3c_master_controller *m)
> > +{
> > +   struct xi3c_master *master = to_xi3c_master(m);
> > +
> > +   xi3c_master_disable(master);
> > +}
> > +
> > +static const struct i3c_master_controller_ops xi3c_master_ops = {
> > +   .bus_init = xi3c_master_bus_init,
> > +   .bus_cleanup = xi3c_master_bus_cleanup,
> > +   .do_daa = xi3c_master_do_daa,
> > +   .supports_ccc_cmd = xi3c_master_supports_ccc_cmd,
> > +   .send_ccc_cmd = xi3c_master_send_ccc_cmd,
> > +   .priv_xfers = xi3c_master_priv_xfers,
> > +   .i2c_xfers = xi3c_master_i2c_xfers,
> > +};
> > +
> > +static int xi3c_master_probe(struct platform_device *pdev)
> > +{
> > +   struct xi3c_master *master;
> > +
> > +   master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
> > +   if (!master)
> > +           return -ENOMEM;
> > +
> > +   master->membase = devm_platform_ioremap_resource(pdev, 0);
> > +   if (IS_ERR(master->membase))
> > +           return PTR_ERR(master->membase);
> > +
> > +   master->pclk = devm_clk_get_enabled(&pdev->dev, NULL);
> > +   if (IS_ERR(master->pclk))
> > +           return dev_err_probe(&pdev->dev, PTR_ERR(master->pclk),
> > +                                "Failed to get and enable clock\n");
> > +
> > +   master->dev = &pdev->dev;
> > +
> > +   mutex_init(&master->lock);
>
> devm_mutex_init()
We will fix.
>
> Frank
> > +   spin_lock_init(&master->xferqueue.lock);
> > +   INIT_LIST_HEAD(&master->xferqueue.list);
> > +
> > +   platform_set_drvdata(pdev, master);
> > +
> > +   return i3c_master_register(&master->base, &pdev->dev,
> > +                              &xi3c_master_ops, false);
> > +}
> > +
> > +static void xi3c_master_remove(struct platform_device *pdev)
> > +{
> > +   struct xi3c_master *master = platform_get_drvdata(pdev);
> > +
> > +   i3c_master_unregister(&master->base);
> > +}
> > +
> > +static const struct of_device_id xi3c_master_of_ids[] = {
> > +   { .compatible = "xlnx,axi-i3c-1.0" },
> > +   { },
> > +};
> > +
> > +static struct platform_driver xi3c_master_driver = {
> > +   .probe = xi3c_master_probe,
> > +   .remove = xi3c_master_remove,
> > +   .driver = {
> > +           .name = "axi-i3c-master",
> > +           .of_match_table = xi3c_master_of_ids,
> > +   },
> > +};
> > +module_platform_driver(xi3c_master_driver);
> > +
> > +MODULE_AUTHOR("Manikanta Guntupalli <manikanta.guntupalli@....com>");
> > +MODULE_DESCRIPTION("AXI I3C master driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.34.1
> >

Thanks,
Manikanta.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ