| 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
| ||
|
Message-ID:
<DM4PR12MB61093B9BB9B3EF1D9F6B499E8C08A@DM4PR12MB6109.namprd12.prod.outlook.com>
Date: Fri, 12 Sep 2025 05:54:39 +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 V6 2/2] i3c: master: Add AMD I3C bus controller driver
[Public]
Hi,
> -----Original Message-----
> From: Frank Li <Frank.li@....com>
> Sent: Thursday, September 11, 2025 9:15 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 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.li@....com>
> > > Sent: Wednesday, September 10, 2025 10:25 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 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.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.
> > > >
> > > > 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-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 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.
>
> >
> > >
> > > ...
> > > > +
> > > > +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