| Message ID | 20200911082806.115-1-kuldip.dwivedi@puresoftware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] spi: spi-nxp-fspi: Add ACPI support | expand |
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 <ard.biesheuvel@arm.com> > Sent: Friday, September 11, 2020 4:46 PM > To: Mark Brown <broonie@kernel.org>; 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>; Samer El-Haj-Mahmoud <Samer.El-Haj- > Mahmoud@arm.com>; Paul Yang <Paul.Yang@arm.com> > 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 <kuldip.dwivedi@puresoftware.com> > Sent: Friday, September 11, 2020 1:58 PM > 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>; > Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Samer El-Haj-Mahmoud > <Samer.El-Haj-Mahmoud@arm.com>; Paul Yang <Paul.Yang@arm.com>; > kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > 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 <kuldip.dwivedi@puresoftware.com> > --- 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 <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; > } > > /* > @@ -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 <kuldip.dwivedi@puresoftware.com> > 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
Hi Mark, > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Friday, September 11, 2020 6:04 PM > To: Ashish Kumar <ashish.kumar@nxp.com> > Cc: kuldip dwivedi <kuldip.dwivedi@puresoftware.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>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; > Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Paul Yang > <Paul.Yang@arm.com> > Subject: Re: [EXT] [PATCH v2] spi: spi-nxp-fspi: Add ACPI support > > On Fri, Sep 11, 2020 at 12:28:47PM +0000, Ashish Kumar wrote: > > > Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > > > Please capture version change summary. > > > > Regards > > Ashish > > It's there: It did not mentioned about removing SWRST, which was present in v1. v2:Notes Looked very similar to v1. 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
> -----Original Message----- > From: Ashish Kumar <ashish.kumar@nxp.com> > Sent: Friday, September 11, 2020 7:03 PM > To: Mark Brown <broonie@kernel.org> > Cc: kuldip dwivedi <kuldip.dwivedi@puresoftware.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>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Samer El- > Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Paul Yang > <Paul.Yang@arm.com> > Subject: RE: [EXT] [PATCH v2] spi: spi-nxp-fspi: Add ACPI support > > Hi Mark, > > > -----Original Message----- > > From: Mark Brown <broonie@kernel.org> > > Sent: Friday, September 11, 2020 6:04 PM > > To: Ashish Kumar <ashish.kumar@nxp.com> > > Cc: kuldip dwivedi <kuldip.dwivedi@puresoftware.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>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Samer > > El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Paul Yang > > <Paul.Yang@arm.com> > > Subject: Re: [EXT] [PATCH v2] spi: spi-nxp-fspi: Add ACPI support > > > > On Fri, Sep 11, 2020 at 12:28:47PM +0000, Ashish Kumar wrote: > > > > Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com> > > > > > Please capture version change summary. > > > > > > Regards > > > Ashish > > > > It's there: > It did not mentioned about removing SWRST, which was present in v1. > v2:Notes Looked very similar to v1. Removed SWRST changes just to keep functionality same in ACP and DT mode, I will post that change separately if It will be required. For tracking I am adding this point in notes > > 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 5. SWRST reset changes are removed from v1 as to keep functionality same in ACP and DT mode, I will post that change separately if It will be required
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 <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; } /* @@ -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,
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, 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(-)