| Message ID | 20200908060227.299-1-kuldip.dwivedi@puresoftware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] spi: spi-nxp-fspi: Add ACPI support | expand |
Hi Kuldeep Dwivedi, > -----Original Message----- > From: kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > Sent: Tuesday, September 8, 2020 11:32 AM > To: Ashish Kumar <ashish.kumar@nxp.com>; Yogesh Gaur > <yogeshgaur.83@gmail.com>; Mark Brown <broonie@kernel.org>; linux- > spi@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Varun Sethi <V.Sethi@nxp.com>; Arokia Samy <arokia.samy@nxp.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > Subject: [EXT] [PATCH v1] 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 <kuldip.dwivedi@puresoftware.com> Does these change affects non-ACPI FSPI driver in Linux? What test case were run to verify the same? > --- > > Notes: > 1. Add ACPI match table > 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 | 66 +++++++++++++++++++++++++++----------- > 1 file changed, 47 insertions(+), 19 deletions(-) > > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c > index 1ccda82da206..acdb186ddfb2 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 Why Update NXP copyright? > + * 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 <frieder.schrempf@kontron.de> > */ > > +#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; > } > > /* > @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) > return ret; > > /* Reset the module */ > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > + Why is this SW reset needed now? This will alter nxp_fspi_resume() function as well. > /* w1c register, wait unit clear */ > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, > FSPI_MCR0_SWRST, 0, POLL_TOUT, false); > @@ -1001,7 +1013,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 +1023,8 @@ 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"); > + if (!res) > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); Why is this needed ?, _byname() will get you fspi_base value. > f->iobase = devm_ioremap_resource(dev, res); > if (IS_ERR(f->iobase)) { > ret = PTR_ERR(f->iobase); > @@ -1020,8 +1034,11 @@ 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"); > if (!res) { > - ret = -ENODEV; > - goto err_put_ctrl; > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); Why is this needed? _byname() will get you fspi_mmap. If fspi_mmap is not present then fetch 1st IORESOURE_MEM ? Regards Ashish > + if (!res) { > + ret = -ENODEV; > + goto err_put_ctrl; > + } > } > > /* assign memory mapped starting address and mapped size. */ > @@ -1029,22 +1046,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 +1146,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 +1163,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 Tue, Sep 08, 2020 at 11:32:27AM +0530, kuldip dwivedi wrote: This appears to be v2 not v1? > 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. As I said on your previous version: | Does NXP know about this ID assignment from their namespace? ACPI IDs | should be namespaced by whoever's assigning the ID to avoid collisions. Please don't ignore review comments, people are generally making them for a reason and are likely to have the same concerns if issues remain unaddressed. Having to repeat the same comments can get repetitive and make people question the value of time spent reviewing. If you disagree with the review comments that's fine but you need to reply and discuss your concerns so that the reviewer can understand your decisions. > @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) > return ret; > > /* Reset the module */ > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > + > /* w1c register, wait unit clear */ > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, > FSPI_MCR0_SWRST, 0, POLL_TOUT, false); Why are you adding this reset? How is it connected to adding ACPI support - it looks like it should be a separate patch.
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Wednesday, September 9, 2020 5:32 PM > To: kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > Cc: Ashish Kumar <ashish.kumar@nxp.com>; Yogesh Gaur > <yogeshgaur.83@gmail.com>; linux-spi@vger.kernel.org; linux- > kernel@vger.kernel.org; Varun Sethi <V.Sethi@nxp.com>; Arokia Samy > <arokia.samy@nxp.com> > Subject: Re: [PATCH v1] spi: spi-nxp-fspi: Add ACPI support > > On Tue, Sep 08, 2020 at 11:32:27AM +0530, kuldip dwivedi wrote: > > This appears to be v2 not v1? This is separate Patch so v1 should be OK here. Earlier one was related to DSPI. https://lore.kernel.org/linux-spi/20200827113216.GA4674@sirena.org.uk/T/#t > > > 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. > > As I said on your previous version: > > | Does NXP know about this ID assignment from their namespace? ACPI IDs > | should be namespaced by whoever's assigning the ID to avoid collisions. Yes, NXP is aware. > > Please don't ignore review comments, people are generally making them for a > reason and are likely to have the same concerns if issues remain unaddressed. > Having to repeat the same comments can get repetitive and make people question > the value of time spent reviewing. If you disagree with the review comments > that's fine but you need to reply and discuss your concerns so that the reviewer > can understand your decisions. This is new Patch for different IP (FSPI) and scenario is different from DSPI driver. > > > @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) > > return ret; > > > > /* Reset the module */ > > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > > + > > /* w1c register, wait unit clear */ > > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, > > FSPI_MCR0_SWRST, 0, POLL_TOUT, false); > > Why are you adding this reset? How is it connected to adding ACPI support - it > looks like it should be a separate patch. I observed a kernel panic in setting up the driver, and this fixed the issue.
> -----Original Message----- > From: Ashish Kumar <ashish.kumar@nxp.com> > Sent: Wednesday, September 9, 2020 4:57 PM > To: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Yogesh Gaur > <yogeshgaur.83@gmail.com>; Mark Brown <broonie@kernel.org>; linux- > spi@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Varun Sethi <V.Sethi@nxp.com>; Arokia Samy <arokia.samy@nxp.com> > Subject: RE: [EXT] [PATCH v1] spi: spi-nxp-fspi: Add ACPI support > > Hi Kuldeep Dwivedi, Don't mind but It's Kuldip not Kuldeep > > > -----Original Message----- > > From: kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > > Sent: Tuesday, September 8, 2020 11:32 AM > > To: Ashish Kumar <ashish.kumar@nxp.com>; Yogesh Gaur > > <yogeshgaur.83@gmail.com>; Mark Brown <broonie@kernel.org>; linux- > > spi@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: Varun Sethi <V.Sethi@nxp.com>; Arokia Samy <arokia.samy@nxp.com>; > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > > Subject: [EXT] [PATCH v1] 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 <kuldip.dwivedi@puresoftware.com> > Does these change affects non-ACPI FSPI driver in Linux? What test case were run > to verify the same? I have verified on both DT and ACPI mode with below method. In this method we Used mtd utility to write some content and then read from same address. Address 0x3000000 has been selected to avoid any data loss of existing UEFI variables. root@localhost:/mnt# ls 1.c EFI i2c_utils mtd_debug mtdinfo reportfreq64 root@localhost:/mnt# cat 1.c 1234567890987654321 root@localhost:/mnt# root@localhost:/mnt# ./mtdinfo Count of MTD devices: 2 Present MTD devices: mtd0, mtd1 Sysfs interface supported: yes root@localhost:/mnt# ./mtd_debug erase /dev/mtd1 0x3000000 0x1000 Erased 4096 bytes from address 0x03000000 in flash root@localhost:/mnt# ./mtd_debug write /dev/mtd1 0x3000000 20 1.c Copied 20 bytes from 1.c to address 0x03000000 in flash root@lOcalhost:/mnt# ./mtd_debug read /dev/mtd1 0x3000000 20 2.c Copied 20 bytes from address 0x03000000 in flash tO 2.c root@localhost:/mnt# cat 2.c 1234567890987654321 > > --- > > > > Notes: > > 1. Add ACPI match table > > 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 | 66 > > +++++++++++++++++++++++++++----------- > > 1 file changed, 47 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c > > index 1ccda82da206..acdb186ddfb2 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 > Why Update NXP copyright? This is asked by NXP only > > > + * 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 <frieder.schrempf@kontron.de> > > */ > > > > +#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; > > } > > > > /* > > @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) > > return ret; > > > > /* Reset the module */ > > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > > + > Why is this SW reset needed now? This will alter nxp_fspi_resume() function as > well. I observed a kernel panic during setting up the driver in nxp_fspi_default_setup function in ACPI boot and this is fixed. > > > /* w1c register, wait unit clear */ > > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, > > FSPI_MCR0_SWRST, 0, POLL_TOUT, > > false); @@ -1001,7 +1013,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 +1023,8 @@ 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"); > > + if (!res) > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > Why is this needed ?, _byname() will get you fspi_base value. In ACPI we can not pass resource value by name where as we have to pass By index. So in 0th Index there is FSPI_BASE and in 1st index FSPIMM_BASE. 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 > > > f->iobase = devm_ioremap_resource(dev, res); > > if (IS_ERR(f->iobase)) { > > ret = PTR_ERR(f->iobase); @@ -1020,8 +1034,11 @@ > > 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"); > > if (!res) { > > - ret = -ENODEV; > > - goto err_put_ctrl; > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > Why is this needed? _byname() will get you fspi_mmap. > If fspi_mmap is not present then fetch 1st IORESOURE_MEM ? In ACPI we can not pass resource value by name where as we have to pass By index. So in 0th Index there is FSPI_BASE and in 1st index FSPIMM_BASE. 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 > > Regards > Ashish > > + if (!res) { > > + ret = -ENODEV; > > + goto err_put_ctrl; > > + } > > } > > > > /* assign memory mapped starting address and mapped size. */ > > @@ -1029,22 +1046,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 +1146,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 +1163,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 Thanks,
On Wed, Sep 09, 2020 at 06:29:10PM +0530, Kuldip Dwivedi wrote: > > | Does NXP know about this ID assignment from their namespace? ACPI IDs > > | should be namespaced by whoever's assigning the ID to avoid > collisions. > Yes, NXP is aware. Can anyone from NXP confirm this? > > Please don't ignore review comments, people are generally making them > for a > > reason and are likely to have the same concerns if issues remain > This is new Patch for different IP (FSPI) and scenario is different from > DSPI driver. If a generic issue like this exists with one patch you should expect that exactly the same issue is going to come up with other very similar patches and therefore ensure they are addressed so people don't feel like you are ignoring them. > > > /* Reset the module */ > > > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > > > + > > Why are you adding this reset? How is it connected to adding ACPI > support - it > > looks like it should be a separate patch. > I observed a kernel panic in setting up the driver, and this fixed the > issue. At the very least this would need to be called out in the changelog but like I say it should really be a separate patch.
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index 1ccda82da206..acdb186ddfb2 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 <frieder.schrempf@kontron.de> */ +#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; } /* @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) return ret; /* Reset the module */ + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); + /* w1c register, wait unit clear */ ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, FSPI_MCR0_SWRST, 0, POLL_TOUT, false); @@ -1001,7 +1013,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 +1023,8 @@ 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"); + if (!res) + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); f->iobase = devm_ioremap_resource(dev, res); if (IS_ERR(f->iobase)) { ret = PTR_ERR(f->iobase); @@ -1020,8 +1034,11 @@ 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"); if (!res) { - ret = -ENODEV; - goto err_put_ctrl; + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (!res) { + ret = -ENODEV; + goto err_put_ctrl; + } } /* assign memory mapped starting address and mapped size. */ @@ -1029,22 +1046,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 +1146,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 +1163,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,
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 <kuldip.dwivedi@puresoftware.com> --- Notes: 1. Add ACPI match table 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 | 66 +++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 19 deletions(-)