Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/4] add dual-boot support
@ 2019-08-26 10:46 Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 1/4] vesnin: add wdt2 section with alt-boot option Ivan Mikhaylov
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-26 10:46 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree, Ivan Mikhaylov

ASPEED SoCs support dual-boot feature for SPI Flash.
When strapped appropriately, the SoC starts wdt2 (/dev/watchdog1)
and if within a minute it is not disabled, it goes off and reboots
the SoC from an alternate SPI Flash chip by changing CS0 controls
to actually drive CS1 line.

When booted from alternate chip, in order to access the main chip
at CS0, the user must reset the appropriate bit in the watchdog
hardware. There is no interface that would allow to do that from
an embedded firmware startup script.

This commit implements support for that feature:

* Enable 'alt-boot' option for wdt2

* Enable secondary SPI flash chip

* Make it possible to get access to the primary SPI flash chip at CS0
  after booting from the alternate chip at CS1. A sysfs interface is added
  to provide an easy way for embedded firmware startup scripts to clear
  the chip select bit to gain access to the primary flash chip in order
  to allow for recovery of its contents.

Ivan Mikhaylov (4):
  vesnin: add wdt2 section with alt-boot option
  vesnin: add secondary SPI flash chip
  watchdog/aspeed: add support for dual boot
  dt-bindings/watchdog: Add access_cs0 option for alt-boot

 .../bindings/watchdog/aspeed-wdt.txt          |  7 +++
 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts   | 12 ++++
 drivers/watchdog/aspeed_wdt.c                 | 62 ++++++++++++++++++-
 3 files changed, 80 insertions(+), 1 deletion(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] vesnin: add wdt2 section with alt-boot option
  2019-08-26 10:46 [PATCH v2 0/4] add dual-boot support Ivan Mikhaylov
@ 2019-08-26 10:46 ` Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 2/4] vesnin: add secondary SPI flash chip Ivan Mikhaylov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-26 10:46 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree, Ivan Mikhaylov

Adds wdt2 section with 'alt-boot' option into dts for vesnin.

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
index 0b9e29c3212e..2ee26c86a32e 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -222,3 +222,7 @@
 &vuart {
 	status = "okay";
 };
+
+&wdt2 {
+	aspeed,alt-boot;
+};
-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] vesnin: add secondary SPI flash chip
  2019-08-26 10:46 [PATCH v2 0/4] add dual-boot support Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 1/4] vesnin: add wdt2 section with alt-boot option Ivan Mikhaylov
@ 2019-08-26 10:46 ` Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 3/4] watchdog/aspeed: add support for dual boot Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot Ivan Mikhaylov
  3 siblings, 0 replies; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-26 10:46 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree, Ivan Mikhaylov

Adds secondary SPI flash chip into dts for vesnin.

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
index 2ee26c86a32e..db4cc3df61ce 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -81,6 +81,14 @@
         label = "bmc";
 #include "openbmc-flash-layout.dtsi"
 	};
