From: sashiko-bot@kernel.org
To: "Santhosh Kumar K" <s-k6@ti.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [RESEND PATCH 2/2] memory: ti-k3-fsas: Add TI FSS_FSAS driver
Date: Mon, 29 Jun 2026 07:07:23 +0000 [thread overview]
Message-ID: <20260629070724.CFE591F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260629065517.868418-3-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The driver accesses memory-mapped registers without enabling the device's power domain via runtime PM, causing a synchronous external abort.
- [High] Probe ordering race condition between the FSS wrapper and the OSPI child device can lead to DMA data corruption.
- [Low] The TI_K3_FSS_FSAS driver can be built on architectures without HAS_IOMEM via COMPILE_TEST, causing build failures.
--
commit 472a60060ef7ef11966794e0d895b41600560dca
Author: Santhosh Kumar K <s-k6@ti.com>
memory: ti-k3-fsas: Add TI FSS_FSAS driver
Add a platform driver for the TI Flash SubSystem Application Subsystem
(FSS_FSAS_GENREGS) in K3 SoCs. This driver takes care of disabling the
OSPI XIP prefetch which causes DMA transfer data corruption.
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index e5527020ff337..954e47810e8db 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -125,6 +125,16 @@ config TI_EMIF_SRAM
> sequence so this driver provides several relocatable PM functions
> for the SoC PM code to use.
>
> +config TI_K3_FSS_FSAS
> + tristate "TI K3 Flash Subsystem Application Subsystem (FSAS) support"
> + depends on ARCH_K3 || COMPILE_TEST
[Severity: Low]
Does this config need to depend on HAS_IOMEM?
If COMPILE_TEST is enabled on architectures that do not support MMIO and do
not select HAS_IOMEM (e.g., s390 or UM), will compiling this driver fail
because devm_platform_ioremap_resource(), readl(), and writel() are not fully
supported?
> + help
> + Driver for the TI K3 Flash Subsystem Application Subsystem
> + (FSS_FSAS_GENREGS) wrapper found on K3 related SoCs.
[ ... ]
> diff --git a/drivers/memory/ti-k3-fsas.c b/drivers/memory/ti-k3-fsas.c
> new file mode 100644
> index 0000000000000..9ff109b8d96fd
> --- /dev/null
> +++ b/drivers/memory/ti-k3-fsas.c
[ ... ]
> +static int k3_fsas_probe(struct platform_device *pdev)
> +{
> + struct k3_fsas *fsas;
> +
> + fsas = devm_kzalloc(&pdev->dev, sizeof(*fsas), GFP_KERNEL);
> + if (!fsas)
> + return -ENOMEM;
> +
> + fsas->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(fsas->base))
> + return PTR_ERR(fsas->base);
> +
> + platform_set_drvdata(pdev, fsas);
> +
> + k3_fsas_disable_xip_prefetch(fsas);
[Severity: High]
Will this register access cause a synchronous external abort (bus fault) by
accessing memory-mapped registers before the device's power domain is enabled?
On TI K3 platforms, power domains and clocks are managed by the
ti_sci_pm_domain driver, and the hardware domain remains power-gated until
explicitly enabled.
Should this driver call pm_runtime_enable() and pm_runtime_resume_and_get()
before calling k3_fsas_disable_xip_prefetch()?
> +
> + return 0;
> +}
[Severity: High]
Does this driver need to explicitly populate its child nodes using
devm_of_platform_populate() to prevent a probe ordering race with the OSPI
child device?
Since the driver core allows a child device to probe even if its parent device
is not yet bound, could the spi-cadence-quadspi driver probe first due to
module load ordering?
If the child initiates DMA transfers before this wrapper has probed and
disabled the prefetch, wouldn't it trigger the exact data corruption this
patch aims to fix?
> +
> +static int k3_fsas_resume(struct device *dev)
> +{
> + k3_fsas_disable_xip_prefetch(dev_get_drvdata(dev));
[Severity: High]
Will this register access crash the system during resume?
Since this driver acts as a parent to the OSPI child device, it resumes before
the child. If the shared hardware power domain is completely powered off
at this point, accessing FSAS_GENREGS_SYSCONFIG here would cause a bus fault.
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260629065517.868418-1-s-k6@ti.com?part=2
prev parent reply other threads:[~2026-06-29 7:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-29 6:55 [RESEND PATCH 0/2] Fix OSPI DMA corruption via FSS_FSAS driver Santhosh Kumar K
2026-06-29 6:55 ` [RESEND PATCH 1/2] dt-bindings: memory: Add TI FSS_FSAS binding Santhosh Kumar K
2026-06-29 6:58 ` sashiko-bot
2026-06-29 6:55 ` [RESEND PATCH 2/2] memory: ti-k3-fsas: Add TI FSS_FSAS driver Santhosh Kumar K
2026-06-29 7:07 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260629070724.CFE591F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=robh@kernel.org \
--cc=s-k6@ti.com \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox