| Message ID | 1434195541-28368-2-git-send-email-noralf@tronnes.org |
|---|---|
| State | New |
| Headers | show |
On 06/13/2015 05:39 AM, Noralf Trønnes wrote: > This adds a new poweroff function to the watchdog driver for the > Raspberry Pi. Currently poweroff/halt results in a reboot. > > The Raspberry Pi firmware uses the RSTS register to know which > partiton to boot from. The partiton value is spread into bits > 0, 2, 4, 6, 8, 10. Partiton 63 is a special partition used by > the firmware to indicate halt. > > The firmware made this change in 19 Aug 2013 and was matched > by the downstream commit: > Changes for new NOOBS multi partition booting from gsh I don't understand why we need a new compatible value here; why not simply modify the existing bcm2835_power_off() function. That is written to do something that's interpreted by the RPi firmware, not something that the bcm2835 HW does. Admittedly the current name is a bit misleading, but fixing that should be a separate change to fixing the implementation to do what the current firmware expects.
Den 16.06.2015 05:07, skrev Stephen Warren: > On 06/13/2015 05:39 AM, Noralf Trønnes wrote: >> This adds a new poweroff function to the watchdog driver for the >> Raspberry Pi. Currently poweroff/halt results in a reboot. >> >> The Raspberry Pi firmware uses the RSTS register to know which >> partiton to boot from. The partiton value is spread into bits >> 0, 2, 4, 6, 8, 10. Partiton 63 is a special partition used by >> the firmware to indicate halt. >> >> The firmware made this change in 19 Aug 2013 and was matched >> by the downstream commit: >> Changes for new NOOBS multi partition booting from gsh > I don't understand why we need a new compatible value here; why not > simply modify the existing bcm2835_power_off() function. That is written > to do something that's interpreted by the RPi firmware, not something > that the bcm2835 HW does. > > Admittedly the current name is a bit misleading, but fixing that should > be a separate change to fixing the implementation to do what the current > firmware expects. There are other boards that use the BCM2835 and I didn't want to break the behaviour for those that use the reference firmware. Roku 2 device uses this soc, and changing bcm2835_power_off() would break support for it. ODROID-W also use BCM2835, but this is a Pi clone so I don't know if they have matched their firmware behaviour to that of the Pi (admittedly not many boards were made, their source of chips went dry).
On 06/16/2015 03:39 AM, Noralf Trønnes wrote: > > Den 16.06.2015 05:07, skrev Stephen Warren: >> On 06/13/2015 05:39 AM, Noralf Trønnes wrote: >>> This adds a new poweroff function to the watchdog driver for the >>> Raspberry Pi. Currently poweroff/halt results in a reboot. >>> >>> The Raspberry Pi firmware uses the RSTS register to know which >>> partiton to boot from. The partiton value is spread into bits >>> 0, 2, 4, 6, 8, 10. Partiton 63 is a special partition used by >>> the firmware to indicate halt. >>> >>> The firmware made this change in 19 Aug 2013 and was matched >>> by the downstream commit: >>> Changes for new NOOBS multi partition booting from gsh >> I don't understand why we need a new compatible value here; why not >> simply modify the existing bcm2835_power_off() function. That is written >> to do something that's interpreted by the RPi firmware, not something >> that the bcm2835 HW does. >> >> Admittedly the current name is a bit misleading, but fixing that should >> be a separate change to fixing the implementation to do what the current >> firmware expects. > > There are other boards that use the BCM2835 and I didn't want to break the > behaviour for those that use the reference firmware. We don't support those other board in mainline Linux AFAIK. In other discussions, Eric Anholt stated that the Roku 2 for example doesn't use the same firmware (albeit they were derived from the same base a long way back apparently) so I have no good reason to believe this logic is a standard across difference bcm2835 devices. Do you know more specific details? > Roku 2 device uses > this soc, and changing bcm2835_power_off() would break support for it. > ODROID-W also use BCM2835, but this is a Pi clone so I don't know if they > have matched their firmware behaviour to that of the Pi (admittedly not > many boards were made, their source of chips went dry).
Den 17.06.2015 03:28, skrev Stephen Warren: > On 06/16/2015 03:39 AM, Noralf Trønnes wrote: >> Den 16.06.2015 05:07, skrev Stephen Warren: >>> On 06/13/2015 05:39 AM, Noralf Trønnes wrote: >>>> This adds a new poweroff function to the watchdog driver for the >>>> Raspberry Pi. Currently poweroff/halt results in a reboot. >>>> >>>> The Raspberry Pi firmware uses the RSTS register to know which >>>> partiton to boot from. The partiton value is spread into bits >>>> 0, 2, 4, 6, 8, 10. Partiton 63 is a special partition used by >>>> the firmware to indicate halt. >>>> >>>> The firmware made this change in 19 Aug 2013 and was matched >>>> by the downstream commit: >>>> Changes for new NOOBS multi partition booting from gsh >>> I don't understand why we need a new compatible value here; why not >>> simply modify the existing bcm2835_power_off() function. That is written >>> to do something that's interpreted by the RPi firmware, not something >>> that the bcm2835 HW does. >>> >>> Admittedly the current name is a bit misleading, but fixing that should >>> be a separate change to fixing the implementation to do what the current >>> firmware expects. >> There are other boards that use the BCM2835 and I didn't want to break the >> behaviour for those that use the reference firmware. > We don't support those other board in mainline Linux AFAIK. In other > discussions, Eric Anholt stated that the Roku 2 for example doesn't use > the same firmware (albeit they were derived from the same base a long > way back apparently) so I have no good reason to believe this logic is a > standard across difference bcm2835 devices. Do you know more specific > details? I didn't know that only Raspberry Pi was supported and I have no details about the other boards. I'll send a new patch. Thanks.
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c index 7116968..fdf0d7d 100644 --- a/drivers/watchdog/bcm2835_wdt.c +++ b/drivers/watchdog/bcm2835_wdt.c @@ -36,6 +36,13 @@ #define PM_RSTC_WRCFG_FULL_RESET 0x00000020 #define PM_RSTC_RESET 0x00000102 +/* + * The Raspberry Pi firmware uses the RSTS register to know which partiton + * to boot from. The partiton value is spread into bits 0, 2, 4, 6, 8, 10. + * Partiton 63 is a special partition used by the firmware to indicate halt. + */ +#define PM_RSTS_RASPBERRYPI_HALT 0x555 + #define SECS_TO_WDOG_TICKS(x) ((x) << 16) #define WDOG_TICKS_TO_SECS(x) ((x) >> 16) @@ -159,6 +166,24 @@ static void bcm2835_power_off(void) bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL); } +static void rpi_power_off(void) +{ + struct device_node *np = + of_find_compatible_node(NULL, NULL, "brcm,raspberrypi-pm-wdt"); + struct platform_device *pdev = of_find_device_by_node(np); + struct bcm2835_wdt *wdt = platform_get_drvdata(pdev); + u32 val; + + val = readl_relaxed(wdt->base + PM_RSTS); + val |= PM_PASSWORD | PM_RSTS_RASPBERRYPI_HALT; + writel_relaxed(val, wdt->base + PM_RSTS); + + /* Continue with normal reset mechanism */ + bcm2835_restart(&wdt->restart_handler, REBOOT_HARD, NULL); +} + +static const struct of_device_id bcm2835_wdt_of_match[]; + static int bcm2835_wdt_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -192,8 +217,12 @@ static int bcm2835_wdt_probe(struct platform_device *pdev) wdt->restart_handler.notifier_call = bcm2835_restart; wdt->restart_handler.priority = 128; register_restart_handler(&wdt->restart_handler); - if (pm_power_off == NULL) - pm_power_off = bcm2835_power_off; + if (!pm_power_off) { + const struct of_device_id *match; + + match = of_match_node(bcm2835_wdt_of_match, pdev->dev.of_node); + pm_power_off = match->data; + } dev_info(dev, "Broadcom BCM2835 watchdog timer"); return 0; @@ -204,7 +233,7 @@ static int bcm2835_wdt_remove(struct platform_device *pdev) struct bcm2835_wdt *wdt = platform_get_drvdata(pdev); unregister_restart_handler(&wdt->restart_handler); - if (pm_power_off == bcm2835_power_off) + if (pm_power_off == bcm2835_power_off || pm_power_off == rpi_power_off) pm_power_off = NULL; watchdog_unregister_device(&bcm2835_wdt_wdd); iounmap(wdt->base); @@ -218,7 +247,8 @@ static void bcm2835_wdt_shutdown(struct platform_device *pdev) } static const struct of_device_id bcm2835_wdt_of_match[] = { - { .compatible = "brcm,bcm2835-pm-wdt", }, + { .compatible = "brcm,bcm2835-pm-wdt", .data = bcm2835_power_off }, + { .compatible = "brcm,raspberrypi-pm-wdt", .data = rpi_power_off }, {}, }; MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);