[Public] Hi,
> -----Original Message----- > From: Guntupalli, Manikanta > Sent: Friday, September 12, 2025 11:25 AM > To: Frank Li <frank...@nxp.com> > Cc: git (AMD-Xilinx) <g...@amd.com>; Simek, Michal <michal.si...@amd.com>; > alexandre.bell...@bootlin.com; r...@kernel.org; krzk...@kernel.org; > conor...@kernel.org; k...@kernel.org; gustavo...@kernel.org; > jarkko.nik...@linux.intel.com; linux-...@lists.infradead.org; > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; linux- > harden...@vger.kernel.org; Pandey, Radhey Shyam > <radhey.shyam.pan...@amd.com>; Goud, Srinivas <srinivas.g...@amd.com>; > Datta, Shubhrajyoti <shubhrajyoti.da...@amd.com>; manion0...@gmail.com > Subject: RE: [PATCH V6 2/2] i3c: master: Add AMD I3C bus controller driver > > Hi, > > > -----Original Message----- > > From: Frank Li <frank...@nxp.com> > > Sent: Thursday, September 11, 2025 9:15 PM > > To: Guntupalli, Manikanta <manikanta.guntupa...@amd.com> > > Cc: git (AMD-Xilinx) <g...@amd.com>; Simek, Michal > > <michal.si...@amd.com>; alexandre.bell...@bootlin.com; > > r...@kernel.org; krzk...@kernel.org; > > conor...@kernel.org; k...@kernel.org; gustavo...@kernel.org; > > jarkko.nik...@linux.intel.com; linux-...@lists.infradead.org; > > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; linux- > > harden...@vger.kernel.org; Pandey, Radhey Shyam > > <radhey.shyam.pan...@amd.com>; Goud, Srinivas <srinivas.g...@amd.com>; > > Datta, Shubhrajyoti <shubhrajyoti.da...@amd.com>; manion0...@gmail.com > > Subject: Re: [PATCH V6 2/2] i3c: master: Add AMD I3C bus controller > > driver > > > > On Thu, Sep 11, 2025 at 06:07:59AM +0000, Guntupalli, Manikanta wrote: > > > [Public] > > > > > > Hi, > > > > > > > -----Original Message----- > > > > From: Frank Li <frank...@nxp.com> > > > > Sent: Wednesday, September 10, 2025 10:25 PM > > > > To: Guntupalli, Manikanta <manikanta.guntupa...@amd.com> > > > > Cc: git (AMD-Xilinx) <g...@amd.com>; Simek, Michal > > > > <michal.si...@amd.com>; alexandre.bell...@bootlin.com; > > > > r...@kernel.org; krzk...@kernel.org; > > > > conor...@kernel.org; k...@kernel.org; gustavo...@kernel.org; > > > > jarkko.nik...@linux.intel.com; linux-...@lists.infradead.org; > > > > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; linux- > > > > harden...@vger.kernel.org; Pandey, Radhey Shyam > > > > <radhey.shyam.pan...@amd.com>; Goud, Srinivas > > > > <srinivas.g...@amd.com>; Datta, Shubhrajyoti > > > > <shubhrajyoti.da...@amd.com>; manion0...@gmail.com > > > > Subject: Re: [PATCH V6 2/2] i3c: master: Add AMD I3C bus > > > > controller driver > > > > > > > > On Wed, Sep 10, 2025 at 04:59:54PM +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.guntupa...@amd.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. > > > > > > > > > > Changes for V3: > > > > > Resolved merge conflicts. > > > > > > > > > > Changes for V4: > > > > > Updated timeout macros. > > > > > Removed type casting for xi3c_is_resp_available() macro. > > > > > Used ioread32() and iowrite32() instead of readl() and writel() > > > > > to keep consistency. > > > > > Read XI3C_RESET_OFFSET reg before udelay(). > > > > > Removed xi3c_master_free_xfer() and directly used kfree(). > > > > > Skipped checking return value of i3c_master_add_i3c_dev_locked(). > > > > > Used devm_mutex_init() instead of mutex_init(). > > > > > > > > > > Changes for V5: > > > > > Used GENMASK_ULL for PID mask as it's 64bit mask. > > > > > > > > > > Changes for V6: > > > > > Removed typecast for xi3c_getrevisionnumber(), > > > > > xi3c_wrfifolevel(), and xi3c_rdfifolevel(). > > > > > Replaced dynamic allocation with a static variable for pid_bcr_dcr. > > > > > Fixed sparse warning in do_daa by typecasting the address parity > > > > > value to u8. > > > > > Fixed sparse warning in xi3c_master_bus_init by typecasting the > > > > > pid value to u64 in info.pid calculation. > > > > > --- > > > > > 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 > > > > > 1af81124bba3..ff603ce5e78d 100644 > > > > > --- a/MAINTAINERS > > > > > +++ b/MAINTAINERS > > > > > @@ -11693,6 +11693,13 @@ L: linux-...@vger.kernel.org > > > > > S: Maintained > > > > > F: drivers/i2c/i2c-stub.c > > > > > > > > > > +I3C DRIVER FOR AMD AXI I3C MASTER > > > > > +M: Manikanta Guntupalli <manikanta.guntupa...@amd.com> > > > > > +R: Michal Simek <michal.si...@amd.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 <j...@codeconstruct.com.au> > > > > > S: Maintained > > > > > diff --git a/drivers/i3c/master/Kconfig > > > > > b/drivers/i3c/master/Kconfig index 13df2944f2ec..4b884a678893 > > > > > 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 HAS_IOMEM > > > > > diff --git a/drivers/i3c/master/Makefile > > > > > b/drivers/i3c/master/Makefile index aac74f3e3851..109bd48cb159 > > > > > 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..cd9d85a0be80 > > > > > --- /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. > > > > > + */ > > > > > + > > > > ... > > > > > + > > > > > +/* timeout waiting for the controller finish transfers */ > > > > > +#define XI3C_XFER_TIMEOUT_MS 10000 > > > > > +#define XI3C_XFER_TIMEOUT > > > > (msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS)) > > > > > > > > Do I missed your reply? I have not seen > > > > https://lore.kernel.org/linux-i3c/aL7+Urm4NB9kwOwQ@lizhi-Precision > > > > -T > > > > ower-5810/ > > > > > > > > If you need define two macro for it. Need unit > > > > XI3C_XFER_TIMEOUT_JIFFIES to avoid confuse. > > > Sure, I'll update the macro name to XI3C_XFER_TIMEOUT_JIFFIES to > > > avoid > > confusion. > > > > > > > > > > > > > + > > > > > +#define xi3c_getrevisionnumber(master) > > > > > \ > > > > > + (FIELD_GET(XI3C_REV_NUM_MASK, > > > > \ > > > > > + ioread32((master)->membase + > > > > > + XI3C_VERSION_OFFSET))) > > > > > + > > > > > > > > ... > > > > > > > > > +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); } > > > > > > > > i3c_writel(readl)_fifo() did similar things, why not use it? > > > > > > > > Did you miss my review comment or I missed your reply? > > > You have missed my earlier reply on this. > > > The helpers i3c_writel_fifo() and i3c_readl_fifo() rely on the CPU's > > > native endianness, whereas in this case the FIFO should always be > > > accessed in > > big-endian format. > > > Hence, we cannot use the common helpers directly. > > > > > > For reference, here's my previous reply: > > > > > > https://lore.kernel.org/all/DM4PR12MB6109F6D5D032723C675472448C0FA@DM4 > > > PR12MB6109.namprd12.prod.outlook.com/ > > > > Sorry, can you improve i3c_writel(readl)_fifo() to support endianness? > > I think not only you have this kind problem in future. > Sure, will add support for endianness. To add support for endianness, we could not find a big-endian counterpart to writesl(). Please let me know if I missed any existing helper. > > > > > > > > > > > > > ... > > > > > + > > > > > +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.guntupa...@amd.com>"); MODULE_DESCRIPTION("AXI I3C > > > > > +master driver"); > > > > MODULE_LICENSE("GPL"); > > > > > -- > > > > > 2.34.1 > > > > > > > > > Thanks, Manikanta.
