| 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: <aMGtdFxlXRanIAuM@lizhi-Precision-Tower-5810>
Date: Wed, 10 Sep 2025 12:55:16 -0400
From: Frank Li <Frank.li@....com>
To: Manikanta Guntupalli <manikanta.guntupalli@....com>
Cc: git@....com, 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, radhey.shyam.pandey@....com,
srinivas.goud@....com, 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-Tower-5810/
If you need define two macro for it. Need unit XI3C_XFER_TIMEOUT_JIFFIES to
avoid confuse.
> +
> +#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?
Frank
...
> +
> +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
>
Powered by blists - more mailing lists