* [PATCHv2] mtd: spi-nor: cadence-quadspi: add reset control
@ 2019-04-09 15:38 Dinh Nguyen
2019-04-17 8:48 ` Tudor.Ambarus
0 siblings, 1 reply; 3+ messages in thread
From: Dinh Nguyen @ 2019-04-09 15:38 UTC (permalink / raw)
To: linux-mtd
Cc: dinguyen, marex, tudor.ambarus, dwmw2, computersforpeace,
bbrezillon, vigneshr, linux-kernel, Tien-Fong Chee
Get the reset control for the QSPI controller and bring it out of reset.
Suggested-by: Tien-Fong Chee <tien.fong.chee@intel.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v2: use devm_reset_control_get_optional_exclusive
print an error message
return -EPROBE_DEFER
---
drivers/mtd/spi-nor/cadence-quadspi.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 792628750eec..c548567adcf0 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -34,6 +34,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/reset.h>
#include <linux/sched.h>
#include <linux/spi/spi.h>
#include <linux/timer.h>
@@ -1336,6 +1337,7 @@ static int cqspi_probe(struct platform_device *pdev)
struct cqspi_st *cqspi;
struct resource *res;
struct resource *res_ahb;
+ struct reset_control *rstc;
const struct cqspi_driver_platdata *ddata;
int ret;
int irq;
@@ -1362,6 +1364,18 @@ static int cqspi_probe(struct platform_device *pdev)
return PTR_ERR(cqspi->clk);
}
+ /* Obtain QSPI reset control */
+ rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
+ if (IS_ERR(rstc)) {
+ dev_err(dev, "Cannot get QSPI reset.\n");
+ if (PTR_ERR(rstc) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ } else {
+ reset_control_assert(rstc);
+ udelay(1);
+ reset_control_deassert(rstc);
+ }
+
/* Obtain and remap controller address. */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
cqspi->iobase = devm_ioremap_resource(dev, res);
--
2.20.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCHv2] mtd: spi-nor: cadence-quadspi: add reset control
2019-04-09 15:38 [PATCHv2] mtd: spi-nor: cadence-quadspi: add reset control Dinh Nguyen
@ 2019-04-17 8:48 ` Tudor.Ambarus
2019-05-07 19:17 ` Dinh Nguyen
0 siblings, 1 reply; 3+ messages in thread
From: Tudor.Ambarus @ 2019-04-17 8:48 UTC (permalink / raw)
To: dinguyen, linux-mtd
Cc: marex, dwmw2, computersforpeace, bbrezillon, vigneshr,
linux-kernel, tien.fong.chee
Hi, Dinh,
On 04/09/2019 06:38 PM, Dinh Nguyen wrote:
> Get the reset control for the QSPI controller and bring it out of reset.
Is there a public datasheet where I can check this?
>
> Suggested-by: Tien-Fong Chee <tien.fong.chee@intel.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
> v2: use devm_reset_control_get_optional_exclusive
> print an error message
> return -EPROBE_DEFER
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 14 ++++++++++++++
would you update the bindings as well?
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 792628750eec..c548567adcf0 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -34,6 +34,7 @@
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> #include <linux/sched.h>
> #include <linux/spi/spi.h>
> #include <linux/timer.h>
> @@ -1336,6 +1337,7 @@ static int cqspi_probe(struct platform_device *pdev)
> struct cqspi_st *cqspi;
> struct resource *res;
> struct resource *res_ahb;
> + struct reset_control *rstc;
> const struct cqspi_driver_platdata *ddata;
> int ret;
> int irq;
> @@ -1362,6 +1364,18 @@ static int cqspi_probe(struct platform_device *pdev)
> return PTR_ERR(cqspi->clk);
> }
>
> + /* Obtain QSPI reset control */
OSPI version of the IP has been added recently. Is this available just for QSPI?
> + rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
> + if (IS_ERR(rstc)) {
> + dev_err(dev, "Cannot get QSPI reset.\n");
> + if (PTR_ERR(rstc) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
why ignoring all the other possible errors? Maybe return PTR_ERR(rstc); ?
> + } else {
> + reset_control_assert(rstc);
Shouldn't the clock be enabled before asserting reset?
Does it matter if reset_control_assert() is returning any error?
> + udelay(1);
Is there a range or 1 us is the maximum time that we should wait?
> + reset_control_deassert(rstc);
Should we check if reset_control_deassert() is returning any error?
Cheers,
ta
> + }
> +
> /* Obtain and remap controller address. */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> cqspi->iobase = devm_ioremap_resource(dev, res);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] mtd: spi-nor: cadence-quadspi: add reset control
2019-04-17 8:48 ` Tudor.Ambarus
@ 2019-05-07 19:17 ` Dinh Nguyen
0 siblings, 0 replies; 3+ messages in thread
From: Dinh Nguyen @ 2019-05-07 19:17 UTC (permalink / raw)
To: Tudor.Ambarus, linux-mtd
Cc: marex, dwmw2, computersforpeace, bbrezillon, vigneshr,
linux-kernel, tien.fong.chee
On 4/17/19 3:48 AM, Tudor.Ambarus@microchip.com wrote:
> Hi, Dinh,
>
> On 04/09/2019 06:38 PM, Dinh Nguyen wrote:
>> Get the reset control for the QSPI controller and bring it out of reset.
>
> Is there a public datasheet where I can check this?
You can look at the reset manager bits here:
https://www.intel.com/content/www/us/en/programmable/hps/arria-10/hps.html#reg_soc_top/sfo1429890575955.html
Otherwise, the QSPI block is just the standard IP from Cadence.
>>
>> Suggested-by: Tien-Fong Chee <tien.fong.chee@intel.com>
>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> v2: use devm_reset_control_get_optional_exclusive
>> print an error message
>> return -EPROBE_DEFER
>> ---
>> drivers/mtd/spi-nor/cadence-quadspi.c | 14 ++++++++++++++
>
> would you update the bindings as well?
Yes, I will do that.
>
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 792628750eec..c548567adcf0 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -34,6 +34,7 @@
>> #include <linux/of.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/reset.h>
>> #include <linux/sched.h>
>> #include <linux/spi/spi.h>
>> #include <linux/timer.h>
>> @@ -1336,6 +1337,7 @@ static int cqspi_probe(struct platform_device *pdev)
>> struct cqspi_st *cqspi;
>> struct resource *res;
>> struct resource *res_ahb;
>> + struct reset_control *rstc;
>> const struct cqspi_driver_platdata *ddata;
>> int ret;
>> int irq;
>> @@ -1362,6 +1364,18 @@ static int cqspi_probe(struct platform_device *pdev)
>> return PTR_ERR(cqspi->clk);
>> }
>>
>> + /* Obtain QSPI reset control */
>
> OSPI version of the IP has been added recently. Is this available just for QSPI?
>
>> + rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
>> + if (IS_ERR(rstc)) {
>> + dev_err(dev, "Cannot get QSPI reset.\n");
>> + if (PTR_ERR(rstc) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>
> why ignoring all the other possible errors? Maybe return PTR_ERR(rstc); ?
Ok..
>
>> + } else {
>> + reset_control_assert(rstc);
>
> Shouldn't the clock be enabled before asserting reset?
>
Yes, it should. Thanks for catching that!
> Does it matter if reset_control_assert() is returning any error?
>
>> + udelay(1);
>
> Is there a range or 1 us is the maximum time that we should wait?
>
>> + reset_control_deassert(rstc);
I'll add a 1-2 us range.
>
> Should we check if reset_control_deassert() is returning any error?
>
I don't think there is a need for error checking here. We've already
made sure that the reset property is there.
Thanks for the review...
Dinh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-07 19:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 15:38 [PATCHv2] mtd: spi-nor: cadence-quadspi: add reset control Dinh Nguyen
2019-04-17 8:48 ` Tudor.Ambarus
2019-05-07 19:17 ` Dinh Nguyen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).