* Re: [PATCH v5 08/12] i3c: dw-i3c-master: Add ACPI core clock frequency quirk
From: Frank Li @ 2026-06-24 18:09 UTC (permalink / raw)
To: Akhil R
Cc: Alexandre Belloni, Frank Li, Miquel Raynal, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Guenter Roeck, Philipp Zabel,
Jon Hunter, Thierry Reding, linux-i3c, devicetree, linux-hwmon,
linux-tegra, linux-kernel
In-Reply-To: <20260624102153.1770072-9-akhilrajeev@nvidia.com>
On Wed, Jun 24, 2026 at 10:21:02AM +0000, Akhil R wrote:
> Some ACPI-enumerated devices like Tegra410 do not expose the controller
> core clock through the clk framework. Unlike device tree, ACPI on Arm does
> not model clock providers. The hardware is expected to have its clocks
> enabled by firmware before the OS takes over.
>
> Make the core clock optional and allow selected ACPI devices to provide the
> core clock rate through the "clock-frequency" _DSD property when the core
> clock is absent.
>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/i3c/master/dw-i3c-master.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 29030fd9594a..8e40d178d500 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -242,6 +242,7 @@
> /* List of quirks */
> #define AMD_I3C_OD_PP_TIMING BIT(1)
> #define DW_I3C_DISABLE_RUNTIME_PM_QUIRK BIT(2)
> +#define DW_I3C_ACPI_SKIP_CLK_RST BIT(3)
>
> struct dw_i3c_cmd {
> u32 cmd_lo;
> @@ -556,13 +557,33 @@ static void dw_i3c_master_set_intr_regs(struct dw_i3c_master *master)
> writel(IBI_REQ_REJECT_ALL, master->regs + IBI_MR_REQ_REJECT);
> }
>
> +static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
> +{
> + unsigned int core_rate_prop;
> +
> + if (master->core_clk)
> + return clk_get_rate(master->core_clk);
> +
> + if (!(master->quirks & DW_I3C_ACPI_SKIP_CLK_RST))
> + dev_err(master->dev, "missing core clock\n");
> + return 0;
> + }
> +
> + if (device_property_read_u32(master->dev, "clock-frequency", &core_rate_prop)) {
> + dev_err(master->dev, "missing clock-frequency property\n");
> + return 0;
> + }
> +
> + return core_rate_prop;
> +}
> +
> static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
> {
> unsigned long core_rate, core_period;
> u32 scl_timing;
> u8 hcnt, lcnt;
>
> - core_rate = clk_get_rate(master->core_clk);
> + core_rate = dw_i3c_master_get_core_rate(master);
> if (!core_rate)
> return -EINVAL;
>
> @@ -615,7 +636,7 @@ static int dw_i2c_clk_cfg(struct dw_i3c_master *master)
> u16 hcnt, lcnt;
> u32 scl_timing;
>
> - core_rate = clk_get_rate(master->core_clk);
> + core_rate = dw_i3c_master_get_core_rate(master);
> if (!core_rate)
> return -EINVAL;
>
> @@ -1577,7 +1598,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
> if (IS_ERR(master->regs))
> return PTR_ERR(master->regs);
>
> - master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
> + master->core_clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
> if (IS_ERR(master->core_clk))
> return PTR_ERR(master->core_clk);
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v7 09/11] cxl: Restore CXL HDM state after PCI reset
From: Richard Cheng @ 2026-06-24 14:55 UTC (permalink / raw)
To: Srirangan Madhavan
Cc: Alison Schofield, Bjorn Helgaas, Dan Williams, Dave Jiang,
Davidlohr Bueso, Ira Weiny, Jonathan Cameron, Vishal Verma,
linux-cxl, linux-pci, linux-kernel, vsethi, alwilliamson,
Dan Williams, Sai Yashwanth Reddy Kancherla, Vishal Aslot,
Manish Honap, Jiandi An, linux-tegra
In-Reply-To: <20260623032453.3404772-10-smadhavan@nvidia.com>
On Tue, Jun 23, 2026 at 03:24:51AM +0800, Srirangan Madhavan wrote:
> After CXL reset, restore PCI config state enough to reach HDM MMIO,
> restore cached global and per-decoder HDM state, and then run the normal
> PCI restore callbacks.
>
> Keep target and sibling IOMMU reset blocks active until HDM restore
> completes so Bus Master Enable cannot reopen DMA before decoder state is
> valid.
>
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
> drivers/cxl/core/hdm.c | 4 +
> drivers/cxl/core/reset.c | 195 ++++++++++++++++++++++++++++++++++++---
> include/cxl/cxl.h | 2 +
> 3 files changed, 190 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0230ebfada42..095cc13e5d00 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -152,6 +152,10 @@ static void cxl_hdm_info_set_decoder(struct cxl_hdm *cxlhdm,
> if (!info || cxld->id >= info->decoder_count)
> return;
>
> + if (cxlhdm->regs.hdm_decoder)
> + info->global_ctrl = readl(cxlhdm->regs.hdm_decoder +
> + CXL_HDM_DECODER_CTRL_OFFSET);
> +
> if (cxld->flags & CXL_DECODER_F_ENABLE)
> info->settings[cxld->id] = cxld->settings;
> else
> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> index 69bcfab89858..d801c91a5cbf 100644
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c
> @@ -83,6 +83,21 @@ static int cxld_await_commit(void __iomem *hdm, int id)
> return -ETIMEDOUT;
> }
>
> +static int cxld_await_uncommit(void __iomem *hdm, int id)
> +{
> + u32 ctrl;
> + int i;
> +
> + for (i = 0; i < COMMIT_TIMEOUT_MS; i++) {
> + ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> + if (!FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl))
> + return 0;
> + fsleep(1000);
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> static void setup_hw_decoder(struct cxl_decoder_settings *settings,
> void __iomem *hdm)
> {
> @@ -92,6 +107,8 @@ static void setup_hw_decoder(struct cxl_decoder_settings *settings,
> u32 ctrl;
>
> ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> + ctrl &= ~(CXL_HDM_DECODER0_CTRL_COMMIT |
> + CXL_HDM_DECODER0_CTRL_COMMIT_ERROR);
> cxld_set_interleave(settings, &ctrl);
> cxld_set_type(settings, &ctrl);
> base = settings->hpa_range.start;
> @@ -300,6 +317,8 @@ int pci_cxl_hdm_init(struct pci_dev *pdev)
>
> info->decoder_count = decoder_count;
> info->regs = regs;
> + info->global_ctrl = readl(regs.hdm_decoder +
> + CXL_HDM_DECODER_CTRL_OFFSET);
>
> settings = info->settings;
> for (int i = 0; i < info->decoder_count; i++) {
> @@ -324,6 +343,100 @@ int pci_cxl_hdm_init(struct pci_dev *pdev)
> return rc;
> }
>
> +static int cxl_hdm_decoder_uncommit(struct pci_dev *pdev, void __iomem *hdm,
> + int id, bool *locked_committed)
> +{
> + u32 ctrl;
> + int rc;
> +
> + *locked_committed = false;
> + ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> + if (ctrl & CXL_HDM_DECODER0_CTRL_LOCK) {
> + if (ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED) {
> + pci_dbg(pdev,
> + "CXL HDM decoder %d retained locked committed state\n",
> + id);
> + *locked_committed = true;
> + return 0;
> + }
> +
> + pci_err(pdev, "CXL HDM decoder %d is locked\n", id);
> + return -EBUSY;
> + }
> +
> + if (!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED))
> + return 0;
> +
> + ctrl &= ~CXL_HDM_DECODER0_CTRL_COMMIT;
> + writel(ctrl, hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> +
> + rc = cxld_await_uncommit(hdm, id);
> + if (rc)
> + pci_err(pdev, "CXL HDM decoder %d uncommit failed: %d\n",
> + id, rc);
> +
> + return rc;
> +}
> +
> +static int cxl_restore_hdm_decoder(struct pci_dev *pdev,
> + struct cxl_decoder_settings *settings,
> + void __iomem *hdm)
> +{
> + bool locked_committed;
> + int rc;
> +
> + if (!(settings->flags & CXL_DECODER_F_ENABLE))
> + return 0;
> +
> + rc = cxl_hdm_decoder_uncommit(pdev, hdm, settings->id,
> + &locked_committed);
> + if (rc)
> + return rc;
> + if (locked_committed)
> + return 0;
> +
> + rc = cxl_commit(settings, hdm);
> + if (rc)
> + pci_err(pdev, "CXL HDM decoder %d restore failed: %d\n",
> + settings->id, rc);
> +
> + return rc;
> +}
> +
> +static int cxl_restore_hdm(struct pci_dev *pdev)
> +{
> + struct cxl_hdm_info *info = READ_ONCE(pdev->hdm);
> + void __iomem *hdm;
> + int first_rc = 0;
> +
> + if (!info)
> + return 0;
> +
> + hdm = info->regs.hdm_decoder;
> + if (!hdm) {
> + pci_err(pdev, "CXL HDM decoder registers unavailable\n");
> + return -ENXIO;
> + }
> +
> + /*
> + * Restore global HDM control before per-decoder commit. PCI config
> + * state has been restored for MMIO access, but IOMMU reset blocks
> + * remain active until HDM restore completes.
> + */
> + writel(info->global_ctrl, hdm + CXL_HDM_DECODER_CTRL_OFFSET);
> +
> + for (int i = 0; i < info->decoder_count; i++) {
> + struct cxl_decoder_settings *settings = &info->settings[i];
> + int rc;
> +
> + rc = cxl_restore_hdm_decoder(pdev, settings, hdm);
> + if (rc && !first_rc)
> + first_rc = rc;
> + }
> +
> + return first_rc;
> +}
> +
> /*
> * CXL r4.0 sec 9.7.2 defines the reset completion timeout encodings.
> * Sec 9.7.3 leaves config-space access behavior undefined for 100 ms after
> @@ -355,6 +468,7 @@ struct cxl_reset_context {
> int nr_siblings;
> int nr_siblings_locked;
> int nr_siblings_prepared;
> + bool target_prepared;
> int sibling_capacity;
> };
>
> @@ -609,6 +723,68 @@ static int cxl_pci_functions_reset_prepare(struct cxl_reset_context *ctx)
> return 0;
> }
>
> +static void cxl_pci_target_reset_done(struct cxl_reset_context *ctx)
> +{
> + if (!ctx->target_prepared)
> + return;
> +
> + pci_dev_reset_iommu_done(ctx->target);
> + ctx->target_prepared = false;
> +}
> +
> +static int cxl_pci_target_reset_prepare(struct cxl_reset_context *ctx)
> +{
> + struct pci_dev *pdev = ctx->target;
> + int rc;
> +
> + if (!pci_wait_for_pending_transaction(pdev))
> + pci_err(pdev, "timed out waiting for pending transactions\n");
> +
> + rc = pci_dev_reset_iommu_prepare(pdev);
> + if (rc) {
> + pci_err(pdev, "failed to stop IOMMU for CXL reset: %d\n", rc);
> + return rc;
> + }
> +
> + ctx->target_prepared = true;
> + return 0;
> +}
> +
> +static void cxl_pci_functions_restore_state(struct cxl_reset_context *ctx)
> +{
> + /*
> + * Restore PCI config state first so HDM MMIO is reachable. The final
> + * pci_dev_restore() pass deliberately replays pci_restore_state()
> + * before invoking driver reset_done() callbacks.
> + */
> + pci_restore_state(ctx->target);
> +
> + for (int i = 0; i < ctx->nr_siblings_prepared; i++)
> + pci_restore_state(ctx->siblings[i].pdev);
> +}
> +
> +static int cxl_restore_hdm_decoders(struct cxl_reset_context *ctx)
> +{
> + int first_rc = 0;
> + int rc;
> +
> + cxl_pci_functions_restore_state(ctx);
> +
> + rc = cxl_restore_hdm(ctx->target);
> + if (rc && !first_rc)
> + first_rc = rc;
> +
> + for (int i = 0; i < ctx->nr_siblings_prepared; i++) {
> + struct pci_dev *sibling = ctx->siblings[i].pdev;
> +
> + rc = cxl_restore_hdm(sibling);
> + if (rc && !first_rc)
> + first_rc = rc;
> + }
> +
> + return first_rc;
> +}
> +
> static void cxl_hdm_range_context_init(struct cxl_hdm_range_context *ctx)
> {
> INIT_LIST_HEAD(&ctx->ranges);
> @@ -985,18 +1161,9 @@ static int cxl_reset_execute(struct pci_dev *pdev, int dvsec)
> if (rc)
> return pcibios_err_to_errno(rc);
>
> - if (!pci_wait_for_pending_transaction(pdev))
> - pci_err(pdev, "timed out waiting for pending transactions\n");
> -
> - rc = pci_dev_reset_iommu_prepare(pdev);
> - if (rc) {
> - pci_err(pdev, "failed to stop IOMMU for CXL reset: %d\n", rc);
> - return rc;
> - }
> -
> rc = cxl_reset_disable_cache(pdev, dvsec, cap);
> if (rc)
> - goto out;
> + return rc;
> cache_disabled = true;
>
> rc = cxl_reset_update_ctrl2(pdev, dvsec, PCI_DVSEC_CXL_INIT_CXL_RST,
> @@ -1020,7 +1187,6 @@ static int cxl_reset_execute(struct pci_dev *pdev, int dvsec)
> rc = rc2;
> }
>
> - pci_dev_reset_iommu_done(pdev);
> return rc;
> }
>
> @@ -1053,12 +1219,19 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
> if (rc)
> goto out_functions_done;
>
> + rc = cxl_pci_target_reset_prepare(&ctx);
> + if (rc)
> + goto out_functions_done;
> +
> scoped_guard(rwsem_write, &cxl_rwsem.region) {
> rc = cxl_hdm_ranges_prepare(&range_ctx, &ctx);
> if (!rc)
> rc = cxl_reset_execute(pdev, dvsec);
> + if (!rc)
> + rc = cxl_restore_hdm_decoders(&ctx);
> }
>
HDM restore only runs when cxl_reset_execute() returns 0. Combine with
the wait_done false -EIO at patch 5, the reset succeeds in HW but execute
returns -EIO, so restore is skipped entirely.
Maybe we should detect an unreachable device (all-ones config) and chain a
link-level recovery instead of leaving it wedged.
--Richard
> + cxl_pci_target_reset_done(&ctx);
> out_functions_done:
> cxl_pci_functions_reset_done(&ctx);
> out_unlock:
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 1fe606f15733..eddc48f1fa49 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -127,11 +127,13 @@ struct cxl_regs {
> * struct cxl_hdm_info - PCI device HDM decoder programming cache
> * @decoder_count: number of decoder settings entries
> * @regs: mapped CXL component registers for this HDM decoder block
> + * @global_ctrl: cached HDM decoder global control register
> * @settings: cached per-decoder programming state
> */
> struct cxl_hdm_info {
> int decoder_count;
> struct cxl_component_regs regs;
> + u32 global_ctrl;
> struct cxl_decoder_settings settings[] __counted_by(decoder_count);
> };
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v7 05/11] cxl: Add CXL Device Reset helper
From: Richard Cheng @ 2026-06-24 14:33 UTC (permalink / raw)
To: Srirangan Madhavan
Cc: Alison Schofield, Bjorn Helgaas, Dan Williams, Dave Jiang,
Davidlohr Bueso, Ira Weiny, Jonathan Cameron, Vishal Verma,
linux-cxl, linux-pci, linux-kernel, vsethi, alwilliamson,
Dan Williams, Sai Yashwanth Reddy Kancherla, Vishal Aslot,
Manish Honap, Jiandi An, linux-tegra
In-Reply-To: <20260623032453.3404772-6-smadhavan@nvidia.com>
On Tue, Jun 23, 2026 at 03:24:47AM +0800, Srirangan Madhavan wrote:
> Add an internal CXL Device Reset helper for Type 2 functions that advertise
> CXL Reset in the CXL Device DVSEC. The helper disables CXL.cache, performs
> cache writeback when supported, initiates reset with Memory Clear disabled,
> waits for completion, and re-enables CXL.cache on exit.
>
> Leave the helper unregistered until range validation and reset-scope
> coordination are in place.
>
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
> drivers/cxl/core/reset.c | 221 ++++++++++++++++++++++++++++++++++
> include/cxl/cxl.h | 7 ++
> include/uapi/linux/pci_regs.h | 14 +++
> 3 files changed, 242 insertions(+)
>
> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> index fc52d3abdb5b..fdfcc9e825e0 100644
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c
> @@ -7,6 +7,8 @@
> #include <linux/export.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> +#include <linux/iommu.h>
> +#include <linux/jiffies.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> #include <linux/slab.h>
> @@ -318,3 +320,222 @@ int pci_cxl_hdm_init(struct pci_dev *pdev)
> cxl_pci_hdm_unmap(pdev, ®s, &map);
> return rc;
> }
> +
> +/*
> + * CXL r4.0 sec 9.7.2 defines the reset completion timeout encodings.
> + * Sec 9.7.3 leaves config-space access behavior undefined for 100 ms after
> + * initiating CXL Reset, then limits software to CXL Status2 access until
> + * reset completion, timeout, or error.
> + */
> +#define CXL_RESET_RRS_WAIT_MS 100
> +#define CXL_RESET_STATUS_POLL_MS 20
> +static const u32 cxl_reset_timeout_ms[] = {
> + 10, 100, 1000, 10000, 100000,
> +};
> +
> +#define CXL_CACHE_WBI_TIMEOUT_US 100000
> +#define CXL_CACHE_WBI_POLL_US 100
> +
> +static int cxl_reset_dvsec(struct pci_dev *pdev)
> +{
> + int dvsec, rc;
> + u16 cap;
> +
> + dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> + PCI_DVSEC_CXL_DEVICE);
> + if (!dvsec)
> + return -ENOTTY;
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CAP, &cap);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + if ((cap & (PCI_DVSEC_CXL_CACHE_CAPABLE |
> + PCI_DVSEC_CXL_MEM_CAPABLE)) !=
> + (PCI_DVSEC_CXL_CACHE_CAPABLE | PCI_DVSEC_CXL_MEM_CAPABLE))
> + return -ENOTTY;
> +
> + if (!(cap & PCI_DVSEC_CXL_RST_CAPABLE))
> + return -ENOTTY;
> +
> + return dvsec;
> +}
> +
> +static int cxl_reset_update_ctrl2(struct pci_dev *pdev, int dvsec, u16 set,
> + u16 clear)
> +{
> + u16 ctrl2;
> + int rc;
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CTRL2, &ctrl2);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + ctrl2 |= set;
> + ctrl2 &= ~clear;
> +
> + rc = pci_write_config_word(pdev, dvsec + PCI_DVSEC_CXL_CTRL2, ctrl2);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + return 0;
> +}
> +
> +static int cxl_reset_enable_cache(struct pci_dev *pdev, int dvsec)
> +{
> + return cxl_reset_update_ctrl2(pdev, dvsec, 0,
> + PCI_DVSEC_CXL_DISABLE_CACHING);
> +}
> +
> +static int cxl_reset_disable_cache(struct pci_dev *pdev, int dvsec, u16 cap)
> +{
> + int remaining_us = CXL_CACHE_WBI_TIMEOUT_US;
> + u16 status2;
> + int rc, rc2;
> +
> + rc = cxl_reset_update_ctrl2(pdev, dvsec,
> + PCI_DVSEC_CXL_DISABLE_CACHING, 0);
> + if (rc)
> + return rc;
> +
> + if (!(cap & PCI_DVSEC_CXL_CACHE_WBI_CAPABLE))
> + return 0;
> +
> + rc = cxl_reset_update_ctrl2(pdev, dvsec,
> + PCI_DVSEC_CXL_INIT_CACHE_WBI, 0);
> + if (rc)
> + goto err_enable_cache;
> +
> + do {
> + usleep_range(CXL_CACHE_WBI_POLL_US, CXL_CACHE_WBI_POLL_US + 1);
> + remaining_us -= CXL_CACHE_WBI_POLL_US;
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_STATUS2,
> + &status2);
> + if (rc) {
> + rc = pcibios_err_to_errno(rc);
> + goto err_enable_cache;
> + }
> + } while (!(status2 & PCI_DVSEC_CXL_CACHE_INV) && remaining_us > 0);
> +
> + if (!(status2 & PCI_DVSEC_CXL_CACHE_INV)) {
> + rc = -ETIMEDOUT;
> + goto err_enable_cache;
> + }
> +
> + return 0;
> +
> +err_enable_cache:
> + /*
> + * DISABLE_CACHING can be rolled back here. INIT_CACHE_WBI is
> + * self-clearing on completion, so leave any in-flight writeback alone.
> + */
> + rc2 = cxl_reset_enable_cache(pdev, dvsec);
> + if (rc2)
> + pci_warn(pdev, "failed to re-enable CXL caching: %d\n", rc2);
> + return rc;
> +}
> +
> +static int cxl_reset_wait_done(struct pci_dev *pdev, int dvsec, u16 cap)
> +{
> + unsigned long deadline;
> + u32 timeout_ms;
> + u16 status2;
> + int idx, rc;
> +
> + idx = FIELD_GET(PCI_DVSEC_CXL_RST_TIMEOUT, cap);
> + if (idx >= ARRAY_SIZE(cxl_reset_timeout_ms)) {
> + int last = ARRAY_SIZE(cxl_reset_timeout_ms) - 1;
> +
> + pci_warn(pdev,
> + "unknown CXL reset timeout encoding %d; using %u ms\n",
> + idx, cxl_reset_timeout_ms[last]);
> + idx = last;
> + }
> +
> + timeout_ms = max_t(u32, cxl_reset_timeout_ms[idx],
> + CXL_RESET_RRS_WAIT_MS);
> + deadline = jiffies + msecs_to_jiffies(timeout_ms);
> + msleep(CXL_RESET_RRS_WAIT_MS);
> +
> + do {
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_STATUS2,
> + &status2);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + if (status2 & PCI_DVSEC_CXL_RST_ERR)
> + return -EIO;
> +
I think this returns -EIO for a reset that actually succeeds.
During the post-reset RRS window the Status2 read comes back 0xffff.
0xffff has bit 2 set, so it's taken as RST_ERR and we bail on the very
first poll. A transient config-read error is likewise treated as fatal instead
of being retried.
I would suggest to skip all-ones reads as "not ready" and keep polling until
the deadline, only act on RST_ERR and RST_DONE from a valid read.
Best regards,
Richard Cheng
> + if (status2 & PCI_DVSEC_CXL_RST_DONE)
> + return 0;
> +
> + if (time_after_eq(jiffies, deadline))
> + return -ETIMEDOUT;
> +
> + msleep(CXL_RESET_STATUS_POLL_MS);
> + } while (true);
> +}
> +
> +static int cxl_reset_execute(struct pci_dev *pdev, int dvsec)
> +{
> + bool cache_disabled = false;
> + u16 cap;
> + int rc;
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CAP, &cap);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + if (!pci_wait_for_pending_transaction(pdev))
> + pci_err(pdev, "timed out waiting for pending transactions\n");
> +
> + rc = pci_dev_reset_iommu_prepare(pdev);
> + if (rc) {
> + pci_err(pdev, "failed to stop IOMMU for CXL reset: %d\n", rc);
> + return rc;
> + }
> +
> + rc = cxl_reset_disable_cache(pdev, dvsec, cap);
> + if (rc)
> + goto out;
> + cache_disabled = true;
> +
> + rc = cxl_reset_update_ctrl2(pdev, dvsec, PCI_DVSEC_CXL_INIT_CXL_RST,
> + PCI_DVSEC_CXL_RST_MEM_CLR_EN);
> + if (rc)
> + goto out;
> +
> + rc = cxl_reset_wait_done(pdev, dvsec, cap);
> + if (rc)
> + goto out;
> +
> +out:
> + if (cache_disabled) {
> + int rc2;
> +
> + rc2 = cxl_reset_enable_cache(pdev, dvsec);
> + if (rc2 && rc)
> + pci_warn(pdev, "failed to re-enable CXL caching: %d\n",
> + rc2);
> + else if (rc2)
> + rc = rc2;
> + }
> +
> + pci_dev_reset_iommu_done(pdev);
> + return rc;
> +}
> +
> +int cxl_reset_function(struct pci_dev *pdev, bool probe)
> +{
> + int dvsec;
> +
> + dvsec = cxl_reset_dvsec(pdev);
> + if (dvsec < 0)
> + return dvsec;
> +
> + if (probe)
> + return 0;
> +
> + return cxl_reset_execute(pdev, dvsec);
> +}
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index e3087b7517e8..1fe606f15733 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -9,6 +9,7 @@
> #include <linux/node.h>
> #include <linux/ioport.h>
> #include <linux/range.h>
> +#include <linux/errno.h>
> #include <cxl/mailbox.h>
>
> /**
> @@ -137,11 +138,17 @@ struct cxl_hdm_info {
> int cxl_commit(struct cxl_decoder_settings *settings, void __iomem *hdm);
> #ifdef CONFIG_CXL_HDM
> int pci_cxl_hdm_init(struct pci_dev *pdev);
> +int cxl_reset_function(struct pci_dev *pdev, bool probe);
> #else
> static inline int pci_cxl_hdm_init(struct pci_dev *pdev)
> {
> return -ENOTTY;
> }
> +
> +static inline int cxl_reset_function(struct pci_dev *pdev, bool probe)
> +{
> + return -ENOTTY;
> +}
> #endif
>
> struct cxl_reg_map {
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 14f634ab9350..194ae56b4404 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1349,10 +1349,24 @@
> /* CXL r4.0, 8.1.3: PCIe DVSEC for CXL Device */
> #define PCI_DVSEC_CXL_DEVICE 0
> #define PCI_DVSEC_CXL_CAP 0xA
> +#define PCI_DVSEC_CXL_CACHE_CAPABLE _BITUL(0)
> #define PCI_DVSEC_CXL_MEM_CAPABLE _BITUL(2)
> #define PCI_DVSEC_CXL_HDM_COUNT __GENMASK(5, 4)
> +#define PCI_DVSEC_CXL_CACHE_WBI_CAPABLE _BITUL(6)
> +#define PCI_DVSEC_CXL_RST_CAPABLE _BITUL(7)
> +#define PCI_DVSEC_CXL_RST_TIMEOUT __GENMASK(10, 8)
> +#define PCI_DVSEC_CXL_RST_MEM_CLR_CAPABLE _BITUL(11)
> #define PCI_DVSEC_CXL_CTRL 0xC
> #define PCI_DVSEC_CXL_MEM_ENABLE _BITUL(2)
> +#define PCI_DVSEC_CXL_CTRL2 0x10
> +#define PCI_DVSEC_CXL_DISABLE_CACHING _BITUL(0)
> +#define PCI_DVSEC_CXL_INIT_CACHE_WBI _BITUL(1)
> +#define PCI_DVSEC_CXL_INIT_CXL_RST _BITUL(2)
> +#define PCI_DVSEC_CXL_RST_MEM_CLR_EN _BITUL(3)
> +#define PCI_DVSEC_CXL_STATUS2 0x12
> +#define PCI_DVSEC_CXL_CACHE_INV _BITUL(0)
> +#define PCI_DVSEC_CXL_RST_DONE _BITUL(1)
> +#define PCI_DVSEC_CXL_RST_ERR _BITUL(2)
> #define PCI_DVSEC_CXL_RANGE_SIZE_HIGH(i) (0x18 + (i * 0x10))
> #define PCI_DVSEC_CXL_RANGE_SIZE_LOW(i) (0x1C + (i * 0x10))
> #define PCI_DVSEC_CXL_MEM_INFO_VALID _BITUL(0)
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v7 00/11] PCI/CXL: Add CXL reset support for Type 2 devices
From: Richard Cheng @ 2026-06-24 14:26 UTC (permalink / raw)
To: Srirangan Madhavan
Cc: Alison Schofield, Bjorn Helgaas, Dan Williams, Dave Jiang,
Davidlohr Bueso, Ira Weiny, Jonathan Cameron, Vishal Verma,
linux-cxl, linux-pci, linux-kernel, vsethi, alwilliamson,
Dan Williams, Sai Yashwanth Reddy Kancherla, Vishal Aslot,
Manish Honap, Jiandi An, linux-tegra
In-Reply-To: <20260623032453.3404772-1-smadhavan@nvidia.com>
On Tue, Jun 23, 2026 at 03:24:42AM +0800, Srirangan Madhavan wrote:
> Hi folks!
>
> This series adds CXL Reset support for CXL Type 2 devices through the
> existing PCI reset_method ABI. The reset sequence follows the CXL 4.0
> specification [1], including CXL.cache disable, optional cache
> writeback, CXL Reset initiation, ResetComplete polling, and ResetError
> reporting.
>
> The userspace ABI is the existing PCI reset interface:
>
> /sys/bus/pci/devices/.../reset_method
> /sys/bus/pci/devices/.../reset
>
> Userspace can select "cxl_reset" in reset_method and then trigger reset
> through the existing reset attribute.
>
Hi Srirangan,
Thanks for the work, I applied your series and run some tests on a
CXL type-2 capable GPU, seems like something is wrong.
Device's BDF is 0002:81:00.0 and DVSEC base is at 0x10c
CAP=0x116, CTRL2=0x11c, STATUS2=0x11e
STATUS2 bits: bit0 CACHE_INV, bit1 RST_DONE, bit2 RST_ERR
CTRL2 bits: bit0 = DISABLE_CACHING
I run all the following command as root.
"""
# b=0002:81:00.0; dev=/sys/bus/pci/devices/$b
# echo cxl_reset > $dev/reset_method
# echo "PRE CAP=0x$(setpci -s $b 0x116.w) CTRL2=0x$(setpci -s $b 0x11c.w) STATUS2=0x$(setpci -s $b 0x11e.w)"
# dmesg -C
# time echo 1 > $dev/reset
# echo "POST CAP=0x$(setpci -s $b 0x116.w) CTRL2=0x$(setpci -s $b 0x11c.w) STATUS2=0x$(setpci -s $b 0x11e.w)"
"""
So we know,
PRE CAP=0x8bd7 CTRL2=0x0000 STATUS2=0x8000
==> RESET rc=1 elapsed_ms=114 err=[bash: line 9: echo: write error: Input/output error]
POST CAP=0x8bd7 CTRL2=0x0001 STATUS2=0x8003
device-present=1 reset_method=cxl_reset leaked_cxl_reset_iomem=0
with dmesg no output.
The write() to reset failed with -EIO after ~114 ms, but STATUS2 went 0x8000 -> 0x8003 .
The device completed the reset, so the kernel returned failure for a reset the HW did successfully.
CTRL2 went 0x0000 -> 0x0001, the device's CXL.cache is disabled after the "failed" reset.
~114ms is almost equal to msleep(100) in cxl_reset_wait_done() + the first poll. On the first poll
the Status2 read returns 0xffff, 0xffff has bit 2 set, which the code reads as RST_ERR -> return -EIO with no retry.
After that I ran the same "echo 1 > $dev/reset" in a 25x loop, logging rc, elapsed ms, CTRL2, STATUS2 at each iteration, then dmesg.
"""
PRE CAP=0x8bd7 CTRL2=0x0001 STATUS2=0x8003
iter 1 rc=1 ms= 114 CTRL2=0x0001 STATUS2=0x8003 present=1
iter 2 rc=1 ms= 171 CTRL2=0xffff STATUS2=0xffff present=1
iter 3 rc=1 ms= 6 CTRL2=0xffff STATUS2=0xffff present=1
[snip]
iter 25 rc=1 ms= 5 CTRL2=0xffff STATUS2=0xffff present=1
### rc histogram: rc=1 : 25x
### leaked cxl_reset iomem regions: 0
"""
The complete dmesg shows
"""
[ 1892.870193] ------------[ cut here ]------------
[ 1892.870215] index 7 is out of range for type 'resource_size_t [6]'
[ 1892.870218] CPU: 121 UID: 0 PID: 19436 Comm: bash Not tainted 7.1.0-rc7+ #1 PREEMPT(full)
[ 1892.870221] Hardware name: NVIDIA VR NVL72/P3809-BMC, BIOS NV_SBIOS: 06.02.00.00, OEM_SBIOS: 06.02.00.00 Mon
Jun 8 08:22:03 PM UTC 2026
[ 1892.870222] Call trace:
[ 1892.870223] show_stack+0x24/0x50 (C)
[ 1892.870229] dump_stack_lvl+0x80/0x140
[ 1892.870236] dump_stack+0x1c/0x38
[ 1892.870237] __ubsan_handle_out_of_bounds+0xd0/0x128
[ 1892.870242] pci_restore_iov_state+0x250/0x270
[ 1892.870249] pci_restore_state+0x10c/0x2c0
[ 1892.870251] pci_dev_restore+0x6c/0xb0
[ 1892.870252] pci_reset_function+0x94/0x160
[ 1892.870254] reset_store+0x78/0xf0
[ 1892.870258] dev_attr_store+0x24/0x78
[ 1892.870263] sysfs_kf_write+0x88/0xc8
[ 1892.870268] kernfs_fop_write_iter+0x170/0x228
[ 1892.870272] vfs_write+0x270/0x3a8
[ 1892.870276] ksys_write+0x7c/0x138
[ 1892.870277] __arm64_sys_write+0x28/0x50
[ 1892.870279] invoke_syscall.constprop.0+0xac/0x100
[ 1892.870282] do_el0_svc+0x4c/0x100
[ 1892.870283] el0_svc+0x50/0x2b0
[ 1892.870285] el0t_64_sync_handler+0xc0/0x108
[ 1892.870286] el0t_64_sync+0x1b8/0x1c0
[ 1892.870291] ---[ end trace ]---
[ 1892.870293] ------------[ cut here ]------------
[ 1892.870293] UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13
[ 1892.870295] shift exponent 64 is too large for 64-bit type 'long unsigned int'
[ 1892.870296] CPU: 121 UID: 0 PID: 19436 Comm: bash Not tainted 7.1.0-rc7+ #1 PREEMPT(full)
[ 1892.870297] Hardware name: NVIDIA VR NVL72/P3809-BMC, BIOS NV_SBIOS: 06.02.00.00, OEM_SBIOS: 06.02.00.00 Mon
Jun 8 08:22:03 PM UTC 2026
[ 1892.870298] Call trace:
[ 1892.870298] show_stack+0x24/0x50 (C)
[ 1892.870299] dump_stack_lvl+0x80/0x140
[ 1892.870300] dump_stack+0x1c/0x38
[ 1892.870301] __ubsan_handle_shift_out_of_bounds+0x154/0x260
[ 1892.870302] pci_rebar_bytes_to_size+0x98/0xc8
[ 1892.870305] pci_restore_iov_state+0x1f0/0x270
[ 1892.870306] pci_restore_state+0x10c/0x2c0
[ 1892.870307] pci_dev_restore+0x6c/0xb0
[ 1892.870308] pci_reset_function+0x94/0x160
[ 1892.870308] reset_store+0x78/0xf0
[ 1892.870309] dev_attr_store+0x24/0x78
[ 1892.870310] sysfs_kf_write+0x88/0xc8
[ 1892.870311] kernfs_fop_write_iter+0x170/0x228
[ 1892.870312] vfs_write+0x270/0x3a8
[ 1892.870313] ksys_write+0x7c/0x138
[ 1892.870314] __arm64_sys_write+0x28/0x50
[ 1892.870315] invoke_syscall.constprop.0+0xac/0x100
[ 1892.870315] do_el0_svc+0x4c/0x100
[ 1892.870316] el0_svc+0x50/0x2b0
[ 1892.870317] el0t_64_sync_handler+0xc0/0x108
[ 1892.870318] el0t_64_sync+0x1b8/0x1c0
[ 1892.870319] ---[ end trace ]---
"""
After iter 1, iter 2 took the device to CTRL2=0xffff STATUS2=0xffff, the config space returns all-ones,
which means the function stopped responding to config cycles and didn't self-recover.
During that iteration's pci_dev_restore(), the PCI core read the device's VF-ReBAR config, which is now 0xffffffff,
and used the garbage as array indices: bar_idx = 7 to dev->sriov->barsz[] which is sized 6. Two UBSAN reports.
Best regards,
Richard Cheng.
> Following Dan's v6 feedback, this replaces the proposed memdev sysfs ABI
> with the existing PCI reset_method interface.
>
> v7 changes from v6 [2]:
> - Move the ABI from a CXL memdev attribute to PCI reset_method.
> - Drop the memdev dependency from reset entry; advertise cxl_reset for
> Type 2 functions that report CXL Reset support in the CXL Device DVSEC.
> - Incorporate Dan's HDM reset refactor: shared decoder settings,
> pci_dev->hdm cached state, and built-in CONFIG_CXL_HDM helpers.
> - Cache endpoint HDM settings during PCI enumeration when MMIO decoding
> is already enabled, and let CXL core refresh the same cache later.
> - Reduce the earlier PCI/CXL save/restore series [3] to the HDM state
> cache and restore infrastructure needed by this reset flow.
> - Use cached HDM ranges to reject reset while affected ranges are busy
> and to invalidate CPU caches before reset.
> - Discover the CXL reset scope with the Non-CXL Function Map and CXL
> cache/mem capability bits.
> - Quiesce affected sibling functions with PCI save/disable and IOMMU
> reset prepare/done before executing reset.
> - Restore cached HDM decoder state after reset before completing PCI
> reset recovery.
> - Keep CXL Reset Memory Clear disabled.
>
> Motivation:
> -----------
> - Type 2 devices need a CXL-specific reset mechanism beyond existing PCI
> reset methods.
>
> - FLR does not reset CXL.cache or CXL.mem protocol state. CXL Reset is
> the architectural reset mechanism for those protocols.
>
> - The PCI reset_method ABI lets userspace select this narrower CXL reset
> before falling back to broader bus reset methods.
>
> Change Description:
> -------------------
>
> Patch 1: cxl/hdm: Split decoder programming into a reusable helper
> - Move shared decoder settings to include/cxl/cxl.h.
> - Factor low-level HDM register programming into cxl_commit().
>
> Patch 2: cxl/hdm: Cache decoder settings on PCI devices
> - Cache CXL core HDM decoder settings in pci_dev->hdm.
> - Refresh the cache as decoders are enumerated, committed, or reset.
>
> Patch 3: cxl/hdm: Cache endpoint decoder settings during PCI enumeration
> - Snapshot endpoint HDM state during PCI capability initialization when
> memory decoding is already enabled.
> - Reuse the same cache when CXL core later enumerates the device.
>
> Patch 4: PCI: Export pci_dev_save_and_disable() and pci_dev_restore()
> - Export PCI reset lifecycle helpers for CXL reset orchestration.
>
> Patch 5: PCI/CXL: Add CXL Device Reset helper
> - Add the internal DVSEC reset sequence.
> - Disable CXL.cache, perform cache writeback where supported, initiate
> CXL Reset, and wait for completion.
>
> Patch 6: PCI/CXL: Validate HDM ranges before CXL reset
> - Collect enabled cached HDM ranges.
> - Reject reset if affected ranges are busy and invalidate CPU caches.
>
> Patch 7: PCI/CXL: Discover the CXL reset scope
> - Discover same-scope CXL functions with the Non-CXL Function Map and
> CXL cache/mem capability bits.
>
> Patch 8: PCI/CXL: Coordinate sibling functions for CXL reset
> - Lock, save, disable, and IOMMU-block affected sibling functions.
> - Include mem-capable siblings in HDM range validation and cache flush.
>
> Patch 9: cxl/pci: Restore CXL HDM state after PCI reset
> - Restore cached global and per-decoder HDM state after reset.
> - Keep IOMMU reset blocks active until HDM restore completes.
>
> Patch 10: PCI/CXL: Expose CXL Reset as a PCI reset method
> - Add "cxl_reset" to the PCI reset_method table for Type 2 reset-capable
> CXL devices.
>
> Patch 11: Documentation/ABI: Document CXL Reset PCI reset method
> - Document the new reset_method value and reset behavior.
>
> The CPU cache invalidation step depends on
> cpu_cache_invalidate_memregion() support for the affected address ranges.
> If no provider is available, reset fails before hardware reset is
> requested.
>
> Example:
>
> echo cxl_reset > /sys/bus/pci/devices/0000:bb:dd.f/reset_method
> echo 1 > /sys/bus/pci/devices/0000:bb:dd.f/reset
>
> Basic CXL DVSEC reset testing was done on a CXL Type 2 device. The reset
> sequence completed successfully and ResetComplete was observed.
>
> References:
> [1] https://computeexpresslink.org/wp-content/uploads/2026/02/CXL-Specification_rev4p0_ver1p0_2026February26_clean_evalcopy_v2.pdf
> [2] https://lore.kernel.org/linux-cxl/20260528083154.137979-1-smadhavan@nvidia.com/
> [3] https://lore.kernel.org/linux-cxl/20260306080026.116789-1-smadhavan@nvidia.com/
>
> Srirangan Madhavan (11):
> cxl/hdm: Split decoder programming into a reusable helper
> cxl/hdm: Cache decoder settings on PCI devices
> cxl/hdm: Cache endpoint decoder settings during PCI enumeration
> PCI: Export pci_dev_save_and_disable() and pci_dev_restore()
> PCI/CXL: Add CXL Device Reset helper
> PCI/CXL: Validate HDM ranges before CXL reset
> PCI/CXL: Discover the CXL reset scope
> PCI/CXL: Coordinate sibling functions for CXL reset
> cxl/pci: Restore CXL HDM state after PCI reset
> PCI/CXL: Expose CXL Reset as a PCI reset method
> Documentation/ABI: Document CXL Reset PCI reset method
>
> Documentation/ABI/testing/sysfs-bus-pci | 14 +
> drivers/cxl/Kconfig | 4 +
> drivers/cxl/core/Makefile | 2 +-
> drivers/cxl/core/hdm.c | 234 ++---
> drivers/cxl/core/region.c | 6 +-
> drivers/cxl/core/reset.c | 1276 +++++++++++++++++++++++
> drivers/cxl/cxl.h | 43 -
> drivers/pci/pci.c | 25 +-
> drivers/pci/probe.c | 2 +
> include/cxl/cxl.h | 85 +-
> include/linux/pci.h | 10 +-
> include/uapi/linux/pci_regs.h | 15 +
> tools/testing/cxl/test/cxl.c | 10 +-
> 13 files changed, 1554 insertions(+), 172 deletions(-)
> create mode 100644 drivers/cxl/core/reset.c
>
> base-commit: 72afdd8181219f459142e571999b3b44ef7b85fb
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload
From: Rafael J. Wysocki @ 2026-06-24 13:20 UTC (permalink / raw)
To: Sumit Gupta
Cc: Rafael J. Wysocki, viresh.kumar, pierre.gondois, ionela.voinescu,
zhenglifeng1, zhanjie9, linux-kernel, linux-pm, linux-tegra,
treding, jonathanh, vsethi, ksitaraman, sanjayc, mochs, bbasu
In-Reply-To: <de03b23f-18a3-4e99-a15a-5044463ac484@nvidia.com>
On Wed, Jun 24, 2026 at 2:56 PM Sumit Gupta <sumitg@nvidia.com> wrote:
>
> Hi Rafael,
>
>
> On 23/06/26 16:30, Rafael J. Wysocki wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Tue, Jun 23, 2026 at 11:54 AM Sumit Gupta <sumitg@nvidia.com> wrote:
> >> Values written to OSPM-set CPPC registers (via sysfs or the autonomous
> >> boot parameter) can be lost in two ways:
> >>
> >> - Across CPU hotplug: the platform may reset a CPU's registers when it
> >> is offlined.
> >> - On driver unload: the value the driver wrote is left in the register
> >> instead of returning to its pre-driver state.
> >>
> >> Add a small table-driven mechanism that handles both:
> >>
> >> - Capture each register's firmware value when a CPU is first seen and
> >> restore it on driver unload.
> >> - Record the last value the driver set and reapply it from ->init()
> >> when the policy is reactivated after CPU hotplug.
> > I'm not sure if this is a good idea TBH.
> >
> > The overall system state when the CPU goes online may be completely
> > different from the system state when the CPU was online last time, so
> > there is no reason to restore its settings from before offline, at
> > least in principle.
>
> These are values userspace deliberately set, more like a frequency QoS
> request that persists until userspace changes it than transient state.
> It's platform dependent though, as the platform I test on preserves them
> across hotplug, but where the platform resets the register on offline
> the value is silently lost. Should the kernel re-apply it on online,
> or is that better left to userspace? If so, I will drop the hotplug reapply
> and keep only the restore on driver unload.
>
> intel_pstate and amd-pstate already re-sync these on CPU online, as they
> keep the policy across hotplug via ->online()/->offline(). So for them
> it's re-syncing the hardware, not restoring state from before offline.
>
> cppc_cpufreq has no ->online()/->offline() today, so it fully tears the
> policy down and rebuilds it, which is why this reads as "restoring
> settings from before offline".
> Would adding ->online()/->offline() be acceptable, with policy preserved
> and ->online() just re-syncing the registers the platform reset?
That would be a better approach IMV.
^ permalink raw reply
* Re: [PATCH] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload
From: Sumit Gupta @ 2026-06-24 12:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: viresh.kumar, pierre.gondois, ionela.voinescu, zhenglifeng1,
zhanjie9, linux-kernel, linux-pm, linux-tegra, treding, jonathanh,
vsethi, ksitaraman, sanjayc, mochs, bbasu, sumitg
In-Reply-To: <CAJZ5v0jvkNfx-1XChRCsGHJAcY7W4j41P+Xug6+TpPge7tHF7Q@mail.gmail.com>
Hi Rafael,
On 23/06/26 16:30, Rafael J. Wysocki wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Jun 23, 2026 at 11:54 AM Sumit Gupta <sumitg@nvidia.com> wrote:
>> Values written to OSPM-set CPPC registers (via sysfs or the autonomous
>> boot parameter) can be lost in two ways:
>>
>> - Across CPU hotplug: the platform may reset a CPU's registers when it
>> is offlined.
>> - On driver unload: the value the driver wrote is left in the register
>> instead of returning to its pre-driver state.
>>
>> Add a small table-driven mechanism that handles both:
>>
>> - Capture each register's firmware value when a CPU is first seen and
>> restore it on driver unload.
>> - Record the last value the driver set and reapply it from ->init()
>> when the policy is reactivated after CPU hotplug.
> I'm not sure if this is a good idea TBH.
>
> The overall system state when the CPU goes online may be completely
> different from the system state when the CPU was online last time, so
> there is no reason to restore its settings from before offline, at
> least in principle.
These are values userspace deliberately set, more like a frequency QoS
request that persists until userspace changes it than transient state.
It's platform dependent though, as the platform I test on preserves them
across hotplug, but where the platform resets the register on offline
the value is silently lost. Should the kernel re-apply it on online,
or is that better left to userspace? If so, I will drop the hotplug reapply
and keep only the restore on driver unload.
intel_pstate and amd-pstate already re-sync these on CPU online, as they
keep the policy across hotplug via ->online()/->offline(). So for them
it's re-syncing the hardware, not restoring state from before offline.
cppc_cpufreq has no ->online()/->offline() today, so it fully tears the
policy down and rebuilds it, which is why this reads as "restoring
settings from before offline".
Would adding ->online()/->offline() be acceptable, with policy preserved
and ->online() just re-syncing the registers the platform reset?
Thanks,
Sumit
>> The firmware value is captured on a CPU's first activation rather than
>> once at module load, so CPUs offline at boot or hot-added later are
>> covered.
>>
>> Reapply is only needed on a full policy teardown and bring-up, which goes
>> through ->init(). In a SHARED_TYPE_ANY policy, offlining a single CPU
>> leaves the shared register untouched, so nothing is lost there.
>>
>> Cover the OSPM Nominal Performance, Autonomous Selection
>> (auto_sel) and Energy Performance Preference (EPP) registers. For
>> auto_sel it replaces the previous unconditional
>> cppc_set_auto_sel(cpu, false) on unload with a restore of the firmware
>> value captured at the CPU's first init.
>>
>> Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>
>> This applies on top of (not yet merged):
>> [1] ACPI: CPPC: Add ospm_nominal_perf support
>> https://lore.kernel.org/lkml/20260615185934.2383514-1-sumitg@nvidia.com/
>> [2] cpufreq: CPPC: add autonomous mode boot parameter support
>> https://lore.kernel.org/lkml/20260623080652.3353386-1-sumitg@nvidia.com/
>>
>> drivers/cpufreq/cppc_cpufreq.c | 194 +++++++++++++++++++++++++++++++--
>> 1 file changed, 186 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index a3fabfb07fbe..d6ea2cbde187 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -28,6 +28,174 @@
>>
>> static struct cpufreq_driver cppc_cpufreq_driver;
>>
>> +/*
>> + * OSPM-set CPPC registers tracked for save/restore. A value set via sysfs or
>> + * the autonomous boot parameter is reapplied across CPU hotplug, and the
>> + * firmware value is restored on driver unload.
>> + */
>> +enum cppc_saved_reg_id {
>> + CPPC_SAVED_OSPM_NOMINAL_PERF,
>> + CPPC_SAVED_AUTO_SEL,
>> + CPPC_SAVED_EPP,
>> + CPPC_NR_SAVED_REGS,
>> +};
>> +
>> +struct cppc_saved_reg {
>> + int (*get)(int cpu, u64 *val);
>> + int (*set)(int cpu, u64 val);
>> +};
>> +
>> +/* u64 wrappers so the bool auto_sel register fits the table signatures. */
>> +static int cppc_get_auto_sel_u64(int cpu, u64 *val)
>> +{
>> + bool enable;
>> + int ret;
>> +
>> + ret = cppc_get_auto_sel(cpu, &enable);
>> + if (ret)
>> + return ret;
>> +
>> + *val = enable;
>> + return 0;
>> +}
>> +
>> +static int cppc_set_auto_sel_u64(int cpu, u64 val)
>> +{
>> + return cppc_set_auto_sel(cpu, !!val);
>> +}
>> +
>> +static const struct cppc_saved_reg cppc_saved_regs[CPPC_NR_SAVED_REGS] = {
>> + [CPPC_SAVED_OSPM_NOMINAL_PERF] = {
>> + cppc_get_ospm_nominal_perf, cppc_set_ospm_nominal_perf,
>> + },
>> + [CPPC_SAVED_AUTO_SEL] = {
>> + cppc_get_auto_sel_u64, cppc_set_auto_sel_u64,
>> + },
>> + [CPPC_SAVED_EPP] = {
>> + cppc_get_epp_perf, cppc_set_epp,
>> + },
>> +};
>> +
>> +/*
>> + * Per-CPU saved state for each register in cppc_saved_regs[]:
>> + * firmware_val - register value before the driver touched it, restored
>> + * on unload
>> + * requested_val - last value the driver set (sysfs or boot parameter),
>> + * reapplied on policy reactivation
>> + * firmware_captured - whether firmware_val has been read, so a not-yet-seen
>> + * CPU isn't mistaken for one whose firmware value is 0
>> + */
>> +struct cppc_saved_state {
>> + u64 firmware_val;
>> + u64 requested_val;
>> + bool firmware_captured;
>> +};
>> +
>> +/*
>> + * Per-CPU and not tied to a policy, so the saved values survive policy
>> + * teardown/bring-up across CPU hotplug. cpu_data->perf_ctrls is per-policy
>> + * and freed on policy ->exit.
>> + */
>> +static DEFINE_PER_CPU(struct cppc_saved_state[CPPC_NR_SAVED_REGS], cppc_saved_state);
>> +
>> +static void cppc_cache_perf_ctrls(struct cppc_cpudata *cpu_data,
>> + enum cppc_saved_reg_id reg, u64 val)
>> +{
>> + switch (reg) {
>> + case CPPC_SAVED_AUTO_SEL:
>> + cpu_data->perf_ctrls.auto_sel = val;
>> + break;
>> + case CPPC_SAVED_EPP:
>> + cpu_data->perf_ctrls.energy_perf = val;
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +/*
>> + * Save the requested value for the given register and CPU, to be reapplied when
>> + * the policy is reactivated after CPU hotplug. Also update the per-policy
>> + * perf_ctrls copy so the saved and current values stay in sync.
>> + */
>> +static void cppc_save_requested(struct cppc_cpudata *cpu_data, unsigned int cpu,
>> + enum cppc_saved_reg_id reg, u64 val)
>> +{
>> + per_cpu(cppc_saved_state, cpu)[reg].requested_val = val;
>> + cppc_cache_perf_ctrls(cpu_data, reg, val);
>> +}
>> +
>> +/*
>> + * Reapply each register's last requested value from ->init(), so a value set
>> + * via sysfs or the boot parameter survives a policy teardown and bring-up
>> + * across CPU hotplug. Also keep perf_ctrls in sync with it.
>> + */
>> +static void cppc_cpufreq_reapply_requested_regs(struct cpufreq_policy *policy)
>> +{
>> + struct cppc_cpudata *cpu_data = policy->driver_data;
>> + unsigned int cpu, i;
>> + u64 val;
>> +
>> + for_each_cpu(cpu, policy->cpus) {
>> + for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
>> + val = per_cpu(cppc_saved_state, cpu)[i].requested_val;
>> + if (val == U64_MAX)
>> + continue;
>> +
>> + cppc_saved_regs[i].set(cpu, val);
>> +
>> + /* Keep perf_ctrls in sync via the policy's CPU. */
>> + if (cpu == policy->cpu)
>> + cppc_cache_perf_ctrls(cpu_data, i, val);
>> + }
>> + }
>> +}
>> +
>> +/*
>> + * On a CPU's first ->init(), capture each register's firmware value to be
>> + * restored on driver unload. Later calls for the same CPU are a no-op. Capturing
>> + * from ->init() rather than module load covers CPUs that appear later. Also seed
>> + * requested_val to U64_MAX so its zeroed default is not taken as a request for 0.
>> + */
>> +static void cppc_cpufreq_save_firmware_regs(struct cpufreq_policy *policy)
>> +{
>> + unsigned int cpu, i;
>> + u64 val;
>> +
>> + for_each_cpu(cpu, policy->cpus) {
>> + for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
>> + struct cppc_saved_state *s =
>> + &per_cpu(cppc_saved_state, cpu)[i];
>> +
>> + /* Capture once per CPU; skip if already recorded. */
>> + if (s->firmware_captured)
>> + continue;
>> +
>> + if (cppc_saved_regs[i].get(cpu, &val))
>> + val = U64_MAX;
>> + s->firmware_val = val;
>> + s->requested_val = U64_MAX;
>> + s->firmware_captured = true;
>> + }
>> + }
>> +}
>> +
>> +/* On driver unload, restore each captured CPU's firmware value. */
>> +static void cppc_cpufreq_restore_firmware_regs(void)
>> +{
>> + unsigned int cpu, i;
>> +
>> + for_each_present_cpu(cpu) {
>> + for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
>> + struct cppc_saved_state *s =
>> + &per_cpu(cppc_saved_state, cpu)[i];
>> +
>> + if (s->firmware_captured && s->firmware_val != U64_MAX)
>> + cppc_saved_regs[i].set(cpu, s->firmware_val);
>> + }
>> + }
>> +}
>> +
>> /* Autonomous Selection boot parameter modes */
>> enum {
>> AUTO_SEL_DISABLED = 0,
>> @@ -766,6 +934,9 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
>> cpu_data->perf_ctrls.desired_perf = caps->highest_perf;
>>
>> + /* Capture a CPU's firmware values on its first init, before any driver write. */
>> + cppc_cpufreq_save_firmware_regs(policy);
>> +
>> /*
>> * Enable autonomous mode on first init if boot param is set.
>> * Check last_governor to detect first init and skip if auto_sel
>> @@ -812,7 +983,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> if (ret && ret != -EOPNOTSUPP)
>> pr_warn("Failed to set EPP for CPU%d (%d)\n", cpu, ret);
>> else if (!ret)
>> - cpu_data->perf_ctrls.energy_perf = epp;
>> + cppc_save_requested(cpu_data, cpu, CPPC_SAVED_EPP, epp);
>> }
>>
>> /* Program min/max/desired into CPPC regs (non-fatal on failure). */
>> @@ -826,7 +997,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> pr_warn("auto_sel CPU%d failed (%d); using OS mode\n",
>> cpu, ret);
>> else if (!ret)
>> - cpu_data->perf_ctrls.auto_sel = true;
>> + cppc_save_requested(cpu_data, cpu, CPPC_SAVED_AUTO_SEL, true);
>> }
>>
>> if (cpu_data->perf_ctrls.auto_sel) {
>> @@ -850,6 +1021,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> }
>>
>> cppc_cpufreq_cpu_fie_init(policy);
>> +
>> + /* Reapply any saved values lost across a full policy teardown. */
>> + cppc_cpufreq_reapply_requested_regs(policy);
>> +
>> return 0;
>>
>> out:
>> @@ -1039,6 +1214,8 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
>> }
>> }
>>
>> + cppc_save_requested(cpu_data, policy->cpu, CPPC_SAVED_AUTO_SEL, val);
>> +
>> return count;
>> }
>>
>> @@ -1111,7 +1288,7 @@ store_energy_performance_preference_val(struct cpufreq_policy *policy,
>> if (ret)
>> return ret;
>>
>> - cpu_data->perf_ctrls.energy_perf = val;
>> + cppc_save_requested(cpu_data, policy->cpu, CPPC_SAVED_EPP, val);
>>
>> return count;
>> }
>> @@ -1193,6 +1370,9 @@ static ssize_t store_ospm_nominal_freq(struct cpufreq_policy *policy,
>> }
>> }
>>
>> + for_each_cpu(sib, policy->cpus)
>> + cppc_save_requested(cpu_data, sib, CPPC_SAVED_OSPM_NOMINAL_PERF, perf);
>> +
>> return count;
>>
>> rollback:
>> @@ -1258,13 +1438,11 @@ static int __init cppc_cpufreq_init(void)
>>
>> static void __exit cppc_cpufreq_exit(void)
>> {
>> - unsigned int cpu;
>> -
>> - for_each_present_cpu(cpu)
>> - cppc_set_auto_sel(cpu, false);
>> -
>> cpufreq_unregister_driver(&cppc_cpufreq_driver);
>> cppc_freq_invariance_exit();
>> +
>> + /* Restore auto_sel and the other saved registers to their firmware value. */
>> + cppc_cpufreq_restore_firmware_regs();
>> }
>>
>> module_param_cb(auto_sel_mode, &auto_sel_mode_ops, &auto_sel_mode, 0444);
>> --
>> 2.34.1
>>
>>
^ permalink raw reply
* Re: [PATCH v6 02/10] arm64: tegra: Remove fallback compatible for GPCDMA
From: Thierry Reding @ 2026-06-24 12:35 UTC (permalink / raw)
To: Rob Herring
Cc: Akhil R, Thierry Reding, Vinod Koul, Frank Li,
Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
Laxman Dewangan, Philipp Zabel, dmaengine, devicetree,
linux-tegra, linux-kernel
In-Reply-To: <ajuv2CVQ-b978cn6@orome>
[-- Attachment #1: Type: text/plain, Size: 967 bytes --]
On Wed, Jun 24, 2026 at 12:22:38PM +0200, Thierry Reding wrote:
> On Tue, Jun 23, 2026 at 09:02:39AM -0500, Rob Herring wrote:
> > On Tue, Mar 31, 2026 at 5:24 AM Akhil R <akhilrajeev@nvidia.com> wrote:
> > >
> > > Remove the fallback compatible string "nvidia,tegra186-gpcdma" for GPCDMA
> > > in Tegra264. Tegra186 compatible cannot work on Tegra264 because of the
> > > register offset changes and absence of the reset property.
> > >
> > > Fixes: 65ef237e4810 ("arm64: tegra: Add Tegra264 support")
> > > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > > ---
> > > arch/arm64/boot/dts/nvidia/tegra264.dtsi | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Thierry, Are you going to apply this? The binding change has been
> > picked up and now there's a warning.
>
> Yes, I have this in my tree of fixes for 7.1 and plan to send it out
> towards the end of this week.
Sorry, fixes for 7.2-rc1, that is.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v7 3/4] PCI: tegra: Add Tegra264 support
From: Thierry Reding @ 2026-06-24 12:35 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter, Karthikeyan Mitran, Hou Zhiqiang,
Thomas Petazzoni, Pali Rohár, Michal Simek, Kevin Xie,
Aksh Garg, linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding, Manikanta Maddireddy
In-Reply-To: <slfaxyt6p5mwsqmxvmriy6npilpjhjxv5ruegj4hnivj6zufkl@o6adjy5jmzfy>
[-- Attachment #1: Type: text/plain, Size: 8181 bytes --]
On Thu, Jun 18, 2026 at 09:26:33AM +0200, Manivannan Sadhasivam wrote:
> On Wed, Jun 17, 2026 at 06:01:30PM +0200, Thierry Reding wrote:
[...]
> > +static int tegra264_pcie_parse_dt(struct tegra264_pcie *pcie)
> > +{
> > + struct device *dev = pcie->dev;
> > + int err;
> > +
> > + pcie->wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
> > + if (IS_ERR(pcie->wake_gpio))
> > + return PTR_ERR(pcie->wake_gpio);
> > +
> > + if (!pcie->wake_gpio)
> > + return 0;
> > +
> > + err = gpiod_to_irq(pcie->wake_gpio);
> > + if (err < 0)
> > + return dev_err_probe(dev, err, "failed to get wake IRQ\n");
> > +
> > + pcie->wake_irq = (unsigned int)err;
> > +
> > + err = devm_device_init_wakeup(dev);
> > + if (err < 0)
> > + return dev_err_probe(dev, err, "failed to initialize wakeup\n");
> > +
> > + err = devm_pm_set_wake_irq(dev, pcie->wake_irq);
> > + if (err < 0)
> > + return dev_err_probe(dev, err, "failed to set wakeup IRQ\n");
> > +
>
> I'd really like to get rid of custom WAKE# handling in the controller drivers.
> Krishna is trying to add generic WAKE# handling in the PCI core and I'd suggest
> you to take a look at the patches:
> https://lore.kernel.org/linux-pci/20260511-wakeirq_support-v10-2-c10af9c9eb8c@oss.qualcomm.com/
>
> But this also means that you need to use switch to Root Port binding to move the
> Root Port properties out of the controller node. This is something we are
> mandating for the new controllers. Not a big change though...
>
> Reference:
>
> Documentation/devicetree/bindings/pci/spacemit,k1-pcie-host.yaml#n80
Okay, let me look into it.
[...]
> > + err = devm_pm_runtime_set_active_enabled(dev);
>
> I belive this has to come after pm_runtime_get_sync() because at this point, the
> controller is not enabled.
Okay, I'll change that.
> > + if (err < 0) {
> > + dev_err_probe(dev, err, "failed to enable runtime PM\n");
> > + goto put_bpmp;
> > + }
> > +
> > + err = pm_runtime_get_sync(dev);
> > + if (err < 0) {
> > + dev_err_probe(dev, err, "failed to power on device\n");
> > + goto put_bpmp;
> > + }
> > +
> > + /* sanity check that programmed ranges match what's in DT */
> > + if (!tegra264_pcie_check_ranges(pdev)) {
> > + err = -EINVAL;
> > + goto put_pm;
> > + }
> > +
> > + pcie->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
> > + if (IS_ERR(pcie->cfg)) {
> > + err = dev_err_probe(dev, PTR_ERR(pcie->cfg),
> > + "failed to create ECAM\n");
> > + goto put_pm;
> > + }
> > +
> > + bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
> > + bridge->sysdata = pcie->cfg;
> > + pcie->ecam = pcie->cfg->win;
> > +
> > + tegra264_pcie_init(pcie);
> > +
> > + if (!pcie->link_up)
> > + return 0;
>
> So not hotplug support? Also, you do not want the driver to error out? I'm
> wondering what's the use then?
Hotplug is supported via pciehp. We skip probing the host bridge if no
link was detected because there's simply nothing attached to the port,
otherwise the link would've come up.
pcie->link_up is slightly misleading because it actually means something
along the lines of "link could be up at some point", either during probe
or after some hotplug event later on. It is only ever false if there's
no link during probe and hotplug isn't supported at all.
> > +
> > + err = pci_host_probe(bridge);
> > + if (err < 0) {
> > + dev_err_probe(dev, err, "failed to register host\n");
> > + goto free_ecam;
> > + }
> > +
> > + return 0;
> > +
> > +free_ecam:
>
> Nit: Prefix 'err' for the labels.
I don't see any benefit of adding a prefix. Seems pretty redundant, but
I also don't feel too strongly about it, so I can add it.
> > + pci_ecam_free(pcie->cfg);
> > +put_pm:
> > + pm_runtime_put_sync(dev);
> > +put_bpmp:
> > + tegra_bpmp_put(pcie->bpmp);
> > +
> > + return err;
> > +}
> > +
> > +static void tegra264_pcie_remove(struct platform_device *pdev)
> > +{
> > + struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
> > +
> > + /*
> > + * If we undo tegra264_pcie_init() then link goes down and need
> > + * controller reset to bring up the link again. Remove intention is
> > + * to clean up the root bridge and re-enumerate during bind.
>
> But the controller will be consuming power even if PCIe is not used. Do you
> really want that? Can't tegra264_pcie_init() handle the initialization? I'm
> wondering how tegra264_pcie_deinit() in tegra264_pcie_suspend() works then.
I had to clarify this with the PCI team and they indicated that
tegra264_pcie_deinit() is actually useless and maybe even harmful. The
reason is that there's a processor on these boards (BPMP) that takes
care of power sequencing and it will automatically take the PCI links
to L2 on suspend and assert PERST#.
Another reason why we don't want to reset the entire controller is that
it is already set up during early boot by UEFI and the kernel driver
does not redo the entire initialization.
So yes, I think a little bit of power consumption is the compromise that
we will have to live with. In the bigger picture it's probably not going
to be noticeable in most cases, and given that these are embedded
platforms we'll likely see fixed configurations most of the time and the
case where we remove the PCIe host controller will not be common.
> > + */
> > + pci_lock_rescan_remove();
> > + pci_stop_root_bus(pcie->bridge->bus);
> > + pci_remove_root_bus(pcie->bridge->bus);
> > + pci_unlock_rescan_remove();
> > +
> > + pm_runtime_put_sync(&pdev->dev);
> > + tegra_bpmp_put(pcie->bpmp);
> > + pci_ecam_free(pcie->cfg);
> > +}
> > +
> > +static int tegra264_pcie_suspend(struct device *dev)
> > +{
> > + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > + int err;
> > +
> > + tegra264_pcie_deinit(pcie);
> > +
> > + if (pcie->wake_gpio && device_may_wakeup(dev)) {
> > + err = enable_irq_wake(pcie->wake_irq);
> > + if (err < 0)
> > + dev_err(dev, "failed to enable wake IRQ: %pe\n",
> > + ERR_PTR(err));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tegra264_pcie_resume(struct device *dev)
> > +{
> > + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > + int err;
> > +
> > + err = pinctrl_pm_select_default_state(dev);
> > + if (err < 0)
> > + dev_err(dev, "failed to configure sideband pins: %pe\n",
> > + ERR_PTR(err));
>
> Please remind me if you justified this manual pinctrl handling before.
This is just regular pinctrl PM boilerplate. There's plenty of other
drivers where we do this, too. We want this because some of the pins
get configured to non-default states on boot/resume, so doing this
here ensures they are muxed correctly.
> > +
> > + if (pcie->wake_gpio && device_may_wakeup(dev)) {
> > + err = disable_irq_wake(pcie->wake_irq);
> > + if (err < 0)
> > + dev_err(dev, "failed to disable wake IRQ: %pe\n",
> > + ERR_PTR(err));
> > + }
> > +
> > + if (pcie->link_up == false)
> > + return 0;
>
> How is this possible? If 'pcie->link_up' was 'false' during probe(), then it is
> going to stay until tegra264_pcie_init() is called below.
Yes, this keeps confusing me, too. The purpose of this is to skip
initialization if we've already determined during probe that there is
never going to be a link. link_up will be false if and only if there was
no link during probe and we don't expect there ever will be a link
because there is no hotplug support.
Maybe a different name for link_up could help here? maybe_link_up
perhaps? I don't know if that's any clearer, but I also couldn't come up
with a better name.
Or maybe we should split this into two booleans, since we're essentially
trying to use one boolean to track a tristate. What we want to know is
if a link is truly up and if the controller should be kept powered for
the case where hotplug is supported.
I suppose we could do:
bool link_up; /* track the link state */
bool supports_hotplug; /* track whether port is hotpluggable */
That would make the code a bit cleaner and easier to follow.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v5 12/12] arm64: defconfig: Enable I3C and SPD5118 hwmon
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Enable I3C subsystem (I3C), Synopsys DesignWare I3C master controller
(DW_I3C_MASTER), and SPD5118 hwmon temperature sensor (SENSORS_SPD5118)
as modules.
The NVIDIA Vera CPU uses SOCAMM LPDDR5X memory module, which contains
SPD5118 (JEDEC JESD300) compliant temperature sensor. This sensor is
accessible over the I3C bus through the DesignWare I3C controller present
on the SoC. Enabling these configs allows monitoring memory module
temperatures on platforms such as Vera Rubin. Vera is an ACPI-based
platform and does not use device tree.
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
arch/arm64/configs/defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f2e6ae93e533..65d9eb56e978 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -640,6 +640,8 @@ CONFIG_I2C_UNIPHIER_F=y
CONFIG_I2C_XILINX=m
CONFIG_I2C_RCAR=y
CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_I3C=m
+CONFIG_DW_I3C_MASTER=m
CONFIG_SPI=y
CONFIG_SPI_APPLE=m
CONFIG_SPI_ARMADA_3700=y
@@ -769,6 +771,7 @@ CONFIG_SENSORS_SL28CPLD=m
CONFIG_SENSORS_AMC6821=m
CONFIG_SENSORS_INA2XX=m
CONFIG_SENSORS_INA3221=m
+CONFIG_SENSORS_SPD5118=m
CONFIG_SENSORS_TMP102=m
CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
CONFIG_CPU_THERMAL=y
--
2.43.0
^ permalink raw reply related
* [PATCH v5 11/12] hwmon: spd5118: Add I3C support
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Add a regmap config and a probe function to support I3C-based
communication with SPD5118 devices.
On an I3C bus, SPD5118 devices are enumerated via SETAASA and always
require an ACPI or device tree entry. Device matching is hence through
the OF match tables only and does not need an I3C class match table. The
device identity is verified in the type registers before proceeding to
the common probe function.
Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/hwmon/Kconfig | 9 ++++---
drivers/hwmon/spd5118.c | 56 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5c2d3ff5fce8..c4bf5475fcb3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2354,12 +2354,15 @@ config SENSORS_INA3221
config SENSORS_SPD5118
tristate "SPD5118 Compliant Temperature Sensors"
- depends on I2C
+ depends on I3C_OR_I2C
select REGMAP_I2C
+ select REGMAP_I3C if I3C
help
If you say yes here you get support for SPD5118 (JEDEC JESD300)
- compliant temperature sensors. Such sensors are found on DDR5 memory
- modules.
+ compliant temperature sensors using I2C or I3C bus interface.
+ Such sensors are found on DDR5 memory modules.
+
+ This driver supports both I2C and I3C interfaces.
This driver can also be built as a module. If so, the module
will be called spd5118.
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 6ba37a719300..9724cf70b61d 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -18,6 +18,7 @@
#include <linux/bits.h>
#include <linux/err.h>
#include <linux/i2c.h>
+#include <linux/i3c/device.h>
#include <linux/hwmon.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -464,6 +465,27 @@ static const struct regmap_config spd5118_regmap8_config = {
.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
};
+/*
+ * SPD5118 2-byte register address format (JESD300-5, Tables 7 & 20):
+ * Byte 1 (on wire first): MemReg | BlkAddr[0] | Address[5:0]
+ * Byte 2 (on wire second): 0000 | BlkAddr[4:1]
+ *
+ * The address byte (with MemReg and lower address bits) must be sent first,
+ * followed by the upper block address byte. With regmap 16-bit register
+ * format, this maps to little-endian: the low byte of the 16-bit value is
+ * transmitted first. No range config is needed since I3C does not use MR11
+ * page switching.
+ */
+static const struct regmap_config spd5118_regmap_i3c_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0x7ff,
+ .reg_format_endian = REGMAP_ENDIAN_LITTLE,
+ .writeable_reg = spd5118_writeable_reg,
+ .volatile_reg = spd5118_volatile_reg,
+ .cache_type = REGCACHE_MAPLE,
+};
+
static int spd5118_suspend(struct device *dev)
{
struct spd5118_data *data = dev_get_drvdata(dev);
@@ -701,7 +723,39 @@ static struct i2c_driver spd5118_i2c_driver = {
.address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
};
-module_i2c_driver(spd5118_i2c_driver);
+/* I3C */
+
+static int spd5118_i3c_probe(struct i3c_device *i3cdev)
+{
+ struct device *dev = i3cdev_to_dev(i3cdev);
+ struct regmap *regmap;
+ u8 regval[2];
+ int err;
+
+ regmap = devm_regmap_init_i3c(i3cdev, &spd5118_regmap_i3c_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
+
+ err = regmap_bulk_read(regmap, SPD5118_REG_TYPE, regval, 2);
+ if (err)
+ return dev_err_probe(dev, err, "failed to read device type\n");
+
+ if (regval[0] != 0x51 || regval[1] != 0x18)
+ return -ENODEV;
+
+ return spd5118_common_probe(dev, regmap);
+}
+
+static struct i3c_driver spd5118_i3c_driver = {
+ .driver = {
+ .name = "spd5118_i3c",
+ .of_match_table = spd5118_of_ids,
+ .pm = pm_sleep_ptr(&spd5118_pm_ops),
+ },
+ .probe = spd5118_i3c_probe,
+};
+
+module_i3c_i2c_driver(spd5118_i3c_driver, &spd5118_i2c_driver);
MODULE_AUTHOR("René Rebe <rene@exactcode.de>");
MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
--
2.43.0
^ permalink raw reply related
* [PATCH v5 10/12] hwmon: spd5118: Remove 16-bit addressing
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
The intent of introducing 16-bit addressing was to support I3C, but it
turns out that I3C does not require reading the Legacy Mode register,
nor any specific encoding for page translation. The testing of 16-bit
code was limited and there are no known users for this feature. Remove
the sections that support 16-bit addressing and prepare the driver to
support I3C appropriately.
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/hwmon/spd5118.c | 79 +++--------------------------------------
1 file changed, 5 insertions(+), 74 deletions(-)
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index cc40661cab21..6ba37a719300 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -66,9 +66,6 @@ static const unsigned short normal_i2c[] = {
#define SPD5118_EEPROM_BASE 0x80
#define SPD5118_EEPROM_SIZE (SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES)
-#define PAGE_ADDR0(page) (((page) & BIT(0)) << 6)
-#define PAGE_ADDR1_4(page) (((page) & GENMASK(4, 1)) >> 1)
-
/* Temperature unit in millicelsius */
#define SPD5118_TEMP_UNIT (MILLIDEGREE_PER_DEGREE / 4)
/* Representable temperature range in millicelsius */
@@ -78,7 +75,6 @@ static const unsigned short normal_i2c[] = {
struct spd5118_data {
struct regmap *regmap;
struct mutex nvmem_lock;
- bool is_16bit;
};
/* hwmon */
@@ -348,12 +344,7 @@ static ssize_t spd5118_nvmem_read_page(struct spd5118_data *data, char *buf,
if (offset + count > SPD5118_PAGE_SIZE)
count = SPD5118_PAGE_SIZE - offset;
- if (data->is_16bit) {
- addr = SPD5118_EEPROM_BASE | PAGE_ADDR0(page) |
- (PAGE_ADDR1_4(page) << 8);
- } else {
- addr = page * 0x100 + SPD5118_EEPROM_BASE;
- }
+ addr = page * 0x100 + SPD5118_EEPROM_BASE;
err = regmap_bulk_read(regmap, addr + offset, buf, count);
if (err)
return err;
@@ -473,15 +464,6 @@ static const struct regmap_config spd5118_regmap8_config = {
.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
};
-static const struct regmap_config spd5118_regmap16_config = {
- .reg_bits = 16,
- .val_bits = 8,
- .max_register = 0x7ff,
- .writeable_reg = spd5118_writeable_reg,
- .volatile_reg = spd5118_volatile_reg,
- .cache_type = REGCACHE_MAPLE,
-};
-
static int spd5118_suspend(struct device *dev)
{
struct spd5118_data *data = dev_get_drvdata(dev);
@@ -519,8 +501,7 @@ static int spd5118_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
-static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
- bool is_16bit)
+static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
{
unsigned int capability, revision, vendor, bank;
struct spd5118_data *data;
@@ -537,8 +518,6 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
if (!(capability & SPD5118_CAP_TS_SUPPORT))
return -ENODEV;
- data->is_16bit = is_16bit;
-
err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
if (err)
return err;
@@ -680,69 +659,21 @@ static int spd5118_i2c_init(struct i2c_client *client)
return 0;
}
-/*
- * 16-bit addressing note:
- *
- * If I2C_FUNC_I2C is not supported by an I2C adapter driver, regmap uses
- * SMBus operations as alternative. To simulate a read operation with a 16-bit
- * address, it writes the address using i2c_smbus_write_byte_data(), followed
- * by one or more calls to i2c_smbus_read_byte() to read the data.
- * Per spd5118 standard, a read operation after writing the address must start
- * with <Sr> (Repeat Start). However, a SMBus read byte operation starts with
- * <S> (Start). This resets the register address in the spd5118 chip. As result,
- * i2c_smbus_read_byte() always returns data from register address 0x00.
- *
- * A working alternative to access chips with 16-bit register addresses in the
- * absence of I2C_FUNC_I2C support is not known.
- *
- * For this reason, 16-bit addressing can only be supported with I2C if the
- * adapter supports I2C_FUNC_I2C.
- *
- * For I2C, the addressing mode selected by the BIOS must not be changed.
- * Experiments show that at least some PC BIOS versions will not change the
- * addressing mode on a soft reboot and end up in setup, claiming that some
- * configuration change happened. This will happen again after a power cycle,
- * which does reset the addressing mode. To prevent this from happening,
- * detect if 16-bit addressing is enabled and always use the currently
- * configured addressing mode.
- */
-
static int spd5118_i2c_probe(struct i2c_client *client)
{
- const struct regmap_config *config;
struct device *dev = &client->dev;
struct regmap *regmap;
- int err, mode;
- bool is_16bit;
+ int err;
err = spd5118_i2c_init(client);
if (err)
return err;
- mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
- if (mode < 0)
- return mode;
-
- is_16bit = mode & SPD5118_LEGACY_MODE_ADDR;
- if (is_16bit) {
- /*
- * See 16-bit addressing note above explaining why it is
- * necessary to check for I2C_FUNC_I2C support here.
- */
- if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
- dev_err(dev, "Adapter does not support 16-bit register addresses\n");
- return -ENODEV;
- }
- config = &spd5118_regmap16_config;
- } else {
- config = &spd5118_regmap8_config;
- }
-
- regmap = devm_regmap_init_i2c(client, config);
+ regmap = devm_regmap_init_i2c(client, &spd5118_regmap8_config);
if (IS_ERR(regmap))
return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
- return spd5118_common_probe(dev, regmap, is_16bit);
+ return spd5118_common_probe(dev, regmap);
}
static const struct i2c_device_id spd5118_i2c_id[] = {
--
2.43.0
^ permalink raw reply related
* [PATCH v5 09/12] i3c: dw-i3c-master: Add ACPI ID for Tegra410
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Update variable names to generic names and add Tegra410 ACPI ID to
support the I3C controller in Tegra410, which is a DesignWare I3C host
controller.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master/dw-i3c-master.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 8e40d178d500..0753f6c853b7 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1855,11 +1855,12 @@ static const struct of_device_id dw_i3c_master_of_match[] = {
};
MODULE_DEVICE_TABLE(of, dw_i3c_master_of_match);
-static const struct acpi_device_id amd_i3c_device_match[] = {
+static const struct acpi_device_id dw_i3c_master_acpi_match[] = {
{ "AMDI0015", AMD_I3C_OD_PP_TIMING },
+ { "NVDA2018", DW_I3C_ACPI_SKIP_CLK_RST },
{ }
};
-MODULE_DEVICE_TABLE(acpi, amd_i3c_device_match);
+MODULE_DEVICE_TABLE(acpi, dw_i3c_master_acpi_match);
static struct platform_driver dw_i3c_driver = {
.probe = dw_i3c_probe,
@@ -1868,7 +1869,7 @@ static struct platform_driver dw_i3c_driver = {
.driver = {
.name = "dw-i3c-master",
.of_match_table = dw_i3c_master_of_match,
- .acpi_match_table = amd_i3c_device_match,
+ .acpi_match_table = dw_i3c_master_acpi_match,
.pm = &dw_i3c_pm_ops,
},
};
--
2.43.0
^ permalink raw reply related
* [PATCH v5 08/12] i3c: dw-i3c-master: Add ACPI core clock frequency quirk
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Some ACPI-enumerated devices like Tegra410 do not expose the controller
core clock through the clk framework. Unlike device tree, ACPI on Arm does
not model clock providers. The hardware is expected to have its clocks
enabled by firmware before the OS takes over.
Make the core clock optional and allow selected ACPI devices to provide the
core clock rate through the "clock-frequency" _DSD property when the core
clock is absent.
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master/dw-i3c-master.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 29030fd9594a..8e40d178d500 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -242,6 +242,7 @@
/* List of quirks */
#define AMD_I3C_OD_PP_TIMING BIT(1)
#define DW_I3C_DISABLE_RUNTIME_PM_QUIRK BIT(2)
+#define DW_I3C_ACPI_SKIP_CLK_RST BIT(3)
struct dw_i3c_cmd {
u32 cmd_lo;
@@ -556,13 +557,33 @@ static void dw_i3c_master_set_intr_regs(struct dw_i3c_master *master)
writel(IBI_REQ_REJECT_ALL, master->regs + IBI_MR_REQ_REJECT);
}
+static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
+{
+ unsigned int core_rate_prop;
+
+ if (master->core_clk)
+ return clk_get_rate(master->core_clk);
+
+ if (!(master->quirks & DW_I3C_ACPI_SKIP_CLK_RST)) {
+ dev_err(master->dev, "missing core clock\n");
+ return 0;
+ }
+
+ if (device_property_read_u32(master->dev, "clock-frequency", &core_rate_prop)) {
+ dev_err(master->dev, "missing clock-frequency property\n");
+ return 0;
+ }
+
+ return core_rate_prop;
+}
+
static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
{
unsigned long core_rate, core_period;
u32 scl_timing;
u8 hcnt, lcnt;
- core_rate = clk_get_rate(master->core_clk);
+ core_rate = dw_i3c_master_get_core_rate(master);
if (!core_rate)
return -EINVAL;
@@ -615,7 +636,7 @@ static int dw_i2c_clk_cfg(struct dw_i3c_master *master)
u16 hcnt, lcnt;
u32 scl_timing;
- core_rate = clk_get_rate(master->core_clk);
+ core_rate = dw_i3c_master_get_core_rate(master);
if (!core_rate)
return -EINVAL;
@@ -1577,7 +1598,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
if (IS_ERR(master->regs))
return PTR_ERR(master->regs);
- master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
+ master->core_clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
if (IS_ERR(master->core_clk))
return PTR_ERR(master->core_clk);
--
2.43.0
^ permalink raw reply related
* [PATCH v5 07/12] i3c: dw-i3c-master: Add SETAASA as supported CCC
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Add SETAASA and SETHID to the supported list of CCC commands for
DesignWare I3C host controller.
SETAASA is a broadcast command that assigns predefined static addresses
to all I3C devices on the bus.
SETHID is to stop HID bit flipping by the SPD Hub to which the SPD devices
are connected. It is a prerequisite command to be sent before SETAASA as
recommended by JESD300-5 and JESD403 sideband bus specifications.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master/dw-i3c-master.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 2f8c0c4683e0..29030fd9594a 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -309,6 +309,8 @@ static bool dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
case I3C_CCC_GETSTATUS:
case I3C_CCC_GETMXDS:
case I3C_CCC_GETHDRCAP:
+ case I3C_CCC_SETAASA:
+ case I3C_CCC_VENDOR(0, true): /* SETHID */
return true;
default:
return false;
--
2.43.0
^ permalink raw reply related
* [PATCH v5 06/12] i3c: master: match I3C device through DT and ACPI
From: Akhil R @ 2026-06-24 10:21 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
SETAASA-based devices cannot always be identified by PID or DCR; the
standard I3C id_table matching may not be applicable. Allow such devices to
match through Device Tree or ACPI.
Emit OF and ACPI modaliases so firmware-matched devices can autoload their
drivers.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index bd0dc76c7ba1..f513169ac395 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/slab.h>
@@ -320,6 +321,15 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
const struct i3c_device *i3cdev = dev_to_i3cdev(dev);
struct i3c_device_info devinfo;
u16 manuf, part, ext;
+ int rc;
+
+ rc = of_device_uevent_modalias(dev, env);
+ if (rc != -ENODEV)
+ return rc;
+
+ rc = acpi_device_uevent_modalias(dev, env);
+ if (rc != -ENODEV)
+ return rc;
if (i3cdev->desc)
devinfo = i3cdev->desc->info;
@@ -345,15 +355,32 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
{
struct i3c_device *i3cdev;
const struct i3c_driver *i3cdrv;
+ u8 static_addr_method = 0;
if (dev->type != &i3c_device_type)
return 0;
i3cdev = dev_to_i3cdev(dev);
i3cdrv = drv_to_i3cdrv(drv);
- if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
+
+ if (i3cdev->desc && i3cdev->desc->boardinfo)
+ static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
+
+ /*
+ * SETAASA-based devices need not always have a matching ID since
+ * it is not mandatory for such devices to implement deviceinfo
+ * CCC commands. Allow them to register through DT or ACPI.
+ */
+ if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))
return 1;
+ if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+ if (of_driver_match_device(dev, drv))
+ return 1;
+ if (acpi_driver_match_device(dev, drv))
+ return 1;
+ }
+
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v5 05/12] i3c: master: Add support for devices without PID
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Devices using SETAASA for address assignment are not required to have
a 48-bit PID according to the I3C specification. Allow such devices to
register and use the static address where PID was required.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master.c | 51 ++++++++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 3deae8cdae52..bd0dc76c7ba1 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1989,8 +1989,17 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
desc->dev->dev.type = &i3c_device_type;
desc->dev->dev.bus = &i3c_bus_type;
desc->dev->dev.release = i3c_device_release;
- dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
- desc->info.pid);
+
+ /*
+ * For devices without PID (e.g., SETAASA devices), use
+ * static address for naming instead.
+ */
+ if (desc->info.pid)
+ dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
+ desc->info.pid);
+ else
+ dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
+ desc->info.static_addr);
if (desc->boardinfo)
device_set_node(&desc->dev->dev,
@@ -2383,8 +2392,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
struct i3c_dev_boardinfo *i3cboardinfo;
list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
- if (i3cdev->info.pid != i3cboardinfo->pid)
- continue;
+ /*
+ * For devices without PID (e.g., SETAASA devices), match by
+ * static address. For devices with PID, match by PID.
+ */
+ if (i3cboardinfo->pid) {
+ if (i3cdev->info.pid != i3cboardinfo->pid)
+ continue;
+ } else {
+ if (!i3cboardinfo->static_addr ||
+ i3cdev->info.static_addr != i3cboardinfo->static_addr)
+ continue;
+ }
i3cdev->boardinfo = i3cboardinfo;
i3cdev->info.static_addr = i3cboardinfo->static_addr;
@@ -2398,8 +2417,12 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
struct i3c_master_controller *master = i3c_dev_get_master(refdev);
struct i3c_dev_desc *i3cdev;
+ if (!refdev->info.pid)
+ return NULL;
+
i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
- if (i3cdev != refdev && i3cdev->info.pid == refdev->info.pid)
+ if (i3cdev != refdev && i3cdev->info.pid &&
+ i3cdev->info.pid == refdev->info.pid)
return i3cdev;
}
@@ -2832,9 +2855,15 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
- if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
- I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
- return -EINVAL;
+ /* For SETAASA devices, validate the static address instead of PID */
+ if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+ if (!boardinfo->static_addr)
+ return -EINVAL;
+ } else {
+ if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
+ I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
+ return -EINVAL;
+ }
boardinfo->init_dyn_addr = init_dyn_addr;
boardinfo->fwnode = fwnode_handle_get(fwnode);
@@ -2857,10 +2886,10 @@ static int i3c_master_add_of_dev(struct i3c_master_controller *master,
return ret;
/*
- * The manufacturer ID can't be 0. If reg[1] == 0 that means we're
- * dealing with an I2C device.
+ * I3C device should have either the manufacturer ID specified or the
+ * address discovery method specified. Else treat it as an I2C device.
*/
- if (!reg[1])
+ if (!reg[1] && !fwnode_property_present(fwnode, "mipi-i3c-static-method"))
ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
else
ret = i3c_master_add_i3c_boardinfo(master, fwnode, reg);
--
2.43.0
^ permalink raw reply related
* [PATCH v5 04/12] i3c: master: Add support for devices using SETAASA
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Add support for devices using SETAASA, such as SPD5118 and SPD5108
attached to DDR5 memory modules that do not support ENTDAA. Follow the
guidelines proposed by the MIPI Discovery and Configuration
Specification [1] for discovering such devices.
SETAASA (Set All Addresses to Static Address) differs from standard I3C
address assignment that uses ENTDAA or SETDASA to assign dynamic
addresses. Devices using SETAASA assign their pre-defined static addresses
as their dynamic addresses during DAA, and it is not mandatory for these
devices to implement standard CCC commands like GETPID, GETDCR, or GETBCR.
For such devices, it is generally recommended to issue SETHID (specified
by JEDEC JESD300) as a prerequisite for SETAASA to stop HID bit flipping.
[1] https://www.mipi.org/mipi-disco-for-i3c-download
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master.c | 83 +++++++++++++++++++++++++++++++++++++-
include/linux/i3c/ccc.h | 1 +
include/linux/i3c/master.h | 15 +++++++
3 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 4bba2bad897a..3deae8cdae52 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -5,6 +5,7 @@
* Author: Boris Brezillon <boris.brezillon@bootlin.com>
*/
+#include <dt-bindings/i3c/i3c.h>
#include <linux/acpi.h>
#include <linux/atomic.h>
#include <linux/bitmap.h>
@@ -1102,6 +1103,51 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
return ret;
}
+/**
+ * i3c_master_setaasa_locked() - start a SETAASA procedure (Set All Addresses to Static Address)
+ * @master: I3C master object
+ *
+ * Send a SETAASA CCC command to set all attached I3C devices' dynamic addresses to
+ * their static address.
+ *
+ * This function must be called with the bus lock held in write mode.
+ *
+ * First, the SETHID CCC command is sent, followed by the SETAASA CCC.
+ *
+ * Return: 0 in case of success, a positive I3C error code if the error is
+ * one of the official Mx error codes, and a negative error code otherwise.
+ */
+static int i3c_master_setaasa_locked(struct i3c_master_controller *master)
+{
+ struct i3c_ccc_cmd_dest dest;
+ struct i3c_ccc_cmd cmd;
+ int ret;
+
+ /*
+ * Send SETHID CCC command. Though it is a standard CCC command specified
+ * in JESD300-5, we are not defining a separate macro to be explicit that
+ * the value falls under the vendor specific range.
+ */
+ i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
+ i3c_ccc_cmd_init(&cmd, false, I3C_CCC_VENDOR(0, true), &dest, 1);
+ ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+ i3c_ccc_cmd_dest_cleanup(&dest);
+ if (ret && cmd.err == I3C_ERROR_M2)
+ ret = 0;
+ if (ret)
+ return ret;
+
+ /* Send SETAASA CCC command */
+ i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
+ i3c_ccc_cmd_init(&cmd, false, I3C_CCC_SETAASA, &dest, 1);
+ ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+ i3c_ccc_cmd_dest_cleanup(&dest);
+ if (ret && cmd.err == I3C_ERROR_M2)
+ ret = 0;
+
+ return ret;
+}
+
/**
* i3c_master_entdaa_locked() - start a DAA (Dynamic Address Assignment)
* procedure
@@ -1878,6 +1924,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
if (ret)
goto err_free_dev;
+ /*
+ * For devices using SETAASA instead of ENTDAA, the address is statically
+ * assigned. Update the dynamic address to the provided static address.
+ * Reattach the I3C device after updating the dynamic address with the same
+ * static address. It is not mandatory for such devices to implement CCC
+ * commands like GETPID, GETDCR etc. Hence, we can return after reattaching.
+ */
+ if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+ i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
+ ret = i3c_master_reattach_i3c_dev_locked(i3cdev, 0);
+ if (ret)
+ goto err_rstdaa;
+
+ return 0;
+ }
+
ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
i3cdev->boardinfo->init_dyn_addr);
if (ret)
@@ -2232,6 +2294,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
if (ret)
goto err_bus_cleanup;
+ if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
+ ret = i3c_master_setaasa_locked(master);
+ if (ret)
+ goto err_bus_cleanup;
+ }
+
/*
* Reserve init_dyn_addr first, and then try to pre-assign dynamic
* address and retrieve device information if needed.
@@ -2724,7 +2792,7 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
struct i3c_dev_boardinfo *boardinfo;
struct device *dev = &master->dev;
enum i3c_addr_slot_status addrstatus;
- u32 init_dyn_addr = 0;
+ u32 init_dyn_addr = 0, static_addr_method = 0;
boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
if (!boardinfo)
@@ -2742,7 +2810,14 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
boardinfo->static_addr = reg[0];
- if (!fwnode_property_read_u32(fwnode, "assigned-address", &init_dyn_addr)) {
+ if (!fwnode_property_read_u32(fwnode, "mipi-i3c-static-method", &static_addr_method))
+ boardinfo->static_addr_method = static_addr_method &
+ (I3C_ADDR_METHOD_SETDASA | I3C_ADDR_METHOD_SETAASA);
+
+ if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+ /* For SETAASA, static address is taken as the dynamic address. */
+ init_dyn_addr = boardinfo->static_addr;
+ } else if (!fwnode_property_read_u32(fwnode, "assigned-address", &init_dyn_addr)) {
if (init_dyn_addr > I3C_MAX_ADDR)
return -EINVAL;
@@ -2752,6 +2827,9 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
return -EINVAL;
}
+ /* Update the address methods required for device discovery */
+ master->addr_method |= boardinfo->static_addr_method;
+
boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
@@ -3386,6 +3464,7 @@ int i3c_master_register(struct i3c_master_controller *master,
master->dev.release = i3c_masterdev_release;
master->ops = ops;
master->secondary = secondary;
+ master->addr_method = I3C_ADDR_METHOD_SETDASA;
INIT_LIST_HEAD(&master->boardinfo.i2c);
INIT_LIST_HEAD(&master->boardinfo.i3c);
diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index ad59a4ae60d1..a145d766ab6f 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -32,6 +32,7 @@
#define I3C_CCC_DEFSLVS I3C_CCC_ID(0x8, true)
#define I3C_CCC_ENTTM I3C_CCC_ID(0xb, true)
#define I3C_CCC_ENTHDR(x) I3C_CCC_ID(0x20 + (x), true)
+#define I3C_CCC_SETAASA I3C_CCC_ID(0x29, true)
/* Unicast-only commands */
#define I3C_CCC_SETDASA I3C_CCC_ID(0x7, false)
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index a16deb04b2e1..2dc139a217bf 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -174,6 +174,14 @@ struct i3c_device_ibi_info {
* assigned a dynamic address by the master. Will be used during
* bus initialization to assign it a specific dynamic address
* before starting DAA (Dynamic Address Assignment)
+ * @static_addr_method: Bitmap describing which methods of Dynamic Address
+ * Assignment from a Static Address are supported by this I3C Target.
+ * A value of 1 in a bit position indicates that the I3C target
+ * supports that method, and a value of 0 indicates that the I3C
+ * target does not support that method.
+ * Bit 0: SETDASA
+ * Bit 1: SETAASA
+ * All other bits are reserved.
* @pid: I3C Provisioned ID exposed by the device. This is a unique identifier
* that may be used to attach boardinfo to i3c_dev_desc when the device
* does not have a static address
@@ -189,6 +197,7 @@ struct i3c_dev_boardinfo {
struct list_head node;
u8 init_dyn_addr;
u8 static_addr;
+ u8 static_addr_method;
u64 pid;
struct fwnode_handle *fwnode;
};
@@ -517,6 +526,11 @@ struct i3c_master_controller_ops {
* @boardinfo.i2c: list of I2C boardinfo objects
* @boardinfo: board-level information attached to devices connected on the bus
* @bus: I3C bus exposed by this master
+ * @addr_method: Bitmap describing which methods of Address Assignment required
+ * to be run for discovering all the devices on the bus.
+ * Bit 0: SETDASA
+ * Bit 1: SETAASA
+ * All other bits are reserved.
* @wq: freezable workqueue which can be used by master
* drivers if they need to postpone operations that need to take place
* in a thread context. Typical examples are Hot Join processing which
@@ -552,6 +566,7 @@ struct i3c_master_controller {
struct list_head i2c;
} boardinfo;
struct i3c_bus bus;
+ u8 addr_method;
struct workqueue_struct *wq;
struct work_struct hj_work;
struct work_struct reg_work;
--
2.43.0
^ permalink raw reply related
* [PATCH v5 03/12] i3c: master: Support ACPI enumeration of child devices
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Although the existing subsystem allows host controllers to register
through the ACPI table, it was not possible to describe I3C or I2C
devices when using ACPI. This is because the driver relied on the reg
property to retrieve the PID, static address, etc., whereas ACPI uses
_ADR or serial resources to describe such devices.
Read _ADR and LVR from ACPI resources and extract the data as per the
ACPI specification for an I3C bus. Also read mipi-i3c-static-address as
per the MIPI DISCO specifications [1] to get the static address to be
used.
Enable describing I3C or I2C devices in the ACPI table. This is required
if the device uses a static address or if it needs device-specific
properties.
[1] https://www.mipi.org/mipi-disco-for-i3c-download
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master.c | 151 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 143 insertions(+), 8 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index b20f56f7b68e..4bba2bad897a 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -5,6 +5,7 @@
* Author: Boris Brezillon <boris.brezillon@bootlin.com>
*/
+#include <linux/acpi.h>
#include <linux/atomic.h>
#include <linux/bitmap.h>
#include <linux/bug.h>
@@ -2622,6 +2623,55 @@ EXPORT_SYMBOL_GPL(i3c_master_do_daa);
#define OF_I3C_REG1_IS_I2C_DEV BIT(31)
+#ifdef CONFIG_ACPI
+static int i3c_acpi_get_i2c_resource(struct acpi_resource *ares, void *data)
+{
+ struct i2c_dev_boardinfo *boardinfo = data;
+ struct acpi_resource_i2c_serialbus *sb;
+
+ if (boardinfo->base.addr || !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ boardinfo->base.addr = sb->slave_address;
+ if (sb->access_mode == ACPI_I2C_10BIT_MODE)
+ boardinfo->base.flags |= I2C_CLIENT_TEN;
+
+ boardinfo->lvr = sb->lvr;
+
+ return 1;
+}
+
+static int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
+ struct fwnode_handle *fwnode)
+{
+ struct acpi_device *adev = to_acpi_device_node(fwnode);
+ LIST_HEAD(resources);
+ int ret;
+
+ boardinfo->base.fwnode = acpi_fwnode_handle(adev);
+ acpi_set_modalias(adev, dev_name(&adev->dev), boardinfo->base.type,
+ sizeof(boardinfo->base.type));
+
+ ret = acpi_dev_get_resources(adev, &resources,
+ i3c_acpi_get_i2c_resource, boardinfo);
+ if (ret < 0)
+ return ret;
+
+ acpi_dev_free_resource_list(&resources);
+
+ if (!boardinfo->base.addr)
+ return -ENODEV;
+
+ return 0;
+}
+#else
+static inline int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
+ struct fwnode_handle *fwnode)
+{
+ return -ENODEV;
+}
+#endif
+
static int
i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
struct fwnode_handle *fwnode, u32 *reg)
@@ -2638,6 +2688,15 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
ret = of_i2c_get_board_info(dev, to_of_node(fwnode), &boardinfo->base);
if (ret)
return ret;
+
+ /* LVR is encoded in reg[2] for Device Tree. */
+ boardinfo->lvr = reg[2];
+ } else if (is_acpi_device_node(fwnode)) {
+ ret = i3c_acpi_add_i2c_boardinfo(boardinfo, fwnode);
+ if (ret) {
+ devm_kfree(dev, boardinfo);
+ return ret;
+ }
} else {
return -EINVAL;
}
@@ -2652,9 +2711,6 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
return -EOPNOTSUPP;
}
- /* LVR is encoded in reg[2]. */
- boardinfo->lvr = reg[2];
-
list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
fwnode_handle_get(fwnode);
@@ -2709,8 +2765,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
return 0;
}
-static int i3c_master_add_dev(struct i3c_master_controller *master,
- struct fwnode_handle *fwnode)
+static int i3c_master_add_of_dev(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode)
{
u32 reg[3];
int ret;
@@ -2734,6 +2790,74 @@ static int i3c_master_add_dev(struct i3c_master_controller *master,
return ret;
}
+#ifdef CONFIG_ACPI
+static int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode)
+{
+ struct acpi_device *adev = to_acpi_device_node(fwnode);
+ acpi_bus_address adr;
+ u32 reg[3] = { 0 };
+ int ret;
+
+ /*
+ * If the ACPI table entry has _ADR method, it's an I3C device.
+ * Otherwise it may be an I2C device described by an I2cSerialBus
+ * resource. If no I2cSerialBus resource is found, ignore the entry.
+ */
+ if (!acpi_has_method(adev->handle, "_ADR")) {
+ ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
+ if (ret == -ENODEV)
+ return 0;
+
+ return ret;
+ }
+
+ adr = acpi_device_adr(adev);
+
+ /* For I3C devices, _ADR will have the 48 bit PID of the device */
+ reg[1] = upper_32_bits(adr);
+ reg[2] = lower_32_bits(adr);
+
+ fwnode_property_read_u32(fwnode, "mipi-i3c-static-address", ®[0]);
+
+ return i3c_master_add_i3c_boardinfo(master, fwnode, reg);
+}
+
+static u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
+{
+ struct acpi_device *adev = to_acpi_device_node(client->dev.fwnode);
+ struct i2c_dev_boardinfo boardinfo = {};
+ LIST_HEAD(resources);
+ int ret;
+ u8 lvr;
+
+ lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
+
+ ret = acpi_dev_get_resources(adev, &resources,
+ i3c_acpi_get_i2c_resource, &boardinfo);
+ if (ret < 0)
+ return lvr;
+
+ if (boardinfo.base.addr)
+ lvr = boardinfo.lvr;
+
+ acpi_dev_free_resource_list(&resources);
+
+ return lvr;
+}
+#else
+static inline int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode)
+{
+ return -ENODEV;
+}
+
+static inline u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
+{
+ return I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
+}
+#endif
+
static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
{
struct device *dev = &master->dev;
@@ -2745,7 +2869,13 @@ static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
return 0;
fwnode_for_each_available_child_node_scoped(fwnode, child) {
- ret = i3c_master_add_dev(master, child);
+ if (is_of_node(child))
+ ret = i3c_master_add_of_dev(master, child);
+ else if (is_acpi_device_node(child))
+ ret = i3c_master_add_acpi_dev(master, child);
+ else
+ continue;
+
if (ret)
return ret;
}
@@ -2813,8 +2943,13 @@ static u8 i3c_master_i2c_get_lvr(struct i2c_client *client)
u8 lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
u32 reg[3];
- if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg", reg, ARRAY_SIZE(reg)))
- lvr = reg[2];
+ if (is_of_node(client->dev.fwnode)) {
+ if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg",
+ reg, ARRAY_SIZE(reg)))
+ lvr = reg[2];
+ } else if (is_acpi_device_node(client->dev.fwnode)) {
+ lvr = i3c_acpi_i2c_get_lvr(client);
+ }
return lvr;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v5 02/12] i3c: master: Use unified device property interface
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Replace all OF-specific functions with unified device property functions
as a prerequisite to support both ACPI and device tree.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
drivers/i3c/master.c | 78 +++++++++++++++++++++-----------------
include/linux/i3c/master.h | 5 ++-
2 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index f1be38a640ca..b20f56f7b68e 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -13,10 +13,12 @@
#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/export.h>
+#include <linux/i2c.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/of.h>
#include <linux/pm_runtime.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
@@ -491,7 +493,7 @@ static void i3c_bus_cleanup(struct i3c_bus *i3cbus)
mutex_unlock(&i3c_core_lock);
}
-static int i3c_bus_init(struct i3c_bus *i3cbus, struct device_node *np)
+static int i3c_bus_init(struct i3c_bus *i3cbus, struct fwnode_handle *fwnode)
{
int ret, start, end, id = -1;
@@ -501,8 +503,8 @@ static int i3c_bus_init(struct i3c_bus *i3cbus, struct device_node *np)
i3c_bus_init_addrslots(i3cbus);
i3cbus->mode = I3C_BUS_MODE_PURE;
- if (np)
- id = of_alias_get_id(np, "i3c");
+ if (fwnode && is_of_node(fwnode))
+ id = of_alias_get_id(to_of_node(fwnode), "i3c");
mutex_lock(&i3c_core_lock);
if (id >= 0) {
@@ -837,7 +839,7 @@ static void i3c_masterdev_release(struct device *dev)
WARN_ON(!list_empty(&bus->devs.i2c) || !list_empty(&bus->devs.i3c));
i3c_bus_cleanup(bus);
- of_node_put(dev->of_node);
+ fwnode_handle_put(dev->fwnode);
}
static const struct device_type i3c_masterdev_type = {
@@ -1044,7 +1046,7 @@ static void i3c_device_release(struct device *dev)
WARN_ON(i3cdev->desc);
- of_node_put(i3cdev->dev.of_node);
+ fwnode_handle_put(dev->fwnode);
kfree(i3cdev);
}
@@ -1928,7 +1930,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
desc->info.pid);
if (desc->boardinfo)
- desc->dev->dev.of_node = desc->boardinfo->of_node;
+ device_set_node(&desc->dev->dev,
+ fwnode_handle_get(desc->boardinfo->fwnode));
ret = device_register(&desc->dev->dev);
if (ret) {
@@ -2620,8 +2623,8 @@ EXPORT_SYMBOL_GPL(i3c_master_do_daa);
#define OF_I3C_REG1_IS_I2C_DEV BIT(31)
static int
-of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
- struct device_node *node, u32 *reg)
+i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode, u32 *reg)
{
struct i2c_dev_boardinfo *boardinfo;
struct device *dev = &master->dev;
@@ -2631,9 +2634,13 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
if (!boardinfo)
return -ENOMEM;
- ret = of_i2c_get_board_info(dev, node, &boardinfo->base);
- if (ret)
- return ret;
+ if (is_of_node(fwnode)) {
+ ret = of_i2c_get_board_info(dev, to_of_node(fwnode), &boardinfo->base);
+ if (ret)
+ return ret;
+ } else {
+ return -EINVAL;
+ }
/*
* The I3C Specification does not clearly say I2C devices with 10-bit
@@ -2649,14 +2656,14 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
boardinfo->lvr = reg[2];
list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
- of_node_get(node);
+ fwnode_handle_get(fwnode);
return 0;
}
static int
-of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
- struct device_node *node, u32 *reg)
+i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode, u32 *reg)
{
struct i3c_dev_boardinfo *boardinfo;
struct device *dev = &master->dev;
@@ -2679,7 +2686,7 @@ of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
boardinfo->static_addr = reg[0];
- if (!of_property_read_u32(node, "assigned-address", &init_dyn_addr)) {
+ if (!fwnode_property_read_u32(fwnode, "assigned-address", &init_dyn_addr)) {
if (init_dyn_addr > I3C_MAX_ADDR)
return -EINVAL;
@@ -2696,14 +2703,14 @@ of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
return -EINVAL;
boardinfo->init_dyn_addr = init_dyn_addr;
- boardinfo->of_node = of_node_get(node);
+ boardinfo->fwnode = fwnode_handle_get(fwnode);
list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
return 0;
}
-static int of_i3c_master_add_dev(struct i3c_master_controller *master,
- struct device_node *node)
+static int i3c_master_add_dev(struct i3c_master_controller *master,
+ struct fwnode_handle *fwnode)
{
u32 reg[3];
int ret;
@@ -2711,7 +2718,7 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
if (!master)
return -EINVAL;
- ret = of_property_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
+ ret = fwnode_property_read_u32_array(fwnode, "reg", reg, ARRAY_SIZE(reg));
if (ret)
return ret;
@@ -2720,25 +2727,25 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
* dealing with an I2C device.
*/
if (!reg[1])
- ret = of_i3c_master_add_i2c_boardinfo(master, node, reg);
+ ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
else
- ret = of_i3c_master_add_i3c_boardinfo(master, node, reg);
+ ret = i3c_master_add_i3c_boardinfo(master, fwnode, reg);
return ret;
}
-static int of_populate_i3c_bus(struct i3c_master_controller *master)
+static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
{
struct device *dev = &master->dev;
- struct device_node *i3cbus_np = dev->of_node;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
int ret;
u32 val;
- if (!i3cbus_np)
+ if (!fwnode)
return 0;
- for_each_available_child_of_node_scoped(i3cbus_np, node) {
- ret = of_i3c_master_add_dev(master, node);
+ fwnode_for_each_available_child_node_scoped(fwnode, child) {
+ ret = i3c_master_add_dev(master, child);
if (ret)
return ret;
}
@@ -2748,10 +2755,10 @@ static int of_populate_i3c_bus(struct i3c_master_controller *master)
* on the bus are not supporting typical rates, or if the bus topology
* prevents it from using max possible rate.
*/
- if (!of_property_read_u32(i3cbus_np, "i2c-scl-hz", &val))
+ if (!device_property_read_u32(dev, "i2c-scl-hz", &val))
master->bus.scl_rate.i2c = val;
- if (!of_property_read_u32(i3cbus_np, "i3c-scl-hz", &val))
+ if (!device_property_read_u32(dev, "i3c-scl-hz", &val))
master->bus.scl_rate.i3c = val;
return 0;
@@ -2806,7 +2813,7 @@ static u8 i3c_master_i2c_get_lvr(struct i2c_client *client)
u8 lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
u32 reg[3];
- if (!of_property_read_u32_array(client->dev.of_node, "reg", reg, ARRAY_SIZE(reg)))
+ if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg", reg, ARRAY_SIZE(reg)))
lvr = reg[2];
return lvr;
@@ -2925,7 +2932,8 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
struct i2c_dev_desc *i2cdev;
struct i2c_dev_boardinfo *i2cboardinfo;
- int ret, id;
+ struct fwnode_handle *fwnode = dev_fwnode(&master->dev);
+ int ret, id = -1;
adap->dev.parent = master->dev.parent;
adap->owner = master->dev.parent->driver->owner;
@@ -2934,7 +2942,9 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
adap->timeout = HZ;
adap->retries = 3;
- id = of_alias_get_id(master->dev.of_node, "i2c");
+ if (fwnode && is_of_node(fwnode))
+ id = of_alias_get_id(to_of_node(fwnode), "i2c");
+
if (id >= 0) {
adap->nr = id;
ret = i2c_add_numbered_adapter(adap);
@@ -3235,7 +3245,7 @@ int i3c_master_register(struct i3c_master_controller *master,
return ret;
master->dev.parent = parent;
- master->dev.of_node = of_node_get(parent->of_node);
+ device_set_node(&master->dev, fwnode_handle_get(dev_fwnode(parent)));
master->dev.bus = &i3c_bus_type;
master->dev.type = &i3c_masterdev_type;
master->dev.release = i3c_masterdev_release;
@@ -3254,13 +3264,13 @@ int i3c_master_register(struct i3c_master_controller *master,
master->dev.coherent_dma_mask = parent->coherent_dma_mask;
master->dev.dma_parms = parent->dma_parms;
- ret = i3c_bus_init(i3cbus, master->dev.of_node);
+ ret = i3c_bus_init(i3cbus, dev_fwnode(&master->dev));
if (ret)
goto err_put_dev;
dev_set_name(&master->dev, "i3c-%d", i3cbus->id);
- ret = of_populate_i3c_bus(master);
+ ret = fwnode_populate_i3c_bus(master);
if (ret)
goto err_put_dev;
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 4d2a68793324..a16deb04b2e1 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -177,7 +177,8 @@ struct i3c_device_ibi_info {
* @pid: I3C Provisioned ID exposed by the device. This is a unique identifier
* that may be used to attach boardinfo to i3c_dev_desc when the device
* does not have a static address
- * @of_node: optional DT node in case the device has been described in the DT
+ * @fwnode: Firmware node (DT or ACPI) in case the device has been
+ * described in firmware
*
* This structure is used to attach board-level information to an I3C device.
* Not all I3C devices connected on the bus will have a boardinfo. It's only
@@ -189,7 +190,7 @@ struct i3c_dev_boardinfo {
u8 init_dyn_addr;
u8 static_addr;
u64 pid;
- struct device_node *of_node;
+ struct fwnode_handle *fwnode;
};
/**
--
2.43.0
^ permalink raw reply related
* [PATCH v5 01/12] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
In-Reply-To: <20260624102153.1770072-1-akhilrajeev@nvidia.com>
Add the 'mipi-i3c-static-method' property mentioned in the MIPI I3C
Discovery and Configuration Specification [1] to specify which discovery
method an I3C device supports during bus initialization. The property is
a bitmap, where a bit value of 1 indicates support for that method, and 0
indicates lack of support.
Bit 0: SETDASA CCC (Direct)
Bit 1: SETAASA CCC (Broadcast)
Bit 2: Other CCC (vendor / standards extension)
All other bits are reserved.
It is specifically needed when an I3C device requires SETAASA for the
address assignment. SETDASA will be supported by default if this property
is absent, which means for now the property just serves as a flag to
enable SETAASA, but keep the property as a bitmap to align with the
specifications.
[1] https://www.mipi.org/mipi-disco-for-i3c-download
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
.../devicetree/bindings/i3c/i3c.yaml | 36 ++++++++++++++++---
include/dt-bindings/i3c/i3c.h | 4 +++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
index e25fa72fd785..5603f2e7807d 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.yaml
+++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
@@ -31,10 +31,12 @@ properties:
described in the device tree, which in turn means we have to describe
I3C devices.
- Another use case for describing an I3C device in the device tree is when
- this I3C device has a static I2C address and we want to assign it a
- specific I3C dynamic address before the DAA takes place (so that other
- devices on the bus can't take this dynamic address).
+ Other use-cases for describing an I3C device in the device tree are:
+ - When the I3C device has a static I2C address and we want to assign
+ it a specific I3C dynamic address before the DAA takes place (so
+ that other devices on the bus can't take this dynamic address).
+ - When the I3C device requires SETAASA for its discovery and uses a
+ pre-defined static address.
"#size-cells":
const: 0
@@ -145,7 +147,31 @@ patternProperties:
Dynamic address to be assigned to this device. In case static address is
present (first cell of the reg property != 0), this address is assigned
through SETDASA. If static address is not present, this address is assigned
- through SETNEWDA after assigning a temporary address via ENTDAA.
+ through SETNEWDA after assigning a temporary address via ENTDAA. If
+ SETAASA is used, this property is not used, and the static address itself
+ becomes the dynamic address.
+
+ mipi-i3c-static-method:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x1
+ maximum: 0x7
+ default: 1
+ description: |
+ Bitmap describing which methods of Dynamic Address Assignment from a
+ static address are supported by this I3C Target. For each defined bit
+ position, a set bit indicates support for that method and a cleared
+ bit indicates lack of support.
+
+ Bit 0: SETDASA CCC (Direct)
+ Bit 1: SETAASA CCC (Broadcast)
+ Bit 2: Other CCC (vendor / standards extension)
+ All other bits are reserved.
+
+ This property follows the MIPI I3C specification. The primary use
+ of this property is to indicate support for SETAASA, i.e Bit 1, but
+ will allow other values mentioned in the specification so that it
+ mirrors the specification. SETDASA will remain as the default method
+ even if this property is not present.
required:
- reg
diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
index 373439218bba..78b8c634aad8 100644
--- a/include/dt-bindings/i3c/i3c.h
+++ b/include/dt-bindings/i3c/i3c.h
@@ -13,4 +13,8 @@
#define I2C_NO_FILTER_HIGH_FREQUENCY (1 << 5)
#define I2C_NO_FILTER_LOW_FREQUENCY (2 << 5)
+#define I3C_ADDR_METHOD_SETDASA (1 << 0)
+#define I3C_ADDR_METHOD_SETAASA (1 << 1)
+#define I3C_ADDR_METHOD_VENDOR (1 << 2)
+
#endif
--
2.43.0
^ permalink raw reply related
* [PATCH v5 00/12] Support ACPI and SETAASA device discovery
From: Akhil R @ 2026-06-24 10:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Frank Li, Miquel Raynal, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Guenter Roeck, Philipp Zabel, Jon Hunter,
Thierry Reding, linux-i3c, devicetree, linux-hwmon, linux-tegra,
linux-kernel, Akhil R
This patch series adds SETAASA device discovery to the I3C subsystem,
enabling support for SPD5118 temperature sensors found on DDR5 memory
modules. The changes also add ACPI support for all existing DAA
methods like SETDASA, SETNEWDA as well as I2C devices on I3C bus.
SPD5118 and similar devices on DDR5 memory modules differ from typical
I3C devices in their initialization. They use SETAASA broadcast CCC
instead of ENTDAA for address assignment, and per JEDEC specification,
are not required to have a Provisioned ID or implement standard device
information CCC commands (GETPID, GETDCR, GETBCR).
The series enables describing all I3C and I2C devices on both Device
Tree and the ACPI table, using unified device property APIs throughout
the I3C core and the Synopsys DesignWare I3C master driver.
Please note that the series modifies drivers across multiple subsystems,
like Device Tree bindings, ACPI, I3C and HWMON.
v4->v5:
* Free ACPI I2C boardinfo when an ACPI child without an I2C resource is
ignored.
* Issue RSTDAA if SETAASA device attach fails after the static address is
used as the dynamic address.
* Emit OF and ACPI modaliases for firmware-matched I3C devices.
* Make the DesignWare core clock optional and use "clock-frequency" as
the fallback when core_clk is absent.
* Keep DesignWare pclk, reset, and match-data handling on their existing
optional paths.
v3->v4:
* Clarify mipi-i3c-static-method bit semantics and assigned-address
* Add I3C_ADDR_METHOD_VENDOR
* Fix fwnode reference handling while converting child property parsing
to use unified firmware-node APIs.
* Align ACPI child enumeration with the I2C core for multiple
I2cSerialBus resources, ignore ACPI child entries without an I2C
resource, and populate I2C modalias information from ACPI.
* Update SETAASA handling to use the static address as the dynamic
address, skip device-info retrieval for SETAASA devices, and tolerate
M2 for SETHID/SETAASA similarly to ENTDAA.
* Reorder DesignWare I3C clock/reset to include optional clock in the
ACPI skip clock/reset quirk.
* Add prints for missing ACPI clock-frequency and SPD5118 I3C
device type read failures.
* Fix grammar in comments and commit messages.
v2->v3:
* Fix maximum value and indent bit list for mipi-i3c-static-method.
* Move I3C_ADDR_METHOD_* macros to dt-bindings header.
* Drop ACPICA commit IDs, keep only the Link: tags.
* Revert the change which proceeds to register other devices if SETAASA
is not supported so that it aligns with the rest of the driver and to
avoid the issues pointed by Sashiko.
* Rework multiple commit messages.
v1->v2:
* Added patch to remove 16-bit addressing support for SPD5118
* Guard ACPI calls with #ifdef CONFIG_ACPI
* Remove CONFIG_OF guard for of_alias_get_highest_id()
* Mask mipi-i3c-static-method in the driver to select only valid values.
* Proceed to register other devices if SETAASA is not supported.
* Update commit message and links in the description of multiple commits.
Akhil R (12):
dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
i3c: master: Use unified device property interface
i3c: master: Support ACPI enumeration of child devices
i3c: master: Add support for devices using SETAASA
i3c: master: Add support for devices without PID
i3c: master: match I3C device through DT and ACPI
i3c: dw-i3c-master: Add SETAASA as supported CCC
i3c: dw-i3c-master: Add ACPI core clock frequency quirk
i3c: dw-i3c-master: Add ACPI ID for Tegra410
hwmon: spd5118: Remove 16-bit addressing
hwmon: spd5118: Add I3C support
arm64: defconfig: Enable I3C and SPD5118 hwmon
.../devicetree/bindings/i3c/i3c.yaml | 36 +-
arch/arm64/configs/defconfig | 3 +
drivers/hwmon/Kconfig | 9 +-
drivers/hwmon/spd5118.c | 119 +++---
drivers/i3c/master.c | 382 +++++++++++++++---
drivers/i3c/master/dw-i3c-master.c | 36 +-
include/dt-bindings/i3c/i3c.h | 4 +
include/linux/i3c/ccc.h | 1 +
include/linux/i3c/master.h | 20 +-
9 files changed, 476 insertions(+), 134 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v6 02/10] arm64: tegra: Remove fallback compatible for GPCDMA
From: Thierry Reding @ 2026-06-24 10:22 UTC (permalink / raw)
To: Rob Herring
Cc: Akhil R, Thierry Reding, Vinod Koul, Frank Li,
Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
Laxman Dewangan, Philipp Zabel, dmaengine, devicetree,
linux-tegra, linux-kernel
In-Reply-To: <CAL_Jsq+bbYZnE=Asv=2VnvTpSsLfKtdpcLvfPzn85hyiyp85cA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 828 bytes --]
On Tue, Jun 23, 2026 at 09:02:39AM -0500, Rob Herring wrote:
> On Tue, Mar 31, 2026 at 5:24 AM Akhil R <akhilrajeev@nvidia.com> wrote:
> >
> > Remove the fallback compatible string "nvidia,tegra186-gpcdma" for GPCDMA
> > in Tegra264. Tegra186 compatible cannot work on Tegra264 because of the
> > register offset changes and absence of the reset property.
> >
> > Fixes: 65ef237e4810 ("arm64: tegra: Add Tegra264 support")
> > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > ---
> > arch/arm64/boot/dts/nvidia/tegra264.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thierry, Are you going to apply this? The binding change has been
> picked up and now there's a warning.
Yes, I have this in my tree of fixes for 7.1 and plan to send it out
towards the end of this week.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] mmc: cqhci: Remove unused intmask parameter from cqhci_irq()
From: Chanwoo Lee @ 2026-06-24 2:19 UTC (permalink / raw)
To: Adrian Hunter, Asutosh Das, Ritesh Harjani, Ulf Hansson,
Chaotian Jing, Matthias Brugger, AngeloGioacchino Del Regno,
Kamal Dasu, Al Cooper, Broadcom internal kernel review list,
Florian Fainelli, Haibo Chen, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Michal Simek,
Thierry Reding, Jonathan Hunter,
open list:EMMC CMDQ HOST CONTROLLER INTERFACE (CQHCI) DRIVER,
open list, moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support,
open list:ARM/QUALCOMM MAILING LIST,
open list:TEGRA ARCHITECTURE SUPPORT
Cc: Chanwoo Lee
In-Reply-To: <CGME20260624021955epcas1p2ba2fa4eb8bd0aafaf7b46bde2cf524be@epcas1p2.samsung.com>
The intmask parameter of cqhci_irq() is never used within the function
body. The function reads the CQHCI interrupt status directly via
cqhci_readl() and processes interrupts independently of the SDHCI
intmask value passed by callers.
Signed-off-by: Chanwoo Lee <cw9316.lee@samsung.com>
---
drivers/mmc/host/cqhci-core.c | 3 +--
drivers/mmc/host/cqhci.h | 3 +--
drivers/mmc/host/mtk-sd.c | 2 +-
drivers/mmc/host/sdhci-brcmstb.c | 2 +-
drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
drivers/mmc/host/sdhci-msm.c | 2 +-
drivers/mmc/host/sdhci-of-arasan.c | 2 +-
drivers/mmc/host/sdhci-of-dwcmshc.c | 2 +-
drivers/mmc/host/sdhci-pci-core.c | 2 +-
drivers/mmc/host/sdhci-pci-gli.c | 2 +-
drivers/mmc/host/sdhci-tegra.c | 2 +-
drivers/mmc/host/sdhci_am654.c | 2 +-
12 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index 178277d90c31..98ceb0b9a6d1 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -819,8 +819,7 @@ static void cqhci_finish_mrq(struct mmc_host *mmc, unsigned int tag)
mmc_cqe_request_done(mmc, mrq);
}
-irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
- int data_error)
+irqreturn_t cqhci_irq(struct mmc_host *mmc, int cmd_error, int data_error)
{
u32 status;
unsigned long tag = 0, comp_status;
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 3668856531c1..8fbbc48c3f85 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -315,8 +315,7 @@ static inline u32 cqhci_readl(struct cqhci_host *host, int reg)
struct platform_device;
-irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
- int data_error);
+irqreturn_t cqhci_irq(struct mmc_host *mmc, int cmd_error, int data_error);
int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, bool dma64);
struct cqhci_host *cqhci_pltfm_init(struct platform_device *pdev);
int cqhci_deactivate(struct mmc_host *mmc);
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index b2680cc054bd..01ea3adbdf3b 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -1805,7 +1805,7 @@ static irqreturn_t msdc_cmdq_irq(struct msdc_host *host, u32 intsts)
cmd_err, dat_err, intsts);
}
- return cqhci_irq(mmc, 0, cmd_err, dat_err);
+ return cqhci_irq(mmc, cmd_err, dat_err);
}
static irqreturn_t msdc_irq(int irq, void *dev_id)
diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 57e45951644e..1de2f05fd958 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -430,7 +430,7 @@ static u32 sdhci_brcmstb_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 18ecddd6df6f..d0fa83f67a80 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1503,7 +1503,7 @@ static u32 esdhc_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 0882ce74e0c9..ceed47ccfda8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2165,7 +2165,7 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 785d3acb18c5..4ca73e7d799e 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -555,7 +555,7 @@ static u32 sdhci_arasan_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index eef53455b8ee..4c5fa6a6931d 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -624,7 +624,7 @@ static u32 dwcmshc_cqe_irq_handler(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index c347fac24515..b121d896a804 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -215,7 +215,7 @@ static u32 sdhci_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index 6e4084407662..b55618566d65 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -1760,7 +1760,7 @@ static u32 sdhci_gl9763e_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 820ce4dae58b..221e48b59f48 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1280,7 +1280,7 @@ static u32 sdhci_tegra_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index d235b0aecfdb..2a27db2f558b 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -462,7 +462,7 @@ static u32 sdhci_am654_cqhci_irq(struct sdhci_host *host, u32 intmask)
if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
return intmask;
- cqhci_irq(host->mmc, intmask, cmd_error, data_error);
+ cqhci_irq(host->mmc, cmd_error, data_error);
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v7 04/11] PCI: Export pci_dev_save_and_disable() and pci_dev_restore()
From: Dan Williams (nvidia) @ 2026-06-24 2:17 UTC (permalink / raw)
To: Srirangan Madhavan, Alison Schofield, Bjorn Helgaas, Dan Williams,
Dave Jiang, Davidlohr Bueso, Ira Weiny, Jonathan Cameron,
Vishal Verma, linux-cxl, linux-pci, linux-kernel
Cc: vsethi, alwilliamson, Dan Williams, Sai Yashwanth Reddy Kancherla,
Vishal Aslot, Manish Honap, Jiandi An, Richard Cheng, linux-tegra,
Srirangan Madhavan
In-Reply-To: <20260623032453.3404772-5-smadhavan@nvidia.com>
Srirangan Madhavan wrote:
> Export the standard PCI reset save/disable and restore helpers so CXL reset
> can split that lifecycle around CXL-specific sequencing while preserving
> driver reset_prepare/reset_done callbacks and PCI config-state handling.
Part of the motivation for creating drivers/cxl/core/reset.c as a
builtin was to avoid needing to export PCI core functions. So no need
for these exports given no modular consumers.
^ permalink raw reply
* Re: [PATCH v7 03/11] cxl: Cache endpoint decoder settings during PCI enumeration
From: Dan Williams (nvidia) @ 2026-06-24 2:15 UTC (permalink / raw)
To: Srirangan Madhavan, Alison Schofield, Bjorn Helgaas, Dan Williams,
Dave Jiang, Davidlohr Bueso, Ira Weiny, Jonathan Cameron,
Vishal Verma, linux-cxl, linux-pci, linux-kernel
Cc: vsethi, alwilliamson, Dan Williams, Sai Yashwanth Reddy Kancherla,
Vishal Aslot, Manish Honap, Jiandi An, Richard Cheng, linux-tegra,
Srirangan Madhavan
In-Reply-To: <20260623032453.3404772-4-smadhavan@nvidia.com>
Srirangan Madhavan wrote:
> Populate pci_dev->hdm from PCI capability initialization when a CXL.mem
> function already has memory decoding enabled. This gives driver-free reset
> paths an early HDM snapshot without enabling BAR decoding during
> enumeration.
>
> CXL core later reuses and refreshes the same cache. Move the register
> helpers into the built-in CONFIG_CXL_HDM set so the early cache path is
> available without cxl_core.
>
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
> drivers/cxl/core/Makefile | 3 +-
> drivers/cxl/core/hdm.c | 59 +++++++----
> drivers/cxl/core/reset.c | 202 ++++++++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 2 +
> include/cxl/cxl.h | 9 ++
> 5 files changed, 252 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index dc075cee0450..69cf2ea7ee74 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_CXL_BUS) += cxl_core.o
> -obj-$(CONFIG_CXL_HDM) += reset.o
> +obj-$(CONFIG_CXL_HDM) += regs.o reset.o
> obj-$(CONFIG_CXL_SUSPEND) += suspend.o
>
> ccflags-y += -I$(srctree)/drivers/cxl
> @@ -8,7 +8,6 @@ CFLAGS_trace.o = -DTRACE_INCLUDE_PATH=. -I$(src)
>
> cxl_core-y := port.o
> cxl_core-y += pmem.o
> -cxl_core-y += regs.o
> cxl_core-y += memdev.o
> cxl_core-y += mbox.o
> cxl_core-y += pci.o
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 83cda63f76a5..0230ebfada42 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -91,11 +91,9 @@ static void clear_hdm_info(void *data)
> WRITE_ONCE(pdev->hdm, NULL);
> }
>
> -static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> +static struct pci_dev *cxl_hdm_to_pci_dev(struct cxl_hdm *cxlhdm)
> {
> struct cxl_port *port = cxlhdm->port;
> - struct cxl_hdm_info *info;
> - struct pci_dev *pdev;
> struct device *uport;
>
> if (is_cxl_endpoint(port)) {
> @@ -107,9 +105,27 @@ static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> }
>
> if (!dev_is_pci(uport))
> + return NULL;
> +
> + return to_pci_dev(uport);
> +}
> +
> +static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> +{
> + struct cxl_hdm_info *info;
> + struct pci_dev *pdev;
> +
> + pdev = cxl_hdm_to_pci_dev(cxlhdm);
> + if (!pdev)
> + return 0;
> +
> + info = READ_ONCE(pdev->hdm);
> + if (info) {
> + if (info->decoder_count != cxlhdm->decoder_count)
> + return -ENXIO;
> return 0;
> + }
>
> - pdev = to_pci_dev(uport);
> info = devm_kzalloc(&pdev->dev,
> struct_size(info, settings, cxlhdm->decoder_count),
> GFP_KERNEL);
> @@ -125,23 +141,13 @@ static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> static void cxl_hdm_info_set_decoder(struct cxl_hdm *cxlhdm,
> struct cxl_decoder *cxld)
> {
> - struct cxl_port *port = cxlhdm->port;
> struct cxl_hdm_info *info;
> struct pci_dev *pdev;
> - struct device *uport;
> -
> - if (is_cxl_endpoint(port)) {
> - struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> -
> - uport = cxlmd->dev.parent;
> - } else {
> - uport = port->uport_dev;
> - }
>
> - if (!dev_is_pci(uport))
> + pdev = cxl_hdm_to_pci_dev(cxlhdm);
> + if (!pdev)
> return;
>
> - pdev = to_pci_dev(uport);
> info = READ_ONCE(pdev->hdm);
> if (!info || cxld->id >= info->decoder_count)
> return;
> @@ -202,6 +208,7 @@ static struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
> struct cxl_register_map *reg_map = &port->reg_map;
> struct device *dev = &port->dev;
> struct cxl_hdm *cxlhdm;
> + struct pci_dev *pdev;
> int rc;
>
> cxlhdm = devm_kzalloc(dev, sizeof(*cxlhdm), GFP_KERNEL);
> @@ -227,11 +234,21 @@ static struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
> return ERR_PTR(-ENODEV);
> }
>
> - rc = cxl_map_component_regs(reg_map, &cxlhdm->regs,
> - BIT(CXL_CM_CAP_CAP_ID_HDM));
> - if (rc) {
> - dev_err(dev, "Failed to map HDM capability.\n");
> - return ERR_PTR(rc);
> + pdev = cxl_hdm_to_pci_dev(cxlhdm);
> + if (pdev) {
> + struct cxl_hdm_info *info = READ_ONCE(pdev->hdm);
> +
> + if (info && info->regs.hdm_decoder)
> + cxlhdm->regs = info->regs;
The driver probably should not reuse the core mapping. The driver's only
role is to update the cached settings if it changes them.
> + }
> +
> + if (!cxlhdm->regs.hdm_decoder) {
> + rc = cxl_map_component_regs(reg_map, &cxlhdm->regs,
> + BIT(CXL_CM_CAP_CAP_ID_HDM));
> + if (rc) {
> + dev_err(dev, "Failed to map HDM capability.\n");
> + return ERR_PTR(rc);
> + }
> }
>
> parse_hdm_decoder_caps(cxlhdm);
> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> index 14f024098e82..fc52d3abdb5b 100644
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c
> @@ -2,9 +2,16 @@
> /* Copyright(c) 2026 NVIDIA Corporation. All rights reserved. */
> #include <linux/delay.h>
> #include <linux/bug.h>
> +#include <linux/bitfield.h>
> #include <linux/errno.h>
> #include <linux/export.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> #include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +
> +#include <cxlpci.h>
>
> #include "cxl.h"
> #include "core.h"
> @@ -116,3 +123,198 @@ int cxl_commit(struct cxl_decoder_settings *settings, void __iomem *hdm)
> return 0;
> }
> EXPORT_SYMBOL_FOR_MODULES(cxl_commit, "cxl_core");
> +
> +#define CXL_HDM_DECODER_MAX_COUNT 32
> +
> +static void cxl_pci_hdm_clear(void *data)
> +{
> + struct pci_dev *pdev = data;
> +
> + WRITE_ONCE(pdev->hdm, NULL);
> +}
> +
> +static void cxl_pci_hdm_unmap(struct pci_dev *pdev,
> + struct cxl_component_regs *regs,
> + struct cxl_register_map *map)
> +{
> + struct cxl_reg_map *hdm_map = &map->component_map.hdm_decoder;
> +
> + if (!regs->hdm_decoder)
> + return;
> +
> + devm_iounmap(&pdev->dev, regs->hdm_decoder);
> + devm_release_mem_region(&pdev->dev, map->resource + hdm_map->offset,
> + hdm_map->size);
> +}
> +
> +static int cxl_pci_hdm_read_decoder(struct pci_dev *pdev,
> + struct cxl_decoder_settings *settings,
> + void __iomem *hdm, int id)
> +{
> + u64 target_or_skip, base, size;
> + u32 ctrl, lo, hi;
> + int rc;
> +
> + *settings = (struct cxl_decoder_settings) {
> + .id = id,
> + };
> +
> + ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id));
> + if (!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED))
> + return 0;
> +
> + lo = readl(hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(id));
> + hi = readl(hdm + CXL_HDM_DECODER0_BASE_HIGH_OFFSET(id));
> + base = ((u64)hi << 32) | lo;
> +
> + lo = readl(hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(id));
> + hi = readl(hdm + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(id));
> + size = ((u64)hi << 32) | lo;
> +
> + if (!size || base == U64_MAX || size == U64_MAX ||
> + base > U64_MAX - (size - 1)) {
> + pci_err(pdev, "CXL HDM decoder %d has invalid range\n", id);
> + return -ENXIO;
> + }
Zero size is ok now (see series from Richard), and the U64_MAX checks in
the main driver are more about catching common MMIO read failure cases.
In any event I think the core of this function and the validation should
be shared with init_hdm_decoder() so these kinds of disconnects are
prevented.
> +
> + lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(id));
> + hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(id));
> + target_or_skip = ((u64)hi << 32) | lo;
> +
> + settings->hpa_range = (struct range) {
> + .start = base,
> + .end = base + size - 1,
> + };
> + settings->targets = target_or_skip;
> + settings->target_type = FIELD_GET(CXL_HDM_DECODER0_CTRL_HOSTONLY, ctrl) ?
> + CXL_DECODER_HOSTONLYMEM : CXL_DECODER_DEVMEM;
> + settings->flags = CXL_DECODER_F_ENABLE;
> + if (ctrl & CXL_HDM_DECODER0_CTRL_LOCK)
> + settings->flags |= CXL_DECODER_F_LOCK;
> +
> + rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl),
> + &settings->interleave_ways);
> + if (rc)
> + return rc;
> +
> + return eig_to_granularity(FIELD_GET(CXL_HDM_DECODER0_CTRL_IG_MASK,
> + ctrl),
> + &settings->interleave_granularity);
> +}
> +
> +static int cxl_pci_hdm_capable(struct pci_dev *pdev)
> +{
> + u16 cap;
> + int dvsec;
> + int rc;
> +
> + dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> + PCI_DVSEC_CXL_DEVICE);
> + if (!dvsec)
> + return -ENOTTY;
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CAP, &cap);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + if (!(cap & PCI_DVSEC_CXL_MEM_CAPABLE))
> + return -ENOTTY;
> +
> + return 0;
> +}
> +
> +int pci_cxl_hdm_init(struct pci_dev *pdev)
Capability init does not fail, so this can be void.
If you want you can make an inner function to report the init failure
via an info message, but otherwise PCI core is not looking for failures.
> +{
> + struct cxl_decoder_settings *settings;
> + struct cxl_component_regs regs = { 0 };
> + struct cxl_register_map map = { 0 };
> + struct cxl_hdm_info *info;
> + bool allocated_info = false;
> + int decoder_count;
> + u16 command;
> + int rc;
> +
> + info = READ_ONCE(pdev->hdm);
> + if (info && info->regs.hdm_decoder)
> + return 0;
> +
> + rc = cxl_pci_hdm_capable(pdev);
> + if (rc)
> + return rc;
> +
> + rc = pci_read_config_word(pdev, PCI_COMMAND, &command);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> +
> + if (!(command & PCI_COMMAND_MEMORY))
> + return -ENOTTY;
This needs some commentary about expectations and likely some formal
recommendation to, and agreement with firmware implementers.
The problem is that CXL memory may be, and usually often is, handed off
actively mapped to the OS. When that happens this needs to be able to
assume that PCI_COMMAND_MEMORY is also enabled for access to the HDM
decoders. It would be broken for firmware to say "here is a device
actively mapping CXL.mem and by the way you can not find out what those
settings are without enabling the device MMIO memory space."
So the expectation is that if a device arrives without
PCI_COMMAND_MEMORY then the OS is free to disable CXL reset when a
driver is not loaded. Conversely, if a device arrives actively providing
system resources, it needs to also arrange for active management.
That said, is it safe to assume that in your testing the memory space is
indeed arriving enabled?
> +
> + if (!info) {
> + info = devm_kzalloc(&pdev->dev,
Devres is not suitable for use outside of a driver.
> + struct_size(info, settings,
> + CXL_HDM_DECODER_MAX_COUNT),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> + allocated_info = true;
> + }
> +
> + rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
> + if (rc)
> + return rc;
> +
> + rc = cxl_setup_regs(&map);
> + if (rc)
> + return rc;
> +
> + if (!map.component_map.hdm_decoder.valid) {
> + rc = -ENODEV;
> + return rc;
> + }
> +
> + rc = cxl_map_component_regs(&map, ®s, BIT(CXL_CM_CAP_CAP_ID_HDM));
This uses devres internally so needs a flavor that does typical
allocation. One way to differentiate would be to set "map.host = NULL"
for this case.
> + if (rc)
> + return rc;
> +
> + decoder_count = cxl_hdm_decoder_count(readl(regs.hdm_decoder +
> + CXL_HDM_DECODER_CAP_OFFSET));
> + if (decoder_count < 0) {
> + rc = decoder_count;
> + goto out_unmap;
> + }
> +
> + if (decoder_count > CXL_HDM_DECODER_MAX_COUNT) {
> + rc = -ENXIO;
> + goto out_unmap;
> + }
> +
> + if (info->decoder_count && info->decoder_count != decoder_count) {
> + rc = -ENXIO;
> + goto out_unmap;
> + }
> +
> + info->decoder_count = decoder_count;
> + info->regs = regs;
> +
> + settings = info->settings;
> + for (int i = 0; i < info->decoder_count; i++) {
> + rc = cxl_pci_hdm_read_decoder(pdev, &settings[i],
> + regs.hdm_decoder, i);
> + if (rc)
> + goto out_unmap;
> + }
> +
> + WRITE_ONCE(pdev->hdm, info);
The HDM info is accessed either by the reset path or the driver and
should be protected by cxl_rwsem.dpa. So WRITE_ONCE() becomes
unnecessary if info update happens under that synchronization.
^ permalink raw reply
page: next (older)
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox