[PATCH 1/2] mfd: madera: Increase wait for silicon reset

[PATCH 1/2] mfd: madera: Increase wait for silicon reset

[Richard Fitzgerald]

Based on latest silicon validation we should allow at least
2 milliseconds after DCVDD enable or hard reset released before
trying to communicate with the silicon.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 drivers/mfd/madera-core.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index 440030cecbbd..3ff9a0615658 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -135,6 +135,15 @@ EXPORT_SYMBOL_GPL(madera_name_from_type);
 #define MADERA_BOOT_POLL_INTERVAL_USEC		5000
 #define MADERA_BOOT_POLL_TIMEOUT_USEC		25000
 
+static void madera_por_delay(void)
+{
+	/*
+	 * After DCVDD enable or hard reset released wait for internal
+	 * POR (power-on-reset) before trying to communicate with the silicon
+	 */
+	usleep_range(2000, 3000);
+}
+
 static int madera_wait_for_boot(struct madera *madera)
 {
 	ktime_t timeout;
@@ -209,7 +218,7 @@ static void madera_disable_hard_reset(struct madera *madera)
 		return;
 
 	gpiod_set_raw_value_cansleep(madera->pdata.reset, 1);
-	usleep_range(1000, 2000);
+	madera_por_delay();
 }
 
 static int __maybe_unused madera_runtime_resume(struct device *dev)
@@ -225,6 +234,7 @@ static int __maybe_unused madera_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	madera_por_delay();
 	regcache_cache_only(madera->regmap, false);
 	regcache_cache_only(madera->regmap_32bit, false);
 
-- 
2.11.0

[PATCH 2/2] mfd: madera: Wait for BOOT_DONE before reading device ID

[Richard Fitzgerald]

It isn't safe to read the device ID in SOFTWARE_RESET register
until the silicon boot sequence has completed. This patch
rearranges the code to wait for BOOT_DONE first.

If we don't have a GPIO to control hard reset we must issue
a soft reset to get the silicon into a known-good booted state
before reading the device ID.

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 drivers/mfd/madera-core.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
index 3ff9a0615658..b9d42edf96c9 100644
--- a/drivers/mfd/madera-core.c
+++ b/drivers/mfd/madera-core.c
@@ -454,10 +454,23 @@ int madera_dev_init(struct madera *madera)
 	regcache_cache_only(madera->regmap, false);
 	regcache_cache_only(madera->regmap_32bit, false);
 
+	/* If we don't have a reset GPIO use a soft reset */
+	if (!madera->pdata.reset) {
+		ret = madera_soft_reset(madera);
+		if (ret)
+			goto err_reset;
+	}
+
 	/*
-	 * Now we can power up and verify that this is a chip we know about
-	 * before we start doing any writes to its registers.
+	 * Must wait for internal boot sequence to complete before
+	 * reading the device ID
 	 */
+	ret = madera_wait_for_boot(madera);
+	if (ret) {
+		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
+		goto err_reset;
+	}
+
 	ret = regmap_read(madera->regmap, MADERA_SOFTWARE_RESET, &hwid);
 	if (ret) {
 		dev_err(dev, "Failed to read ID register: %d\n", ret);
@@ -519,22 +532,6 @@ int madera_dev_init(struct madera *madera)
 		goto err_reset;
 	}
 
-	/*
-	 * It looks like a device we support. If we don't have a hard reset
-	 * we can now attempt a soft reset.
-	 */
-	if (!madera->pdata.reset) {
-		ret = madera_soft_reset(madera);
-		if (ret)
-			goto err_reset;
-	}
-
-	ret = madera_wait_for_boot(madera);
-	if (ret) {
-		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
-		goto err_reset;
-	}
-
 	ret = regmap_read(madera->regmap, MADERA_HARDWARE_REVISION,
 			  &madera->rev);
 	if (ret) {
-- 
2.11.0

Re: [PATCH 2/2] mfd: madera: Wait for BOOT_DONE before reading device ID

[Charles Keepax]

On Mon, Oct 01, 2018 at 03:33:57PM +0100, Richard Fitzgerald wrote:
> It isn't safe to read the device ID in SOFTWARE_RESET register
> until the silicon boot sequence has completed. This patch
> rearranges the code to wait for BOOT_DONE first.
> 
> If we don't have a GPIO to control hard reset we must issue
> a soft reset to get the silicon into a known-good booted state
> before reading the device ID.
> 
> Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  drivers/mfd/madera-core.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
> index 3ff9a0615658..b9d42edf96c9 100644
> --- a/drivers/mfd/madera-core.c
> +++ b/drivers/mfd/madera-core.c
> @@ -454,10 +454,23 @@ int madera_dev_init(struct madera *madera)
>  	regcache_cache_only(madera->regmap, false);
>  	regcache_cache_only(madera->regmap_32bit, false);
>  
> +	/* If we don't have a reset GPIO use a soft reset */
> +	if (!madera->pdata.reset) {
> +		ret = madera_soft_reset(madera);
> +		if (ret)
> +			goto err_reset;
> +	}
> +

I think we should leave this soft reset lower down its better
to only write to the chip once we have identified it. It will
mean we need to wait for boot done twice (in the soft reset
case) but its only on probe and that is probably better than
doing some writes to an unknown chip.

>  	/*
> -	 * Now we can power up and verify that this is a chip we know about
> -	 * before we start doing any writes to its registers.
> +	 * Must wait for internal boot sequence to complete before
> +	 * reading the device ID
>  	 */
> +	ret = madera_wait_for_boot(madera);
> +	if (ret) {
> +		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
> +		goto err_reset;
> +	}
> +
>  	ret = regmap_read(madera->regmap, MADERA_SOFTWARE_RESET, &hwid);
>  	if (ret) {
>  		dev_err(dev, "Failed to read ID register: %d\n", ret);
> @@ -519,22 +532,6 @@ int madera_dev_init(struct madera *madera)
>  		goto err_reset;
>  	}
>  
> -	/*
> -	 * It looks like a device we support. If we don't have a hard reset
> -	 * we can now attempt a soft reset.
> -	 */
> -	if (!madera->pdata.reset) {
> -		ret = madera_soft_reset(madera);
> -		if (ret)
> -			goto err_reset;
> -	}
> -
> -	ret = madera_wait_for_boot(madera);
> -	if (ret) {
> -		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
> -		goto err_reset;
> -	}

And this guy would then only need done in the case of a soft
reset.

Thanks,
Charles

> -
>  	ret = regmap_read(madera->regmap, MADERA_HARDWARE_REVISION,
>  			  &madera->rev);
>  	if (ret) {
> -- 
> 2.11.0