RE: [EXT] [PATCH v1] spi: spi-nxp-fspi: Add ACPI support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi Kuldeep Dwivedi,

> -----Original Message-----
> From: kuldip dwivedi <kuldip.dwivedi@xxxxxxxxxxxxxxxx>
> Sent: Tuesday, September 8, 2020 11:32 AM
> To: Ashish Kumar <ashish.kumar@xxxxxxx>; Yogesh Gaur
> <yogeshgaur.83@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; linux-
> spi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Varun Sethi <V.Sethi@xxxxxxx>; Arokia Samy <arokia.samy@xxxxxxx>;
> kuldip dwivedi <kuldip.dwivedi@xxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxx>
>   */
> 
> +#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
[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux