Currently NXP fspi driver has support of DT only. Adding ACPI
support to the driver so that it can be used by UEFI firmware
booting in ACPI mode. This driver will be probed if any firmware
will expose HID "NXP0009" in DSDT table.
Signed-off-by: kuldip dwivedi <[email protected]>
---
Notes:
1. Add ACPI match table, NXP members are added to confirm HID for FSPI
2. Change the DT specific APIs to device property APIs
so that same API can be used in DT and ACPi mode.
3. Omit clock configuration part - in ACPI world, the firmware
is responsible for clock maintenance.
4. This patch is tested on LX2160A platform
drivers/spi/spi-nxp-fspi.c | 61 +++++++++++++++++++++++++++-----------
1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 1ccda82da206..9e3991ec0dd2 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -3,7 +3,8 @@
/*
* NXP FlexSPI(FSPI) controller driver.
*
- * Copyright 2019 NXP.
+ * Copyright 2019-2020 NXP
+ * Copyright 2020 Puresoftware Ltd.
*
* FlexSPI is a flexsible SPI host controller which supports two SPI
* channels and up to 4 external devices. Each channel supports
@@ -30,6 +31,7 @@
* Frieder Schrempf <[email protected]>
*/
+#include <linux/acpi.h>
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/completion.h>
@@ -563,6 +565,9 @@ static int nxp_fspi_clk_prep_enable(struct nxp_fspi *f)
{
int ret;
+ if (is_acpi_node(f->dev->fwnode))
+ return 0;
+
ret = clk_prepare_enable(f->clk_en);
if (ret)
return ret;
@@ -576,10 +581,15 @@ static int nxp_fspi_clk_prep_enable(struct nxp_fspi *f)
return 0;
}
-static void nxp_fspi_clk_disable_unprep(struct nxp_fspi *f)
+static int nxp_fspi_clk_disable_unprep(struct nxp_fspi *f)
{
+ if (is_acpi_node(f->dev->fwnode))
+ return 0;
+
clk_disable_unprepare(f->clk);
clk_disable_unprepare(f->clk_en);
+
+ return 0;
}
/*
@@ -1001,7 +1011,7 @@ static int nxp_fspi_probe(struct platform_device *pdev)
f = spi_controller_get_devdata(ctlr);
f->dev = dev;
- f->devtype_data = of_device_get_match_data(dev);
+ f->devtype_data = device_get_match_data(dev);
if (!f->devtype_data) {
ret = -ENODEV;
goto err_put_ctrl;
@@ -1011,6 +1021,9 @@ static int nxp_fspi_probe(struct platform_device *pdev)
/* find the resources - configuration register address space */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_base");
+#ifdef CONFIG_ACPI
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+#endif
f->iobase = devm_ioremap_resource(dev, res);
if (IS_ERR(f->iobase)) {
ret = PTR_ERR(f->iobase);
@@ -1019,6 +1032,9 @@ static int nxp_fspi_probe(struct platform_device *pdev)
/* find the resources - controller memory mapped space */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_mmap");
+#ifdef CONFIG_ACPI
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+#endif
if (!res) {
ret = -ENODEV;
goto err_put_ctrl;
@@ -1029,22 +1045,24 @@ static int nxp_fspi_probe(struct platform_device *pdev)
f->memmap_phy_size = resource_size(res);
/* find the clocks */
- f->clk_en = devm_clk_get(dev, "fspi_en");
- if (IS_ERR(f->clk_en)) {
- ret = PTR_ERR(f->clk_en);
- goto err_put_ctrl;
- }
+ if (dev_of_node(&pdev->dev)) {
+ f->clk_en = devm_clk_get(dev, "fspi_en");
+ if (IS_ERR(f->clk_en)) {
+ ret = PTR_ERR(f->clk_en);
+ goto err_put_ctrl;
+ }
- f->clk = devm_clk_get(dev, "fspi");
- if (IS_ERR(f->clk)) {
- ret = PTR_ERR(f->clk);
- goto err_put_ctrl;
- }
+ f->clk = devm_clk_get(dev, "fspi");
+ if (IS_ERR(f->clk)) {
+ ret = PTR_ERR(f->clk);
+ goto err_put_ctrl;
+ }
- ret = nxp_fspi_clk_prep_enable(f);
- if (ret) {
- dev_err(dev, "can not enable the clock\n");
- goto err_put_ctrl;
+ ret = nxp_fspi_clk_prep_enable(f);
+ if (ret) {
+ dev_err(dev, "can not enable the clock\n");
+ goto err_put_ctrl;
+ }
}
/* find the irq */
@@ -1127,6 +1145,14 @@ static const struct of_device_id nxp_fspi_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id nxp_fspi_acpi_ids[] = {
+ { "NXP0009", .driver_data = (kernel_ulong_t)&lx2160a_data, },
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, nxp_fspi_acpi_ids);
+#endif
+
static const struct dev_pm_ops nxp_fspi_pm_ops = {
.suspend = nxp_fspi_suspend,
.resume = nxp_fspi_resume,
@@ -1136,6 +1162,7 @@ static struct platform_driver nxp_fspi_driver = {
.driver = {
.name = "nxp-fspi",
.of_match_table = nxp_fspi_dt_ids,
+ .acpi_match_table = ACPI_PTR(nxp_fspi_acpi_ids),
.pm = &nxp_fspi_pm_ops,
},
.probe = nxp_fspi_probe,
--
2.17.1
On Fri, Sep 11, 2020 at 01:58:06PM +0530, kuldip dwivedi wrote:
> /* find the resources - configuration register address space */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_base");
> +#ifdef CONFIG_ACPI
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +#endif
This is buggy, it is equivalent to just removing the name based lookup
since we'll do the name based lookup then unconditionally overwrite the
results with an index based lookup.
On 9/11/20 1:00 PM, Mark Brown wrote:
> On Fri, Sep 11, 2020 at 01:58:06PM +0530, kuldip dwivedi wrote:
>
>> /* find the resources - configuration register address space */
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_base");
>> +#ifdef CONFIG_ACPI
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +#endif
>
> This is buggy, it is equivalent to just removing the name based lookup
> since we'll do the name based lookup then unconditionally overwrite the
> results with an index based lookup.
>
Also, note that CONFIG_ACPI kernels may still boot in DT mode.
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Friday, September 11, 2020 4:46 PM
> To: Mark Brown <[email protected]>; kuldip dwivedi
> <[email protected]>
> Cc: Ashish Kumar <[email protected]>; Yogesh Gaur
> <[email protected]>; [email protected]; linux-
> [email protected]; Varun Sethi <[email protected]>; Arokia Samy
> <[email protected]>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> [email protected]>; Paul Yang <[email protected]>
> Subject: Re: [PATCH v2] spi: spi-nxp-fspi: Add ACPI support
>
> On 9/11/20 1:00 PM, Mark Brown wrote:
> > On Fri, Sep 11, 2020 at 01:58:06PM +0530, kuldip dwivedi wrote:
> >
> >> /* find the resources - configuration register address space */
> >> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> >> "fspi_base");
> >> +#ifdef CONFIG_ACPI
> >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); #endif
> >
> > This is buggy, it is equivalent to just removing the name based lookup
> > since we'll do the name based lookup then unconditionally overwrite
> > the results with an index based lookup.
> >
>
> Also, note that CONFIG_ACPI kernels may still boot in DT mode.
In case of ACPI we need to use indexed based lookup.
For reference Please see , Line :23 and Line:24
https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platfor
ms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Dsdt/FSPI.asl?h=LX2160_UEFI_
ACPI_EAR3
Here I can think two possible approaches
1. Changes in DT to use indexed based lookup
2. We Can check for ACPI fw node to distinguish between DT and ACPI like
below..
if (is_acpi_node(f->dev->fwnode))
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
else
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"fspi_base");
Please share your thoughts
Thanks
On Fri, Sep 11, 2020 at 05:32:24PM +0530, Kuldip Dwivedi wrote:
> Here I can think two possible approaches
> 1. Changes in DT to use indexed based lookup
This is not going to be OK, it will break existing users relying on the
naming and makes the DTs less usable.
> 2. We Can check for ACPI fw node to distinguish between DT and ACPI like
> below..
> if (is_acpi_node(f->dev->fwnode))
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> else
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "fspi_base");
Yes, this should be good.
Hi Kuldeep,
> -----Original Message-----
> From: kuldip dwivedi <[email protected]>
> Sent: Friday, September 11, 2020 1:58 PM
> To: Ashish Kumar <[email protected]>; Yogesh Gaur
> <[email protected]>; Mark Brown <[email protected]>; linux-
> [email protected]; [email protected]
> Cc: Varun Sethi <[email protected]>; Arokia Samy <[email protected]>;
> Ard Biesheuvel <[email protected]>; Samer El-Haj-Mahmoud
> <[email protected]>; Paul Yang <[email protected]>;
> kuldip dwivedi <[email protected]>
> Subject: [EXT] [PATCH v2] spi: spi-nxp-fspi: Add ACPI support
>
> Caution: EXT Email
>
> Currently NXP fspi driver has support of DT only. Adding ACPI
> support to the driver so that it can be used by UEFI firmware
> booting in ACPI mode. This driver will be probed if any firmware
> will expose HID "NXP0009" in DSDT table.
>
> Signed-off-by: kuldip dwivedi <[email protected]>
> ---
Please capture version change summary.
Regards
Ashish
>
> Notes:
> 1. Add ACPI match table, NXP members are added to confirm HID for FSPI
> 2. Change the DT specific APIs to device property APIs
> so that same API can be used in DT and ACPi mode.
> 3. Omit clock configuration part - in ACPI world, the firmware
> is responsible for clock maintenance.
> 4. This patch is tested on LX2160A platform
>
> drivers/spi/spi-nxp-fspi.c | 61 +++++++++++++++++++++++++++-----------
> 1 file changed, 44 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index 1ccda82da206..9e3991ec0dd2 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -3,7 +3,8 @@
> /*
> * NXP FlexSPI(FSPI) controller driver.
> *
> - * Copyright 2019 NXP.
> + * Copyright 2019-2020 NXP
> + * Copyright 2020 Puresoftware Ltd.
> *
> * FlexSPI is a flexsible SPI host controller which supports two SPI
> * channels and up to 4 external devices. Each channel supports
> @@ -30,6 +31,7 @@
> * Frieder Schrempf <[email protected]>
> */
>
> +#include <linux/acpi.h>
> #include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/completion.h>
> @@ -563,6 +565,9 @@ static int nxp_fspi_clk_prep_enable(struct nxp_fspi
> *f)
> {
> int ret;
>
> + if (is_acpi_node(f->dev->fwnode))
> + return 0;
> +
> ret = clk_prepare_enable(f->clk_en);
> if (ret)
> return ret;
> @@ -576,10 +581,15 @@ static int nxp_fspi_clk_prep_enable(struct
> nxp_fspi *f)
> return 0;
> }
>
> -static void nxp_fspi_clk_disable_unprep(struct nxp_fspi *f)
> +static int nxp_fspi_clk_disable_unprep(struct nxp_fspi *f)
> {
> + if (is_acpi_node(f->dev->fwnode))
> + return 0;
> +
> clk_disable_unprepare(f->clk);
> clk_disable_unprepare(f->clk_en);
> +
> + return 0;
> }
>
> /*
> @@ -1001,7 +1011,7 @@ static int nxp_fspi_probe(struct platform_device
> *pdev)
>
> f = spi_controller_get_devdata(ctlr);
> f->dev = dev;
> - f->devtype_data = of_device_get_match_data(dev);
> + f->devtype_data = device_get_match_data(dev);
> if (!f->devtype_data) {
> ret = -ENODEV;
> goto err_put_ctrl;
> @@ -1011,6 +1021,9 @@ static int nxp_fspi_probe(struct platform_device
> *pdev)
>
> /* find the resources - configuration register address space */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "fspi_base");
> +#ifdef CONFIG_ACPI
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +#endif
> f->iobase = devm_ioremap_resource(dev, res);
> if (IS_ERR(f->iobase)) {
> ret = PTR_ERR(f->iobase);
> @@ -1019,6 +1032,9 @@ static int nxp_fspi_probe(struct platform_device
> *pdev)
>
> /* find the resources - controller memory mapped space */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "fspi_mmap");
> +#ifdef CONFIG_ACPI
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +#endif
> if (!res) {
> ret = -ENODEV;
> goto err_put_ctrl;
> @@ -1029,22 +1045,24 @@ static int nxp_fspi_probe(struct platform_device
> *pdev)
> f->memmap_phy_size = resource_size(res);
>
> /* find the clocks */
> - f->clk_en = devm_clk_get(dev, "fspi_en");
> - if (IS_ERR(f->clk_en)) {
> - ret = PTR_ERR(f->clk_en);
> - goto err_put_ctrl;
> - }
> + if (dev_of_node(&pdev->dev)) {
> + f->clk_en = devm_clk_get(dev, "fspi_en");
> + if (IS_ERR(f->clk_en)) {
> + ret = PTR_ERR(f->clk_en);
> + goto err_put_ctrl;
> + }
>
> - f->clk = devm_clk_get(dev, "fspi");
> - if (IS_ERR(f->clk)) {
> - ret = PTR_ERR(f->clk);
> - goto err_put_ctrl;
> - }
> + f->clk = devm_clk_get(dev, "fspi");
> + if (IS_ERR(f->clk)) {
> + ret = PTR_ERR(f->clk);
> + goto err_put_ctrl;
> + }
>
> - ret = nxp_fspi_clk_prep_enable(f);
> - if (ret) {
> - dev_err(dev, "can not enable the clock\n");
> - goto err_put_ctrl;
> + ret = nxp_fspi_clk_prep_enable(f);
> + if (ret) {
> + dev_err(dev, "can not enable the clock\n");
> + goto err_put_ctrl;
> + }
> }
>
> /* find the irq */
> @@ -1127,6 +1145,14 @@ static const struct of_device_id nxp_fspi_dt_ids[]
> = {
> };
> MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id nxp_fspi_acpi_ids[] = {
> + { "NXP0009", .driver_data = (kernel_ulong_t)&lx2160a_data, },
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, nxp_fspi_acpi_ids);
> +#endif
> +
> static const struct dev_pm_ops nxp_fspi_pm_ops = {
> .suspend = nxp_fspi_suspend,
> .resume = nxp_fspi_resume,
> @@ -1136,6 +1162,7 @@ static struct platform_driver nxp_fspi_driver = {
> .driver = {
> .name = "nxp-fspi",
> .of_match_table = nxp_fspi_dt_ids,
> + .acpi_match_table = ACPI_PTR(nxp_fspi_acpi_ids),
> .pm = &nxp_fspi_pm_ops,
> },
> .probe = nxp_fspi_probe,
> --
> 2.17.1
On Fri, Sep 11, 2020 at 12:28:47PM +0000, Ashish Kumar wrote:
> > Signed-off-by: kuldip dwivedi <[email protected]>
> Please capture version change summary.
>
> Regards
> Ashish
It's there:
> >
> > Notes:
> > 1. Add ACPI match table, NXP members are added to confirm HID for FSPI
> > 2. Change the DT specific APIs to device property APIs
> > so that same API can be used in DT and ACPi mode.
> > 3. Omit clock configuration part - in ACPI world, the firmware
> > is responsible for clock maintenance.
> > 4. This patch is tested on LX2160A platform