| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<DU0PR04MB9496A5E2AFDFB6F24B3B8B5290D7A@DU0PR04MB9496.eurprd04.prod.outlook.com>
Date: Wed, 19 Nov 2025 08:07:05 +0000
From: Bough Chen <haibo.chen@....com>
To: Frank Li <frank.li@....com>
CC: Han Xu <han.xu@....com>, Mark Brown <broonie@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
<s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, "linux-spi@...r.kernel.org"
<linux-spi@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v3 2/2] spi: add driver for NXP XSPI controller
> -----Original Message-----
> From: Frank Li <frank.li@....com>
> Sent: 2025年11月19日 0:33
> To: Bough Chen <haibo.chen@....com>
> Cc: Han Xu <han.xu@....com>; Mark Brown <broonie@...nel.org>; Rob
> Herring <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor
> Dooley <conor+dt@...nel.org>; Shawn Guo <shawnguo@...nel.org>; Sascha
> Hauer <s.hauer@...gutronix.de>; Pengutronix Kernel Team
> <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>;
> linux-spi@...r.kernel.org; imx@...ts.linux.dev; devicetree@...r.kernel.org;
> linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org
> Subject: Re: [PATCH v3 2/2] spi: add driver for NXP XSPI controller
>
> On Tue, Nov 18, 2025 at 11:34:17AM +0800, Haibo Chen wrote:
> > Add driver support for NXP XSPI controller.
> >
> > XSPI is a flexible SPI host controller which supports up to
> > 2 external devices (2 CS). It support Single/Dual/Quad/Octal mode data
> > transfer.
> >
> > The difference between XSPI and Flexspi:
> > 1.the register layout is total different.
> > 2.XSPI support multiple independent execution environments
> > (EENVs) for HW virtualization with some limitations. Each EENV has its
> > own interrupt and its own set of programming registers that exists in
> > a specific offset range in the XSPI memory map.
> > The main environment (EENV0) address space contains all of the
> > registers for controlling EENV0 plus all of the general XSPI control
> > and programming registers. The register mnemonics for the user
> > environments (EENV1 to EENV4) have "_SUB_n" appended to the mnemonic
> > for the corresponding main-environment register.
> >
> > Current driver based on EENV0, which means system already give
> > EENV0 right to linux.
> >
> > This driver use SPI memory interface of the SPI framework to issue
> > flash memory operations. Tested this driver with mtd_debug and UBIFS
> > on NXP i.MX943 EVK board which has one MT35XU512ABA spi nor flash. NOw
> > this driver has the following key features:
> > - Support up to OCT DDR mode
> > - Support AHB read
> > - Support IP read and IP write
> > - Support two CS
> >
> > Signed-off-by: Haibo Chen <haibo.chen@....com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/spi/Kconfig | 10 +
> > drivers/spi/Makefile | 1 +
> > drivers/spi/spi-nxp-xspi.c | 1367
> > ++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 1379 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> >
> 2f17f925ee23dd90acc1b4bf25f158070cd2b65e..527b4f284c3207fb9760ece5cc
> 1d
> > 350e7ad8fe50 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -18853,6 +18853,7 @@ L: linux-spi@...r.kernel.org
> > L: imx@...ts.linux.dev
> > S: Maintained
> > F: Documentation/devicetree/bindings/spi/nxp,imx94-xspi.yaml
> > +F: drivers/spi/spi-nxp-xspi.c
> >
...
> > +
> > +static void nxp_xspi_cleanup(void *data) {
> > + struct nxp_xspi *xspi = data;
> > +
> > + pm_runtime_get_sync(xspi->dev);
> > +
> > + /* Disable interrupt */
> > + writel(0, xspi->iobase + XSPI_RSER);
> > + /* Clear all the internal logic flags */
> > + writel(0xFFFFFFFF, xspi->iobase + XSPI_FR);
> > + /* Disable the hardware */
> > + writel(XSPI_MCR_MDIS, xspi->iobase + XSPI_MCR);
> > +
> > + clk_disable_unprepare(xspi->clk);
> > +
> > + if (xspi->ahb_addr)
> > + iounmap(xspi->ahb_addr);
> > +
> > + pm_runtime_disable(xspi->dev);
> > + pm_runtime_put_noidle(xspi->dev);
>
> if use pm_runtime_put() here, needn't call clk_disable_unprepare() here.
> after use runtime pm to manage clocks, it'd better use it all place.
>
Seems still need to use clk_disable_unprepare here, because pm_runtime_put_noidle() only decrease the runtime PM usage counter.
But adjust the sequence like below seems more readable:
pm_runtime_disable(xspi->dev);
pm_runtime_put_noidle(xspi->dev);
clk_disable_unprepare(xspi->clk);
Regards
Haibo Chen
> > +}
> > +
> > +static int nxp_xspi_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct spi_controller *ctlr;
> > + struct nxp_xspi *xspi;
> > + struct resource *res;
> > + int ret, irq;
> > +
> > + ctlr = devm_spi_alloc_host(dev, sizeof(*xspi));
> > + if (!ctlr)
> > + return -ENOMEM;
> > +
> > + ctlr->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL |
> > + SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL;
> > +
> > + xspi = spi_controller_get_devdata(ctlr);
> > + xspi->dev = dev;
> > + xspi->devtype_data = device_get_match_data(dev);
> > + if (!xspi->devtype_data)
> > + return -ENODEV;
> > +
> > + platform_set_drvdata(pdev, xspi);
> > +
> > + /* Find the resources - configuration register address space */
> > + xspi->iobase = devm_platform_ioremap_resource_byname(pdev, "base");
> > + if (IS_ERR(xspi->iobase))
> > + return PTR_ERR(xspi->iobase);
> > +
> > + /* Find the resources - controller memory mapped space */
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mmap");
> > + if (!res)
> > + return -ENODEV;
> > +
> > + /* Assign memory mapped starting address and mapped size. */
> > + xspi->memmap_phy = res->start;
> > + xspi->memmap_phy_size = resource_size(res);
> > +
> > + /* Find the clocks */
> > + xspi->clk = devm_clk_get(dev, "per");
> > + if (IS_ERR(xspi->clk))
> > + return PTR_ERR(xspi->clk);
> > +
> > + /* Find the irq */
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq, "Failed to get irq source");
> > +
> > + pm_runtime_set_autosuspend_delay(dev, XSPI_RPM_TIMEOUT);
> > + pm_runtime_use_autosuspend(dev);
> > + pm_runtime_enable(dev);
>
> devm_pm_runtime_enable(dev)
>
> Frank
> > +
> > + /* Enable clock */
> > + ret = pm_runtime_get_sync(dev);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to enable clock");
> > +
> > + /* Clear potential interrupt by write xspi errstat */
> > + writel(0xFFFFFFFF, xspi->iobase + XSPI_ERRSTAT);
> > + writel(0xFFFFFFFF, xspi->iobase + XSPI_FR);
> > +
> > + nxp_xspi_default_setup(xspi);
> > +
> > + ret = pm_runtime_put_sync(dev);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to disable clock");
> > +
> > + ret = devm_request_irq(dev, irq,
> > + nxp_xspi_irq_handler, 0, pdev->name, xspi);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to request irq");
> > +
> > + ret = devm_mutex_init(dev, &xspi->lock);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(dev, nxp_xspi_cleanup, xspi);
> > + if (ret)
> > + return ret;
> > +
> > + ctlr->bus_num = -1;
> > + ctlr->num_chipselect = NXP_XSPI_MAX_CHIPSELECT;
> > + ctlr->mem_ops = &nxp_xspi_mem_ops;
> > + ctlr->mem_caps = &nxp_xspi_mem_caps;
> > + ctlr->dev.of_node = dev->of_node;
> > +
> > + return devm_spi_register_controller(dev, ctlr); }
> > +
> > +static int nxp_xspi_runtime_suspend(struct device *dev) {
> > + struct nxp_xspi *xspi = dev_get_drvdata(dev);
> > + u32 reg;
> > +
> > + reg = readl(xspi->iobase + XSPI_MCR);
> > + reg |= XSPI_MCR_MDIS;
> > + writel(reg, xspi->iobase + XSPI_MCR);
> > +
> > + clk_disable_unprepare(xspi->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int nxp_xspi_runtime_resume(struct device *dev) {
> > + struct nxp_xspi *xspi = dev_get_drvdata(dev);
> > + u32 reg;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(xspi->clk);
> > + if (ret)
> > + return ret;
> > +
> > + reg = readl(xspi->iobase + XSPI_MCR);
> > + reg &= ~XSPI_MCR_MDIS;
> > + writel(reg, xspi->iobase + XSPI_MCR);
> > +
> > + return 0;
> > +}
> > +
> > +static int nxp_xspi_suspend(struct device *dev) {
> > + int ret;
> > +
> > + ret = pinctrl_pm_select_sleep_state(dev);
> > + if (ret) {
> > + dev_err(dev, "select flexspi sleep pinctrl failed!\n");
> > + return ret;
> > + }
> > +
> > + return pm_runtime_force_suspend(dev); }
> > +
> > +static int nxp_xspi_resume(struct device *dev) {
> > + struct nxp_xspi *xspi = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = pm_runtime_force_resume(dev);
> > + if (ret)
> > + return ret;
> > +
> > + nxp_xspi_default_setup(xspi);
> > +
> > + ret = pinctrl_pm_select_default_state(dev);
> > + if (ret)
> > + dev_err(dev, "select flexspi default pinctrl failed!\n");
> > +
> > + return ret;
> > +}
> > +
> > +
> > +static const struct dev_pm_ops nxp_xspi_pm_ops = {
> > + RUNTIME_PM_OPS(nxp_xspi_runtime_suspend,
> nxp_xspi_runtime_resume, NULL)
> > + SYSTEM_SLEEP_PM_OPS(nxp_xspi_suspend, nxp_xspi_resume) };
> > +
> > +static const struct of_device_id nxp_xspi_dt_ids[] = {
> > + { .compatible = "nxp,imx94-xspi", .data = (void *)&imx94_data, },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, nxp_xspi_dt_ids);
> > +
> > +static struct platform_driver nxp_xspi_driver = {
> > + .driver = {
> > + .name = "nxp-xspi",
> > + .of_match_table = nxp_xspi_dt_ids,
> > + .pm = pm_ptr(&nxp_xspi_pm_ops),
> > + },
> > + .probe = nxp_xspi_probe,
> > +};
> > +module_platform_driver(nxp_xspi_driver);
> > +
> > +MODULE_DESCRIPTION("NXP xSPI Controller Driver");
> MODULE_AUTHOR("NXP
> > +Semiconductor"); MODULE_AUTHOR("Haibo Chen
> <haibo.chen@....com>");
> > +MODULE_LICENSE("GPL");
> >
> > --
> > 2.34.1
> >
Powered by blists - more mailing lists