+
+	flash@1 {
+		status = "okay";
+		reg = < 1 >;
+		compatible = "jedec,spi-nor";
+		m25p,fast-read;
+		label = "alt";
+	};
 };
 
 &spi {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] watchdog/aspeed: add support for dual boot
  2019-08-26 10:46 [PATCH v2 0/4] add dual-boot support Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 1/4] vesnin: add wdt2 section with alt-boot option Ivan Mikhaylov
  2019-08-26 10:46 ` [PATCH v2 2/4] vesnin: add secondary SPI flash chip Ivan Mikhaylov
@ 2019-08-26 10:46 ` Ivan Mikhaylov
  2019-08-27  0:14   ` Guenter Roeck
  2019-08-26 10:46 ` [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot Ivan Mikhaylov
  3 siblings, 1 reply; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-26 10:46 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree, Ivan Mikhaylov

Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
to clear out boot code source and re-enable access to the primary SPI flash
chip while booted via wdt2 from the alternate chip.

AST2400 datasheet says:
"In the 2nd flash booting mode, all the address mapping to CS0# would be
re-directed to CS1#. And CS0# is not accessable under this mode. To access
CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
register WDT30.bit[1]."

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 drivers/watchdog/aspeed_wdt.c | 62 ++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index cc71861e033a..bbc42847c0e3 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -53,6 +53,8 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
 #define   WDT_CTRL_ENABLE		BIT(0)
 #define WDT_TIMEOUT_STATUS	0x10
 #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY	BIT(1)
+#define WDT_CLEAR_TIMEOUT_STATUS	0x14
+#define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION	BIT(0)
 
 /*
  * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
@@ -165,6 +167,57 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
 	return 0;
 }
 
+/* access_cs0 shows if cs0 is accessible, hence the reverted bit */
+static ssize_t access_cs0_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
+	uint32_t status = readl(wdt->base + WDT_TIMEOUT_STATUS);
+
+	return sprintf(buf, "%u\n",
+			!(status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY));
+}
+
+static ssize_t access_cs0_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t size)
+{
+	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
+	unsigned long val;
+
+	if (kstrtoul(buf, 10, &val))
+		return -EINVAL;
+
+	if (val)
+		writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
+			wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
+
+	return size;
+}
+
+/*
+ * At alternate side the 'access_cs0' sysfs node provides:
+ *   ast2400: a way to get access to the primary SPI flash chip at CS0
+ *            after booting from the alternate chip at CS1.
+ *   ast2500: a way to restore the normal address mapping from
+ *            (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
+ *
+ * Clearing the boot code selection and timeout counter also resets to the
+ * initial state the chip select line mapping. When the SoC is in normal
+ * mapping state (i.e. booted from CS0), clearing those bits does nothing for
+ * both versions of the SoC. For alternate boot mode (booted from CS1 due to
+ * wdt2 expiration) the behavior differs as described above.
+ *
+ * This option can be used with wdt2 (watchdog1) only.
+ */
+static DEVICE_ATTR_RW(access_cs0);
+
+static struct attribute *bswitch_attrs[] = {
+	&dev_attr_access_cs0.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(bswitch);
+
 static const struct watchdog_ops aspeed_wdt_ops = {
 	.start		= aspeed_wdt_start,
 	.stop		= aspeed_wdt_stop,
@@ -306,9 +359,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 	}
 
 	status = readl(wdt->base + WDT_TIMEOUT_STATUS);
-	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
+	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
 		wdt->wdd.bootstatus = WDIOF_CARDRESET;
 
+		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
+			of_device_is_compatible(np, "aspeed,ast2500-wdt"))
+			wdt->wdd.groups = bswitch_groups;
+	}
+
+	dev_set_drvdata(dev, wdt);
+
 	return devm_watchdog_register_device(dev, &wdt->wdd);
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
  2019-08-26 10:46 [PATCH v2 0/4] add dual-boot support Ivan Mikhaylov
                   ` (2 preceding siblings ...)
  2019-08-26 10:46 ` [PATCH v2 3/4] watchdog/aspeed: add support for dual boot Ivan Mikhaylov
@ 2019-08-26 10:46 ` Ivan Mikhaylov
  2019-08-26 23:57   ` Andrew Jeffery
  2019-08-29 22:56   ` Rob Herring
  3 siblings, 2 replies; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-26 10:46 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree, Ivan Mikhaylov

The option for the ast2400/2500 to get access to CS0 at runtime.

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
index c5077a1f5cb3..023a9b578df6 100644
--- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
@@ -34,6 +34,13 @@ Optional properties:
                 engine is responsible for this.
 
  - aspeed,alt-boot:    If property is present then boot from alternate block.
+                       At alternate side 'access_cs0' sysfs file provides:
+                       ast2400: a way to get access to the primary SPI flash
+                                chip at CS0 after booting from the alternate
+                                chip at CS1.
+                       ast2500: a way to restore the normal address mapping from
+                                (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
+
  - aspeed,external-signal: If property is present then signal is sent to
 			external reset counter (only WDT1 and WDT2). If not
 			specified no external signal is sent.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
  2019-08-26 10:46 ` [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot Ivan Mikhaylov
@ 2019-08-26 23:57   ` Andrew Jeffery
  2019-08-27  0:08     ` Guenter Roeck
  2019-08-29 22:56   ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Andrew Jeffery @ 2019-08-26 23:57 UTC (permalink / raw)
  To: Ivan Mikhaylov, Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, linux-watchdog, linux-arm-kernel, linux-aspeed,
	linux-kernel, Alexander Amelkin, openbmc, Rob Herring,
	Mark Rutland, devicetree



On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
> The option for the ast2400/2500 to get access to CS0 at runtime.
> 
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
>  Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt 
> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> index c5077a1f5cb3..023a9b578df6 100644
> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> @@ -34,6 +34,13 @@ Optional properties:
>                  engine is responsible for this.
>  
>   - aspeed,alt-boot:    If property is present then boot from alternate 
> block.
> +                       At alternate side 'access_cs0' sysfs file 
> provides:

Why are we talking about sysfs in the devicetree binding? This patch
doesn't seem right to me.

Also if we're not supporting the aspeed,alt-boot property we should
probably document it as deprecated rather than making it disappear,
unless you're going to fix the systems that are using it.

Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
  2019-08-26 23:57   ` Andrew Jeffery
@ 2019-08-27  0:08     ` Guenter Roeck
  2019-08-27  0:11       ` Andrew Jeffery
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2019-08-27  0:08 UTC (permalink / raw)
  To: Andrew Jeffery, Ivan Mikhaylov, Wim Van Sebroeck
  Cc: Joel Stanley, linux-watchdog, linux-arm-kernel, linux-aspeed,
	linux-kernel, Alexander Amelkin, openbmc, Rob Herring,
	Mark Rutland, devicetree

On 8/26/19 4:57 PM, Andrew Jeffery wrote:
> 
> 
> On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
>> The option for the ast2400/2500 to get access to CS0 at runtime.
>>
>> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
>> ---
>>   Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> index c5077a1f5cb3..023a9b578df6 100644
>> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> @@ -34,6 +34,13 @@ Optional properties:
>>                   engine is responsible for this.
>>   
>>    - aspeed,alt-boot:    If property is present then boot from alternate
>> block.
>> +                       At alternate side 'access_cs0' sysfs file
>> provides:
> 
> Why are we talking about sysfs in the devicetree binding? This patch
> doesn't seem right to me.
> 

Correct; this is the wrong document. The attribute also will need
to be better explained. "At alternate side" does not explain (at
least not at all clearly enough) that the attribute only exists
if the system has booted from the alternate flash / block.

> Also if we're not supporting the aspeed,alt-boot property we should
> probably document it as deprecated rather than making it disappear,
> unless you're going to fix the systems that are using it.
> 
Sorry, you lost me here. Where is it made to disappear ?

Guenter

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
  2019-08-27  0:08     ` Guenter Roeck
@ 2019-08-27  0:11       ` Andrew Jeffery
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jeffery @ 2019-08-27  0:11 UTC (permalink / raw)
  To: Guenter Roeck, Ivan Mikhaylov, Wim Van Sebroeck
  Cc: Joel Stanley, linux-watchdog, linux-arm-kernel, linux-aspeed,
	linux-kernel, Alexander Amelkin, openbmc, Rob Herring,
	Mark Rutland, devicetree



On Tue, 27 Aug 2019, at 09:38, Guenter Roeck wrote:
> On 8/26/19 4:57 PM, Andrew Jeffery wrote:
> > 
> > 
> > On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
> >> The option for the ast2400/2500 to get access to CS0 at runtime.
> >>
> >> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> >> ---
> >>   Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> index c5077a1f5cb3..023a9b578df6 100644
> >> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> @@ -34,6 +34,13 @@ Optional properties:
> >>                   engine is responsible for this.
> >>   
> >>    - aspeed,alt-boot:    If property is present then boot from alternate
> >> block.
> >> +                       At alternate side 'access_cs0' sysfs file
> >> provides:
> > 
> > Why are we talking about sysfs in the devicetree binding? This patch
> > doesn't seem right to me.
> > 
> 
> Correct; this is the wrong document. The attribute also will need
> to be better explained. "At alternate side" does not explain (at
> least not at all clearly enough) that the attribute only exists
> if the system has booted from the alternate flash / block.
> 
> > Also if we're not supporting the aspeed,alt-boot property we should
> > probably document it as deprecated rather than making it disappear,
> > unless you're going to fix the systems that are using it.
> > 
> Sorry, you lost me here. Where is it made to disappear ?

Oh, hah, I read the bullet '-' as a diff marker. Maybe I should go back to
bed!

Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] watchdog/aspeed: add support for dual boot
  2019-08-26 10:46 ` [PATCH v2 3/4] watchdog/aspeed: add support for dual boot Ivan Mikhaylov
@ 2019-08-27  0:14   ` Guenter Roeck
  2019-08-27  9:24     ` Ivan Mikhaylov
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2019-08-27  0:14 UTC (permalink / raw)
  To: Ivan Mikhaylov, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree

On 8/26/19 3:46 AM, Ivan Mikhaylov wrote:
> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> to clear out boot code source and re-enable access to the primary SPI flash
> chip while booted via wdt2 from the alternate chip.
> 
> AST2400 datasheet says:
> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> register WDT30.bit[1]."
> 
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
>   drivers/watchdog/aspeed_wdt.c | 62 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index cc71861e033a..bbc42847c0e3 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -53,6 +53,8 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
>   #define   WDT_CTRL_ENABLE		BIT(0)
>   #define WDT_TIMEOUT_STATUS	0x10
>   #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY	BIT(1)
> +#define WDT_CLEAR_TIMEOUT_STATUS	0x14
> +#define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION	BIT(0)
>   
>   /*
>    * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
> @@ -165,6 +167,57 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
>   	return 0;
>   }
>   
> +/* access_cs0 shows if cs0 is accessible, hence the reverted bit */
> +static ssize_t access_cs0_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)

This and other multi-line declarations do not appear to be aligned
with '('.

> +{
> +	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
> +	uint32_t status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> +
> +	return sprintf(buf, "%u\n",
> +			!(status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY));
> +}
> +
> +static ssize_t access_cs0_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t size)
> +{
> +	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
> +	unsigned long val;
> +
> +	if (kstrtoul(buf, 10, &val))
> +		return -EINVAL;
> +
> +	if (val)
> +		writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> +			wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> +
> +	return size;
> +}
> +
> +/*
> + * At alternate side the 'access_cs0' sysfs node provides:
> + *   ast2400: a way to get access to the primary SPI flash chip at CS0
> + *            after booting from the alternate chip at CS1.
> + *   ast2500: a way to restore the normal address mapping from
> + *            (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
> + *
> + * Clearing the boot code selection and timeout counter also resets to the
> + * initial state the chip select line mapping. When the SoC is in normal
> + * mapping state (i.e. booted from CS0), clearing those bits does nothing for
> + * both versions of the SoC. For alternate boot mode (booted from CS1 due to
> + * wdt2 expiration) the behavior differs as described above.
> + *
The above needs to be in the sysfs attribute documentation as well.

> + * This option can be used with wdt2 (watchdog1) only.

This implies a specific watchdog numbering which is not guaranteed.
Someone might implement a system with some external watchdog.

> + */
> +static DEVICE_ATTR_RW(access_cs0);
> +
> +static struct attribute *bswitch_attrs[] = {
> +	&dev_attr_access_cs0.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(bswitch);
> +
>   static const struct watchdog_ops aspeed_wdt_ops = {
>   	.start		= aspeed_wdt_start,
>   	.stop		= aspeed_wdt_stop,
> @@ -306,9 +359,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>   	}
>   
>   	status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> -	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
> +	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
>   		wdt->wdd.bootstatus = WDIOF_CARDRESET;
>   
> +		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
> +			of_device_is_compatible(np, "aspeed,ast2500-wdt"))
> +			wdt->wdd.groups = bswitch_groups;

Kind of odd that the attribute only exists if the system booted from the
second flash, but if that is what you want I won't object. Just make sure
that this is explained properly.

> +	}
> +
> +	dev_set_drvdata(dev, wdt);
> +
>   	return devm_watchdog_register_device(dev, &wdt->wdd);
>   }
>   
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] watchdog/aspeed: add support for dual boot
  2019-08-27  0:14   ` Guenter Roeck
@ 2019-08-27  9:24     ` Ivan Mikhaylov
  2019-08-27 13:06       ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: Ivan Mikhaylov @ 2019-08-27  9:24 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree

On Mon, 2019-08-26 at 17:14 -0700, Guenter Roeck wrote:
> > +/*
> > + * At alternate side the 'access_cs0' sysfs node provides:
> > + *   ast2400: a way to get access to the primary SPI flash chip at CS0
> > + *            after booting from the alternate chip at CS1.
> > + *   ast2500: a way to restore the normal address mapping from
> > + *            (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
> > + *
> > + * Clearing the boot code selection and timeout counter also resets to the
> > + * initial state the chip select line mapping. When the SoC is in normal
> > + * mapping state (i.e. booted from CS0), clearing those bits does nothing
> > for
> > + * both versions of the SoC. For alternate boot mode (booted from CS1 due
> > to
> > + * wdt2 expiration) the behavior differs as described above.
> > + *
> The above needs to be in the sysfs attribute documentation as well.

My apologies but I didn't find any suitable, only watchdog parameters with
dtbindings file, where should I put it? Documentation/watchdog/aspeed-wdt-
sysfs.rst?

> > + * This option can be used with wdt2 (watchdog1) only.
> 
> This implies a specific watchdog numbering which is not guaranteed.
> Someone might implement a system with some external watchdog.
> 
> > + */
> > +static DEVICE_ATTR_RW(access_cs0);
> > +
> > +static struct attribute *bswitch_attrs[] = {
> > +	&dev_attr_access_cs0.attr,
> > +	NULL
> > +};
> > +ATTRIBUTE_GROUPS(bswitch);
> > +
> >   static const struct watchdog_ops aspeed_wdt_ops = {
> >   	.start		= aspeed_wdt_start,
> >   	.stop		= aspeed_wdt_stop,
> > @@ -306,9 +359,16 @@ static int aspeed_wdt_probe(struct platform_device
> > *pdev)
> >   	}
> >   
> >   	status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> > -	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
> > +	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
> >   		wdt->wdd.bootstatus = WDIOF_CARDRESET;
> >   
> > +		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
> > +			of_device_is_compatible(np, "aspeed,ast2500-wdt"))
> > +			wdt->wdd.groups = bswitch_groups;

> Kind of odd that the attribute only exists if the system booted from the
> second flash, but if that is what you want I won't object. Just make sure
> that this is explained properly.
Perhaps dts configuration option would be better solution for it then? "force-
cs0-switch" as example? Also, if it would be an option, dtbindings/wdt file for
documentation will be the right place for it. Usage of this at side 0 will not
get any good/bad results, it just makes user confused, so I decided to put it
only at side 1. It works only for ast2400/2500 board unfortunately, for 2600
there is big difference in switching mechanism. Any other thoughts how to make
it better?

Thanks.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] watchdog/aspeed: add support for dual boot
  2019-08-27  9:24     ` Ivan Mikhaylov
@ 2019-08-27 13:06       ` Guenter Roeck
  0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2019-08-27 13:06 UTC (permalink / raw)
  To: Ivan Mikhaylov, Wim Van Sebroeck
  Cc: Joel Stanley, Andrew Jeffery, linux-watchdog, linux-arm-kernel,
	linux-aspeed, linux-kernel, Alexander Amelkin, openbmc,
	Rob Herring, Mark Rutland, devicetree

On 8/27/19 2:24 AM, Ivan Mikhaylov wrote:
> On Mon, 2019-08-26 at 17:14 -0700, Guenter Roeck wrote:
>>> +/*
>>> + * At alternate side the 'access_cs0' sysfs node provides:
>>> + *   ast2400: a way to get access to the primary SPI flash chip at CS0
>>> + *            after booting from the alternate chip at CS1.
>>> + *   ast2500: a way to restore the normal address mapping from
>>> + *            (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
>>> + *
>>> + * Clearing the boot code selection and timeout counter also resets to the
>>> + * initial state the chip select line mapping. When the SoC is in normal
>>> + * mapping state (i.e. booted from CS0), clearing those bits does nothing
>>> for
>>> + * both versions of the SoC. For alternate boot mode (booted from CS1 due
>>> to
>>> + * wdt2 expiration) the behavior differs as described above.
>>> + *
>> The above needs to be in the sysfs attribute documentation as well.
> 
> My apologies but I didn't find any suitable, only watchdog parameters with
> dtbindings file, where should I put it? Documentation/watchdog/aspeed-wdt-
> sysfs.rst?
> 

Documentation/ABI/testing/sysfs-class-watchdog

Guenter

>>> + * This option can be used with wdt2 (watchdog1) only.
>>
>> This implies a specific watchdog numbering which is not guaranteed.
>> Someone might implement a system with some external watchdog.
>>
>>> + */
>>> +static DEVICE_ATTR_RW(access_cs0);
>>> +
>>> +static struct attribute *bswitch_attrs[] = {
>>> +	&dev_attr_access_cs0.attr,
>>> +	NULL
>>> +};
>>> +ATTRIBUTE_GROUPS(bswitch);
>>> +
>>>    static const struct watchdog_ops aspeed_wdt_ops = {
>>>    	.start		= aspeed_wdt_start,
>>>    	.stop		= aspeed_wdt_stop,
>>> @@ -306,9 +359,16 @@ static int aspeed_wdt_probe(struct platform_device
>>> *pdev)
>>>    	}
>>>    
>>>    	status = readl(wdt->base + WDT_TIMEOUT_STATUS);
>>> -	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
>>> +	if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
>>>    		wdt->wdd.bootstatus = WDIOF_CARDRESET;
>>>    
>>> +		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
>>> +			of_device_is_compatible(np, "aspeed,ast2500-wdt"))
>>> +			wdt->wdd.groups = bswitch_groups;
> 
>> Kind of odd that the attribute only exists if the system booted from the
>> second flash, but if that is what you want I won't object. Just make sure
>> that this is explained properly.
> Perhaps dts configuration option would be better solution for it then? "force-
> cs0-switch" as example? Also, if it would be an option, dtbindings/wdt file for

