[PATCH v3] xen/rpi4: implement watchdog-based reset

[PATCH v3] xen/rpi4: implement watchdog-based reset

[Stefano Stabellini]

The preferred method to reboot RPi4 is PSCI. If it is not available,
touching the watchdog is required to be able to reboot the board.

The implementation is based on
drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Acked-by: Julien Grall <jgrall@amazon.com>
CC: roman@zededa.com
---
Changes in v3:
- fix typo in commit message
- dprintk -> printk
---
 xen/arch/arm/platforms/brcm-raspberry-pi.c | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c
index f5ae58a7d5..811b40b1a6 100644
--- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
+++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
@@ -17,6 +17,10 @@
  * GNU General Public License for more details.
  */
 
+#include <xen/delay.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <asm/io.h>
 #include <asm/platform.h>
 
 static const char *const rpi4_dt_compat[] __initconst =
@@ -37,12 +41,69 @@ static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
      * The aux peripheral also shares a page with the aux UART.
      */
     DT_MATCH_COMPATIBLE("brcm,bcm2835-aux"),
+    /* Special device used for rebooting */
+    DT_MATCH_COMPATIBLE("brcm,bcm2835-pm"),
     { /* sentinel */ },
 };
 
+
+#define PM_PASSWORD                 0x5a000000
+#define PM_RSTC                     0x1c
+#define PM_WDOG                     0x24
+#define PM_RSTC_WRCFG_FULL_RESET    0x00000020
+#define PM_RSTC_WRCFG_CLR           0xffffffcf
+
+static void __iomem *rpi4_map_watchdog(void)
+{
+    void __iomem *base;
+    struct dt_device_node *node;
+    paddr_t start, len;
+    int ret;
+
+    node = dt_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm");
+    if ( !node )
+        return NULL;
+
+    ret = dt_device_get_address(node, 0, &start, &len);
+    if ( ret )
+    {
+        printk("Cannot read watchdog register address\n");
+        return NULL;
+    }
+
+    base = ioremap_nocache(start & PAGE_MASK, PAGE_SIZE);
+    if ( !base )
+    {
+        printk("Unable to map watchdog register!\n");
+        return NULL;
+    }
+
+    return base;
+}
+
+static void rpi4_reset(void)
+{
+    uint32_t val;
+    void __iomem *base = rpi4_map_watchdog();
+
+    if ( !base )
+        return;
+
+    /* use a timeout of 10 ticks (~150us) */
+    writel(10 | PM_PASSWORD, base + PM_WDOG);
+    val = readl(base + PM_RSTC);
+    val &= PM_RSTC_WRCFG_CLR;
+    val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
+    writel(val, base + PM_RSTC);
+
+    /* No sleeping, possibly atomic. */
+    mdelay(1);
+}
+
 PLATFORM_START(rpi4, "Raspberry Pi 4")
     .compatible     = rpi4_dt_compat,
     .blacklist_dev  = rpi4_blacklist_dev,
+    .reset = rpi4_reset,
     .dma_bitsize    = 30,
 PLATFORM_END
 
-- 
2.17.1

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Bertrand Marquis]

> On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> The preferred method to reboot RPi4 is PSCI. If it is not available,
> touching the watchdog is required to be able to reboot the board.
> 
> The implementation is based on
> drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Acked-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Maybe a printk if reset was not successful ?

Cheers
Bertrand

