| Message ID | 1434195541-28368-1-git-send-email-noralf@tronnes.org |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/13/2015 05:39 AM, Noralf Trønnes wrote: > The Raspberry Pi uses a new value for halt in the PM_RSTS watchdog > register. Expand the compatible string to cover this. FWIW, the series, Tested-by: Stephen Warren <swarren@wwwdotorg.org> ... but that doesn't imply my ack for the patches. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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). -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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). -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt index f801d71..1a80931 100644 --- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt @@ -2,7 +2,8 @@ BCM2835 Watchdog timer Required properties: -- compatible : should be "brcm,bcm2835-pm-wdt" +- compatible : should be "brcm,raspberrypi-pm-wdt" for the Raspberry Pi and + "brcm,bcm2835-pm-wdt" for others. - reg : Specifies base physical address and size of the registers. Optional properties:
The Raspberry Pi has changed how its firmware detects a poweroff and needs special handling for this. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- v2: Fix typo in commit message Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.2.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html