You said earlier that this can not be done automatically but _must_ be done
from user space after the system has booted. Otherwise you could just
automatically switch to cs0 when the driver probes.

As I said, all I am asking for is proper documentation.

Guenter

> documentation will be the right place for it. Usage of this at side 0 will not
> get any good/bad results, it just makes user confused, so I decided to put it
> only at side 1. It works only for ast2400/2500 board unfortunately, for 2600
> there is big difference in switching mechanism. Any other thoughts how to make
> it better?
> 
> Thanks.
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
  2019-08-26 10:46 ` [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot Ivan Mikhaylov
  2019-08-26 23:57   ` Andrew Jeffery
@ 2019-08-29 22:56   ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2019-08-29 22:56 UTC (permalink / raw)
  To: Ivan Mikhaylov
  Cc: Guenter Roeck, Wim Van Sebroeck, Joel Stanley, Andrew Jeffery,
	linux-watchdog, linux-arm-kernel, linux-aspeed, linux-kernel,
	Alexander Amelkin, openbmc, Mark Rutland, devicetree

On Mon, Aug 26, 2019 at 01:46:36PM +0300, Ivan Mikhaylov wrote:
> The option for the ast2400/2500 to get access to CS0 at runtime.
> 
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
>  Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> index c5077a1f5cb3..023a9b578df6 100644
> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> @@ -34,6 +34,13 @@ Optional properties:
>                  engine is responsible for this.
>  
>   - aspeed,alt-boot:    If property is present then boot from alternate block.
> +                       At alternate side 'access_cs0' sysfs file provides:

What's sysfs?

Don't put Linux stuff in bindings.

> +                       ast2400: a way to get access to the primary SPI flash
> +                                chip at CS0 after booting from the alternate
> +                                chip at CS1.
> +                       ast2500: a way to restore the normal address mapping from
> +                                (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
> +
>   - aspeed,external-signal: If property is present then signal is sent to
>  			external reset counter (only WDT1 and WDT2). If not
>  			specified no external signal is sent.
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 10:46 [PATCH v2 0/4] add dual-boot support Ivan Mikhaylov
2019-08-26 10:46 ` [PATCH v2 1/4] vesnin: add wdt2 section with alt-boot option Ivan Mikhaylov
2019-08-26 10:46 ` [PATCH v2 2/4] vesnin: add secondary SPI flash chip Ivan Mikhaylov
2019-08-26 10:46 ` [PATCH v2 3/4] watchdog/aspeed: add support for dual boot Ivan Mikhaylov
2019-08-27  0:14   ` Guenter Roeck
2019-08-27  9:24     ` Ivan Mikhaylov
2019-08-27 13:06       ` Guenter Roeck
2019-08-26 10:46 ` [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot Ivan Mikhaylov
2019-08-26 23:57   ` Andrew Jeffery
2019-08-27  0:08     ` Guenter Roeck
2019-08-27  0:11       ` Andrew Jeffery
2019-08-29 22:56   ` Rob Herring

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox