Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi, On 9/28/2017 8:31 PM, matthew.gerl...@linux.intel.com wrote: > > Hi Vignesh, > > I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi > kernel, and it did not go well. Commands to the flash chip timedout > resulting in the probe function failing. I ran into other problems, not > related to cadence-quadspi, that prevented me from testing against 4.9 and > 4.12 kernels, but I suspect similar behavior. > Ok, thanks! I will keep the clk_*() calls for now. Regards Vignesh > Matthew Gerlach > > On Wed, 27 Sep 2017, Vignesh R wrote: > >> Hi Matthew, >> >> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: >> [...] >> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for >> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see >> if its possible to get rid of clk_*() calls in favor of pm_*() calls. > > Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. >>> >>> The question is, if we do the replacement, will it break on socfpga ? >>> A quick test might be useful. >>> >> >> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls >> like below patch would be helpful: >> >> >> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c >> b/drivers/mtd/spi-nor/cadence-quadspi.c >> index 53c7d8e0327a..7ad3e176cc88 100644 >> --- a/drivers/mtd/spi-nor/cadence-quadspi.c >> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c >> @@ -34,6 +34,7 @@ >> #include >> #include >> #include >> +#include >> >> #define CQSPI_NAME "cadence-qspi" >> #define CQSPI_MAX_CHIPSELECT 16 >> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) >>return -ENXIO; >>} >> >> - ret = clk_prepare_enable(cqspi->clk); >> - if (ret) { >> - dev_err(dev, "Cannot enable QSPI clock.\n"); >> - return ret; >> - } >> + pm_runtime_enable(dev); >> + pm_runtime_get_sync(dev); >> >>cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); >> >> >> >> >> >> -- >> Regards >> Vignesh >> >
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi, On 9/28/2017 8:31 PM, matthew.gerl...@linux.intel.com wrote: > > Hi Vignesh, > > I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi > kernel, and it did not go well. Commands to the flash chip timedout > resulting in the probe function failing. I ran into other problems, not > related to cadence-quadspi, that prevented me from testing against 4.9 and > 4.12 kernels, but I suspect similar behavior. > Ok, thanks! I will keep the clk_*() calls for now. Regards Vignesh > Matthew Gerlach > > On Wed, 27 Sep 2017, Vignesh R wrote: > >> Hi Matthew, >> >> On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: >> [...] >> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for >> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see >> if its possible to get rid of clk_*() calls in favor of pm_*() calls. > > Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. >>> >>> The question is, if we do the replacement, will it break on socfpga ? >>> A quick test might be useful. >>> >> >> yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls >> like below patch would be helpful: >> >> >> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c >> b/drivers/mtd/spi-nor/cadence-quadspi.c >> index 53c7d8e0327a..7ad3e176cc88 100644 >> --- a/drivers/mtd/spi-nor/cadence-quadspi.c >> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c >> @@ -34,6 +34,7 @@ >> #include >> #include >> #include >> +#include >> >> #define CQSPI_NAME "cadence-qspi" >> #define CQSPI_MAX_CHIPSELECT 16 >> @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) >>return -ENXIO; >>} >> >> - ret = clk_prepare_enable(cqspi->clk); >> - if (ret) { >> - dev_err(dev, "Cannot enable QSPI clock.\n"); >> - return ret; >> - } >> + pm_runtime_enable(dev); >> + pm_runtime_get_sync(dev); >> >>cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); >> >> >> >> >> >> -- >> Regards >> Vignesh >> >
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi Vignesh, I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi kernel, and it did not go well. Commands to the flash chip timedout resulting in the probe function failing. I ran into other problems, not related to cadence-quadspi, that prevented me from testing against 4.9 and 4.12 kernels, but I suspect similar behavior. Matthew Gerlach On Wed, 27 Sep 2017, Vignesh R wrote: Hi Matthew, On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: [...] Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. The question is, if we do the replacement, will it break on socfpga ? A quick test might be useful. yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls like below patch would be helpful: diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index 53c7d8e0327a..7ad3e176cc88 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -34,6 +34,7 @@ #include #include #include +#include #define CQSPI_NAME "cadence-qspi" #define CQSPI_MAX_CHIPSELECT 16 @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) return -ENXIO; } - ret = clk_prepare_enable(cqspi->clk); - if (ret) { - dev_err(dev, "Cannot enable QSPI clock.\n"); - return ret; - } + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); -- Regards Vignesh
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi Vignesh, I tried this patch on an Arria10 SOCFPGA devkit against the 4.1.33-ltsi kernel, and it did not go well. Commands to the flash chip timedout resulting in the probe function failing. I ran into other problems, not related to cadence-quadspi, that prevented me from testing against 4.9 and 4.12 kernels, but I suspect similar behavior. Matthew Gerlach On Wed, 27 Sep 2017, Vignesh R wrote: Hi Matthew, On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: [...] Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. The question is, if we do the replacement, will it break on socfpga ? A quick test might be useful. yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls like below patch would be helpful: diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index 53c7d8e0327a..7ad3e176cc88 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -34,6 +34,7 @@ #include #include #include +#include #define CQSPI_NAME "cadence-qspi" #define CQSPI_MAX_CHIPSELECT 16 @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) return -ENXIO; } - ret = clk_prepare_enable(cqspi->clk); - if (ret) { - dev_err(dev, "Cannot enable QSPI clock.\n"); - return ret; - } + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); -- Regards Vignesh
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi Matthew, On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: [...] Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. >>> >>> Not of the top of my head, sorry. +CC Matthew, he should know. >> >> I am not an expert at the clock framework nor the power management, but I >> did ask around a bit. No one I asked was planning to change the clk_*() >> calls to pm_*() call, but the feedback was that it would be a good idea. > > The question is, if we do the replacement, will it break on socfpga ? > A quick test might be useful. > yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls like below patch would be helpful: diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index 53c7d8e0327a..7ad3e176cc88 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -34,6 +34,7 @@ #include #include #include +#include #define CQSPI_NAME "cadence-qspi" #define CQSPI_MAX_CHIPSELECT 16 @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) return -ENXIO; } - ret = clk_prepare_enable(cqspi->clk); - if (ret) { - dev_err(dev, "Cannot enable QSPI clock.\n"); - return ret; - } + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); -- Regards Vignesh
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
Hi Matthew, On Tuesday 26 September 2017 05:19 AM, Marek Vasut wrote: [...] Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. >>> >>> Not of the top of my head, sorry. +CC Matthew, he should know. >> >> I am not an expert at the clock framework nor the power management, but I >> did ask around a bit. No one I asked was planning to change the clk_*() >> calls to pm_*() call, but the feedback was that it would be a good idea. > > The question is, if we do the replacement, will it break on socfpga ? > A quick test might be useful. > yes, a quick qspi test with clk_prepare_enable() replaced by pm_*() calls like below patch would be helpful: diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c index 53c7d8e0327a..7ad3e176cc88 100644 --- a/drivers/mtd/spi-nor/cadence-quadspi.c +++ b/drivers/mtd/spi-nor/cadence-quadspi.c @@ -34,6 +34,7 @@ #include #include #include +#include #define CQSPI_NAME "cadence-qspi" #define CQSPI_MAX_CHIPSELECT 16 @@ -1206,11 +1207,8 @@ static int cqspi_probe(struct platform_device *pdev) return -ENXIO; } - ret = clk_prepare_enable(cqspi->clk); - if (ret) { - dev_err(dev, "Cannot enable QSPI clock.\n"); - return ret; - } + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); -- Regards Vignesh
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/26/2017 12:41 AM, matthew.gerl...@linux.intel.com wrote: > > > On Sun, 24 Sep 2017, Marek Vasut wrote: > >> On 09/24/2017 03:27 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 6:42 PM, Marek Vasut wrote: On 09/24/2017 03:08 PM, Vignesh R wrote: > > > On 9/24/2017 5:31 PM, Marek Vasut wrote: >> On 09/24/2017 12:59 PM, Vignesh R wrote: >>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>> >>> Signed-off-by: Vignesh R>> >> Are you planning to add some more fine-grained PM control later? > > Yes, I will need to add fine-grained PM control at some point. But, > for > now SoC does not really support low power mode or runtime power saving > option. > The fact that driver still uses clk_prepare_*() calls to > enable/disable > clocks instead of pm_*() calls makes it a bit tricky though. > > Just figured out I forgot to add cleanup code in error handling > path of > probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. >>> >>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for >>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see >>> if its possible to get rid of clk_*() calls in favor of pm_*() calls. >> >> Not of the top of my head, sorry. +CC Matthew, he should know. > > I am not an expert at the clock framework nor the power management, but I > did ask around a bit. No one I asked was planning to change the clk_*() > calls to pm_*() call, but the feedback was that it would be a good idea. The question is, if we do the replacement, will it break on socfpga ? A quick test might be useful. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/26/2017 12:41 AM, matthew.gerl...@linux.intel.com wrote: > > > On Sun, 24 Sep 2017, Marek Vasut wrote: > >> On 09/24/2017 03:27 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 6:42 PM, Marek Vasut wrote: On 09/24/2017 03:08 PM, Vignesh R wrote: > > > On 9/24/2017 5:31 PM, Marek Vasut wrote: >> On 09/24/2017 12:59 PM, Vignesh R wrote: >>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>> >>> Signed-off-by: Vignesh R >> >> Are you planning to add some more fine-grained PM control later? > > Yes, I will need to add fine-grained PM control at some point. But, > for > now SoC does not really support low power mode or runtime power saving > option. > The fact that driver still uses clk_prepare_*() calls to > enable/disable > clocks instead of pm_*() calls makes it a bit tricky though. > > Just figured out I forgot to add cleanup code in error handling > path of > probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. >>> >>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for >>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see >>> if its possible to get rid of clk_*() calls in favor of pm_*() calls. >> >> Not of the top of my head, sorry. +CC Matthew, he should know. > > I am not an expert at the clock framework nor the power management, but I > did ask around a bit. No one I asked was planning to change the clk_*() > calls to pm_*() call, but the feedback was that it would be a good idea. The question is, if we do the replacement, will it break on socfpga ? A quick test might be useful. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On Sun, 24 Sep 2017, Marek Vasut wrote: On 09/24/2017 03:27 PM, Vignesh R wrote: On 9/24/2017 6:42 PM, Marek Vasut wrote: On 09/24/2017 03:08 PM, Vignesh R wrote: On 9/24/2017 5:31 PM, Marek Vasut wrote: On 09/24/2017 12:59 PM, Vignesh R wrote: Add pm_runtime* calls to cadence-quadspi driver. This is required to switch on QSPI power domain on TI 66AK2G SoC during probe. Signed-off-by: Vignesh RAre you planning to add some more fine-grained PM control later? Yes, I will need to add fine-grained PM control at some point. But, for now SoC does not really support low power mode or runtime power saving option. The fact that driver still uses clk_prepare_*() calls to enable/disable clocks instead of pm_*() calls makes it a bit tricky though. Just figured out I forgot to add cleanup code in error handling path of probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. Matthew Gerlach -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On Sun, 24 Sep 2017, Marek Vasut wrote: On 09/24/2017 03:27 PM, Vignesh R wrote: On 9/24/2017 6:42 PM, Marek Vasut wrote: On 09/24/2017 03:08 PM, Vignesh R wrote: On 9/24/2017 5:31 PM, Marek Vasut wrote: On 09/24/2017 12:59 PM, Vignesh R wrote: Add pm_runtime* calls to cadence-quadspi driver. This is required to switch on QSPI power domain on TI 66AK2G SoC during probe. Signed-off-by: Vignesh R Are you planning to add some more fine-grained PM control later? Yes, I will need to add fine-grained PM control at some point. But, for now SoC does not really support low power mode or runtime power saving option. The fact that driver still uses clk_prepare_*() calls to enable/disable clocks instead of pm_*() calls makes it a bit tricky though. Just figured out I forgot to add cleanup code in error handling path of probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. I am not an expert at the clock framework nor the power management, but I did ask around a bit. No one I asked was planning to change the clk_*() calls to pm_*() call, but the feedback was that it would be a good idea. Matthew Gerlach -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:27 PM, Vignesh R wrote: > > > On 9/24/2017 6:42 PM, Marek Vasut wrote: >> On 09/24/2017 03:08 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 5:31 PM, Marek Vasut wrote: On 09/24/2017 12:59 PM, Vignesh R wrote: > Add pm_runtime* calls to cadence-quadspi driver. This is required to > switch on QSPI power domain on TI 66AK2G SoC during probe. > > Signed-off-by: Vignesh RAre you planning to add some more fine-grained PM control later? >>> >>> Yes, I will need to add fine-grained PM control at some point. But, for >>> now SoC does not really support low power mode or runtime power saving >>> option. >>> The fact that driver still uses clk_prepare_*() calls to enable/disable >>> clocks instead of pm_*() calls makes it a bit tricky though. >>> >>> Just figured out I forgot to add cleanup code in error handling path of >>> probe(). Will fix that and send a v4. >> >> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM >> either, so it's fine for now. >> > > Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for > QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see > if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:27 PM, Vignesh R wrote: > > > On 9/24/2017 6:42 PM, Marek Vasut wrote: >> On 09/24/2017 03:08 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 5:31 PM, Marek Vasut wrote: On 09/24/2017 12:59 PM, Vignesh R wrote: > Add pm_runtime* calls to cadence-quadspi driver. This is required to > switch on QSPI power domain on TI 66AK2G SoC during probe. > > Signed-off-by: Vignesh R Are you planning to add some more fine-grained PM control later? >>> >>> Yes, I will need to add fine-grained PM control at some point. But, for >>> now SoC does not really support low power mode or runtime power saving >>> option. >>> The fact that driver still uses clk_prepare_*() calls to enable/disable >>> clocks instead of pm_*() calls makes it a bit tricky though. >>> >>> Just figured out I forgot to add cleanup code in error handling path of >>> probe(). Will fix that and send a v4. >> >> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM >> either, so it's fine for now. >> > > Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for > QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see > if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 9/24/2017 6:42 PM, Marek Vasut wrote: > On 09/24/2017 03:08 PM, Vignesh R wrote: >> >> >> On 9/24/2017 5:31 PM, Marek Vasut wrote: >>> On 09/24/2017 12:59 PM, Vignesh R wrote: Add pm_runtime* calls to cadence-quadspi driver. This is required to switch on QSPI power domain on TI 66AK2G SoC during probe. Signed-off-by: Vignesh R>>> >>> Are you planning to add some more fine-grained PM control later? >> >> Yes, I will need to add fine-grained PM control at some point. But, for >> now SoC does not really support low power mode or runtime power saving >> option. >> The fact that driver still uses clk_prepare_*() calls to enable/disable >> clocks instead of pm_*() calls makes it a bit tricky though. >> >> Just figured out I forgot to add cleanup code in error handling path of >> probe(). Will fix that and send a v4. > > OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM > either, so it's fine for now. > Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls.
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 9/24/2017 6:42 PM, Marek Vasut wrote: > On 09/24/2017 03:08 PM, Vignesh R wrote: >> >> >> On 9/24/2017 5:31 PM, Marek Vasut wrote: >>> On 09/24/2017 12:59 PM, Vignesh R wrote: Add pm_runtime* calls to cadence-quadspi driver. This is required to switch on QSPI power domain on TI 66AK2G SoC during probe. Signed-off-by: Vignesh R >>> >>> Are you planning to add some more fine-grained PM control later? >> >> Yes, I will need to add fine-grained PM control at some point. But, for >> now SoC does not really support low power mode or runtime power saving >> option. >> The fact that driver still uses clk_prepare_*() calls to enable/disable >> clocks instead of pm_*() calls makes it a bit tricky though. >> >> Just figured out I forgot to add cleanup code in error handling path of >> probe(). Will fix that and send a v4. > > OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM > either, so it's fine for now. > Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see if its possible to get rid of clk_*() calls in favor of pm_*() calls.
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:08 PM, Vignesh R wrote: > > > On 9/24/2017 5:31 PM, Marek Vasut wrote: >> On 09/24/2017 12:59 PM, Vignesh R wrote: >>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>> >>> Signed-off-by: Vignesh R>> >> Are you planning to add some more fine-grained PM control later? > > Yes, I will need to add fine-grained PM control at some point. But, for > now SoC does not really support low power mode or runtime power saving > option. > The fact that driver still uses clk_prepare_*() calls to enable/disable > clocks instead of pm_*() calls makes it a bit tricky though. > > Just figured out I forgot to add cleanup code in error handling path of > probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:08 PM, Vignesh R wrote: > > > On 9/24/2017 5:31 PM, Marek Vasut wrote: >> On 09/24/2017 12:59 PM, Vignesh R wrote: >>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>> >>> Signed-off-by: Vignesh R >> >> Are you planning to add some more fine-grained PM control later? > > Yes, I will need to add fine-grained PM control at some point. But, for > now SoC does not really support low power mode or runtime power saving > option. > The fact that driver still uses clk_prepare_*() calls to enable/disable > clocks instead of pm_*() calls makes it a bit tricky though. > > Just figured out I forgot to add cleanup code in error handling path of > probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 9/24/2017 5:31 PM, Marek Vasut wrote: > On 09/24/2017 12:59 PM, Vignesh R wrote: >> Add pm_runtime* calls to cadence-quadspi driver. This is required to >> switch on QSPI power domain on TI 66AK2G SoC during probe. >> >> Signed-off-by: Vignesh R> > Are you planning to add some more fine-grained PM control later? Yes, I will need to add fine-grained PM control at some point. But, for now SoC does not really support low power mode or runtime power saving option. The fact that driver still uses clk_prepare_*() calls to enable/disable clocks instead of pm_*() calls makes it a bit tricky though. Just figured out I forgot to add cleanup code in error handling path of probe(). Will fix that and send a v4. > >> --- >> drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c >> b/drivers/mtd/spi-nor/cadence-quadspi.c >> index d9629e8f4798..2c8e6226d267 100644 >> --- a/drivers/mtd/spi-nor/cadence-quadspi.c >> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c >> @@ -31,6 +31,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev) >> return -ENXIO; >> } >> >> +pm_runtime_enable(>dev); >> +ret = pm_runtime_get_sync(>dev); >> +if (ret < 0) { >> +pm_runtime_put_noidle(>dev); >> +return ret; >> +} >> + >> ret = clk_prepare_enable(cqspi->clk); >> if (ret) { >> dev_err(dev, "Cannot enable QSPI clock.\n"); >> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev) >> >> clk_disable_unprepare(cqspi->clk); >> >> +pm_runtime_put_sync(>dev); >> +pm_runtime_disable(>dev); >> + >> return 0; >> } >> >> > >
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 9/24/2017 5:31 PM, Marek Vasut wrote: > On 09/24/2017 12:59 PM, Vignesh R wrote: >> Add pm_runtime* calls to cadence-quadspi driver. This is required to >> switch on QSPI power domain on TI 66AK2G SoC during probe. >> >> Signed-off-by: Vignesh R > > Are you planning to add some more fine-grained PM control later? Yes, I will need to add fine-grained PM control at some point. But, for now SoC does not really support low power mode or runtime power saving option. The fact that driver still uses clk_prepare_*() calls to enable/disable clocks instead of pm_*() calls makes it a bit tricky though. Just figured out I forgot to add cleanup code in error handling path of probe(). Will fix that and send a v4. > >> --- >> drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c >> b/drivers/mtd/spi-nor/cadence-quadspi.c >> index d9629e8f4798..2c8e6226d267 100644 >> --- a/drivers/mtd/spi-nor/cadence-quadspi.c >> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c >> @@ -31,6 +31,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev) >> return -ENXIO; >> } >> >> +pm_runtime_enable(>dev); >> +ret = pm_runtime_get_sync(>dev); >> +if (ret < 0) { >> +pm_runtime_put_noidle(>dev); >> +return ret; >> +} >> + >> ret = clk_prepare_enable(cqspi->clk); >> if (ret) { >> dev_err(dev, "Cannot enable QSPI clock.\n"); >> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev) >> >> clk_disable_unprepare(cqspi->clk); >> >> +pm_runtime_put_sync(>dev); >> +pm_runtime_disable(>dev); >> + >> return 0; >> } >> >> > >
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 12:59 PM, Vignesh R wrote: > Add pm_runtime* calls to cadence-quadspi driver. This is required to > switch on QSPI power domain on TI 66AK2G SoC during probe. > > Signed-off-by: Vignesh RAre you planning to add some more fine-grained PM control later? > --- > drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c > b/drivers/mtd/spi-nor/cadence-quadspi.c > index d9629e8f4798..2c8e6226d267 100644 > --- a/drivers/mtd/spi-nor/cadence-quadspi.c > +++ b/drivers/mtd/spi-nor/cadence-quadspi.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev) > return -ENXIO; > } > > + pm_runtime_enable(>dev); > + ret = pm_runtime_get_sync(>dev); > + if (ret < 0) { > + pm_runtime_put_noidle(>dev); > + return ret; > + } > + > ret = clk_prepare_enable(cqspi->clk); > if (ret) { > dev_err(dev, "Cannot enable QSPI clock.\n"); > @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev) > > clk_disable_unprepare(cqspi->clk); > > + pm_runtime_put_sync(>dev); > + pm_runtime_disable(>dev); > + > return 0; > } > > -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 12:59 PM, Vignesh R wrote: > Add pm_runtime* calls to cadence-quadspi driver. This is required to > switch on QSPI power domain on TI 66AK2G SoC during probe. > > Signed-off-by: Vignesh R Are you planning to add some more fine-grained PM control later? > --- > drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c > b/drivers/mtd/spi-nor/cadence-quadspi.c > index d9629e8f4798..2c8e6226d267 100644 > --- a/drivers/mtd/spi-nor/cadence-quadspi.c > +++ b/drivers/mtd/spi-nor/cadence-quadspi.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev) > return -ENXIO; > } > > + pm_runtime_enable(>dev); > + ret = pm_runtime_get_sync(>dev); > + if (ret < 0) { > + pm_runtime_put_noidle(>dev); > + return ret; > + } > + > ret = clk_prepare_enable(cqspi->clk); > if (ret) { > dev_err(dev, "Cannot enable QSPI clock.\n"); > @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev) > > clk_disable_unprepare(cqspi->clk); > > + pm_runtime_put_sync(>dev); > + pm_runtime_disable(>dev); > + > return 0; > } > > -- Best regards, Marek Vasut