> CC: roman@zededa.com
> ---
> Changes in v3:
> - fix typo in commit message
> - dprintk -> printk
> ---
> xen/arch/arm/platforms/brcm-raspberry-pi.c | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> 
> diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> index f5ae58a7d5..811b40b1a6 100644
> --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
> +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> @@ -17,6 +17,10 @@
>  * GNU General Public License for more details.
>  */
> 
> +#include <xen/delay.h>
> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <asm/io.h>
> #include <asm/platform.h>
> 
> static const char *const rpi4_dt_compat[] __initconst =
> @@ -37,12 +41,69 @@ static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
>      * The aux peripheral also shares a page with the aux UART.
>      */
>     DT_MATCH_COMPATIBLE("brcm,bcm2835-aux"),
> +    /* Special device used for rebooting */
> +    DT_MATCH_COMPATIBLE("brcm,bcm2835-pm"),
>     { /* sentinel */ },
> };
> 
> +
> +#define PM_PASSWORD                 0x5a000000
> +#define PM_RSTC                     0x1c
> +#define PM_WDOG                     0x24
> +#define PM_RSTC_WRCFG_FULL_RESET    0x00000020
> +#define PM_RSTC_WRCFG_CLR           0xffffffcf
> +
> +static void __iomem *rpi4_map_watchdog(void)
> +{
> +    void __iomem *base;
> +    struct dt_device_node *node;
> +    paddr_t start, len;
> +    int ret;
> +
> +    node = dt_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm");
> +    if ( !node )
> +        return NULL;
> +
> +    ret = dt_device_get_address(node, 0, &start, &len);
> +    if ( ret )
> +    {
> +        printk("Cannot read watchdog register address\n");
> +        return NULL;
> +    }
> +
> +    base = ioremap_nocache(start & PAGE_MASK, PAGE_SIZE);
> +    if ( !base )
> +    {
> +        printk("Unable to map watchdog register!\n");
> +        return NULL;
> +    }
> +
> +    return base;
> +}
> +
> +static void rpi4_reset(void)
> +{
> +    uint32_t val;
> +    void __iomem *base = rpi4_map_watchdog();
> +
> +    if ( !base )
> +        return;
> +
> +    /* use a timeout of 10 ticks (~150us) */
> +    writel(10 | PM_PASSWORD, base + PM_WDOG);
> +    val = readl(base + PM_RSTC);
> +    val &= PM_RSTC_WRCFG_CLR;
> +    val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> +    writel(val, base + PM_RSTC);
> +
> +    /* No sleeping, possibly atomic. */
> +    mdelay(1);
> +}
> +
> PLATFORM_START(rpi4, "Raspberry Pi 4")
>     .compatible     = rpi4_dt_compat,
>     .blacklist_dev  = rpi4_blacklist_dev,
> +    .reset = rpi4_reset,
>     .dma_bitsize    = 30,
> PLATFORM_END
> 
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Stefano Stabellini]

On Thu, 8 Oct 2020, Bertrand Marquis wrote:
> > On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > The preferred method to reboot RPi4 is PSCI. If it is not available,
> > touching the watchdog is required to be able to reboot the board.
> > 
> > The implementation is based on
> > drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > Acked-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> Maybe a printk if reset was not successful ?

That not quite platform specific but we could add a printk to
xen/arch/arm/shutdown.c:machine_restart if we are still alive after
100ms.

I'll commit this patch as is and maybe send another one for
machine_restart.


> > CC: roman@zededa.com
> > ---
> > Changes in v3:
> > - fix typo in commit message
> > - dprintk -> printk
> > ---
> > xen/arch/arm/platforms/brcm-raspberry-pi.c | 61 ++++++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> > 
> > diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> > index f5ae58a7d5..811b40b1a6 100644
> > --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c
> > +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c
> > @@ -17,6 +17,10 @@
> >  * GNU General Public License for more details.
> >  */
> > 
> > +#include <xen/delay.h>
> > +#include <xen/mm.h>
> > +#include <xen/vmap.h>
> > +#include <asm/io.h>
> > #include <asm/platform.h>
> > 
> > static const char *const rpi4_dt_compat[] __initconst =
> > @@ -37,12 +41,69 @@ static const struct dt_device_match rpi4_blacklist_dev[] __initconst =
> >      * The aux peripheral also shares a page with the aux UART.
> >      */
> >     DT_MATCH_COMPATIBLE("brcm,bcm2835-aux"),
> > +    /* Special device used for rebooting */
> > +    DT_MATCH_COMPATIBLE("brcm,bcm2835-pm"),
> >     { /* sentinel */ },
> > };
> > 
> > +
> > +#define PM_PASSWORD                 0x5a000000
> > +#define PM_RSTC                     0x1c
> > +#define PM_WDOG                     0x24
> > +#define PM_RSTC_WRCFG_FULL_RESET    0x00000020
> > +#define PM_RSTC_WRCFG_CLR           0xffffffcf
> > +
> > +static void __iomem *rpi4_map_watchdog(void)
> > +{
> > +    void __iomem *base;
> > +    struct dt_device_node *node;
> > +    paddr_t start, len;
> > +    int ret;
> > +
> > +    node = dt_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm");
> > +    if ( !node )
> > +        return NULL;
> > +
> > +    ret = dt_device_get_address(node, 0, &start, &len);
> > +    if ( ret )
> > +    {
> > +        printk("Cannot read watchdog register address\n");
> > +        return NULL;
> > +    }
> > +
> > +    base = ioremap_nocache(start & PAGE_MASK, PAGE_SIZE);
> > +    if ( !base )
> > +    {
> > +        printk("Unable to map watchdog register!\n");
> > +        return NULL;
> > +    }
> > +
> > +    return base;
> > +}
> > +
> > +static void rpi4_reset(void)
> > +{
> > +    uint32_t val;
> > +    void __iomem *base = rpi4_map_watchdog();
> > +
> > +    if ( !base )
> > +        return;
> > +
> > +    /* use a timeout of 10 ticks (~150us) */
> > +    writel(10 | PM_PASSWORD, base + PM_WDOG);
> > +    val = readl(base + PM_RSTC);
> > +    val &= PM_RSTC_WRCFG_CLR;
> > +    val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
> > +    writel(val, base + PM_RSTC);
> > +
> > +    /* No sleeping, possibly atomic. */
> > +    mdelay(1);
> > +}
> > +
> > PLATFORM_START(rpi4, "Raspberry Pi 4")
> >     .compatible     = rpi4_dt_compat,
> >     .blacklist_dev  = rpi4_blacklist_dev,
> > +    .reset = rpi4_reset,
> >     .dma_bitsize    = 30,
> > PLATFORM_END
> > 
> > -- 
> > 2.17.1
> > 
> > 
> 

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Roman Shaposhnik]

On Thu, Oct 8, 2020 at 12:58 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> > On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
> >
> > The preferred method to reboot RPi4 is PSCI. If it is not available,
> > touching the watchdog is required to be able to reboot the board.
> >
> > The implementation is based on
> > drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > Acked-by: Julien Grall <jgrall@amazon.com>
>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

FWIW:

Tested-by: Roman Shaposhnik <roman@zededa.com>

Great to see it being merged -- one less custom patch for us to carry in EVE.

Thanks,
Roman.

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Bertrand Marquis]

> On 8 Oct 2020, at 19:27, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 8 Oct 2020, Bertrand Marquis wrote:
>>> On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> 
>>> The preferred method to reboot RPi4 is PSCI. If it is not available,
>>> touching the watchdog is required to be able to reboot the board.
>>> 
>>> The implementation is based on
>>> drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
>>> 
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> Acked-by: Julien Grall <jgrall@amazon.com>
>> 
>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> 
>> Maybe a printk if reset was not successful ?
> 
> That not quite platform specific but we could add a printk to
> xen/arch/arm/shutdown.c:machine_restart if we are still alive after
> 100ms.

Even nicer :-)
Definitely usefull to see something if for one reason reset/restart did
not succeed for whatever reason.

> 
> I'll commit this patch as is and maybe send another one for
> machine_restart.

Please tell me if you want me to handle that one (at the end I did request
that so not really fare to ask you to do it:-) ).

Cheers
Bertrand

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Stefano Stabellini]

On Fri, 9 Oct 2020, Bertrand Marquis wrote:
> > On 8 Oct 2020, at 19:27, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > On Thu, 8 Oct 2020, Bertrand Marquis wrote:
> >>> On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
> >>> 
> >>> The preferred method to reboot RPi4 is PSCI. If it is not available,
> >>> touching the watchdog is required to be able to reboot the board.
> >>> 
> >>> The implementation is based on
> >>> drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
> >>> 
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >>> Acked-by: Julien Grall <jgrall@amazon.com>
> >> 
> >> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >> 
> >> Maybe a printk if reset was not successful ?
> > 
> > That not quite platform specific but we could add a printk to
> > xen/arch/arm/shutdown.c:machine_restart if we are still alive after
> > 100ms.
> 
> Even nicer :-)
> Definitely usefull to see something if for one reason reset/restart did
> not succeed for whatever reason.
> 
> > 
> > I'll commit this patch as is and maybe send another one for
> > machine_restart.
> 
> Please tell me if you want me to handle that one (at the end I did request
> that so not really fare to ask you to do it:-) ).

Since you are volunteering, yes please :-)

Re: [PATCH v3] xen/rpi4: implement watchdog-based reset

[Bertrand Marquis]

Hi Stefano,

> On 9 Oct 2020, at 17:33, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Fri, 9 Oct 2020, Bertrand Marquis wrote:
>>> On 8 Oct 2020, at 19:27, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> 
>>> On Thu, 8 Oct 2020, Bertrand Marquis wrote:
>>>>> On 7 Oct 2020, at 23:38, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>>>> 
>>>>> The preferred method to reboot RPi4 is PSCI. If it is not available,
>>>>> touching the watchdog is required to be able to reboot the board.
>>>>> 
>>>>> The implementation is based on
>>>>> drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux v5.9-rc7.
>>>>> 
>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>>> Acked-by: Julien Grall <jgrall@amazon.com>
>>>> 
>>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> 
>>>> Maybe a printk if reset was not successful ?
>>> 
>>> That not quite platform specific but we could add a printk to
>>> xen/arch/arm/shutdown.c:machine_restart if we are still alive after
>>> 100ms.
>> 
>> Even nicer :-)
>> Definitely usefull to see something if for one reason reset/restart did
>> not succeed for whatever reason.
>> 
>>> 
>>> I'll commit this patch as is and maybe send another one for
>>> machine_restart.
>> 
>> Please tell me if you want me to handle that one (at the end I did request
>> that so not really fare to ask you to do it:-) ).
> 
> Since you are volunteering, yes please :-)
Fare enough

I will add this to my small fixes list :-)

Cheers
Bertrand