| Message ID | 20181120172000.15102-1-eric@anholt.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | BCM2835 PM driver | expand |
Hi Eric, > Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben: > > > This series moves the BCM2835 WDT driver that controls a fraction of > the PM block out to soc/ and adds most of the rest of its > functionality. My motivation has been to have V3D be functional > without firmware calls, probably improve its interactivity (since > we'll be able to power on/off without RPC to the firmware that may be > busy with other tasks), and (in a patch not submitted in this series) > extend its binding to use the reset controller instead of trying to > reset by toggling its power domain. > > I've tested V3D with a few hours of running a V3D test, sleep(1) (to > trigger PM domain off); running a GPU hang job (to trigger reset); > sleep(1). The non-hanging success-case job always passed, and dmesg > had no complaints from bcm2835-pm. The other power domains are not > tested, but I've done my best. > > This series will probably also be of interest to the > https://github.com/christinaa/rpi-open-firmware project for enabling USB. > apologize to give you my feedback after you send out the series. I know you won't be happy about it, but i think we need a little more complex but future proof solution for this power driver. According to the register definition of the PM block, we have multiple functions here (power domains, watchdog, pads/pinctrl, ...). Since this is common for ARM SoCs there is a subsystem called mfd (multi function device) [1] to abstract all resources of the IP block. This has the advantage that we don't need a monolithic driver which takes care of all functions. According to this approach we would have the following drivers: mfd/bcm2835.c soc/bcm/bcm2835-power.c watchdog/bcm2835_wdt.c Best regards Stefan [1] - http://events17.linuxfoundation.org/sites/events/files/slides/belloni-mfd-regmap-syscon_0.pdf
Stefan Wahren <stefan.wahren@i2se.com> writes: > Hi Eric, > >> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben: >> >> >> This series moves the BCM2835 WDT driver that controls a fraction of >> the PM block out to soc/ and adds most of the rest of its >> functionality. My motivation has been to have V3D be functional >> without firmware calls, probably improve its interactivity (since >> we'll be able to power on/off without RPC to the firmware that may be >> busy with other tasks), and (in a patch not submitted in this series) >> extend its binding to use the reset controller instead of trying to >> reset by toggling its power domain. >> >> I've tested V3D with a few hours of running a V3D test, sleep(1) (to >> trigger PM domain off); running a GPU hang job (to trigger reset); >> sleep(1). The non-hanging success-case job always passed, and dmesg >> had no complaints from bcm2835-pm. The other power domains are not >> tested, but I've done my best. >> >> This series will probably also be of interest to the >> https://github.com/christinaa/rpi-open-firmware project for enabling USB. >> > > apologize to give you my feedback after you send out the series. > > I know you won't be happy about it, but i think we need a little more > complex but future proof solution for this power driver. According to > the register definition of the PM block, we have multiple functions > here (power domains, watchdog, pads/pinctrl, ...). Since this is > common for ARM SoCs there is a subsystem called mfd (multi function > device) [1] to abstract all resources of the IP block. > > This has the advantage that we don't need a monolithic driver which > takes care of all functions. I consider your "advantage" to be a disadvantage. By forcing the split, you end up having more driver files to manage, more platform devices, and more error-prone code to get the resources from the parent down into the client. It feels like writing software for the sake of writing software, rather than solving a concrete problem. My original series: 10 files changed, 951 insertions(+), 273 deletions(-) The MFD series: 12 files changed, 882 insertions(+), 25 deletions(-)
On Tue, Nov 20, 2018 at 10:34 PM Eric Anholt <eric@anholt.net> wrote: > >> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben: > >> > >> > >> This series moves the BCM2835 WDT driver that controls a fraction of > >> the PM block out to soc/ and adds most of the rest of its > >> functionality. My motivation has been to have V3D be functional > >> without firmware calls, probably improve its interactivity (since > >> we'll be able to power on/off without RPC to the firmware that may be > >> busy with other tasks), and (in a patch not submitted in this series) > >> extend its binding to use the reset controller instead of trying to > >> reset by toggling its power domain. > >> > >> I've tested V3D with a few hours of running a V3D test, sleep(1) (to > >> trigger PM domain off); running a GPU hang job (to trigger reset); > >> sleep(1). The non-hanging success-case job always passed, and dmesg > >> had no complaints from bcm2835-pm. The other power domains are not > >> tested, but I've done my best. > >> > >> This series will probably also be of interest to the > >> https://github.com/christinaa/rpi-open-firmware project for enabling USB. > >> > > > > apologize to give you my feedback after you send out the series. > > > > I know you won't be happy about it, but i think we need a little more > > complex but future proof solution for this power driver. According to > > the register definition of the PM block, we have multiple functions > > here (power domains, watchdog, pads/pinctrl, ...). Since this is > > common for ARM SoCs there is a subsystem called mfd (multi function > > device) [1] to abstract all resources of the IP block. > > > > This has the advantage that we don't need a monolithic driver which > > takes care of all functions. > > I consider your "advantage" to be a disadvantage. By forcing the split, > you end up having more driver files to manage, more platform devices, > and more error-prone code to get the resources from the parent down into > the client. It feels like writing software for the sake of writing > software, rather than solving a concrete problem. > > My original series: > > 10 files changed, 951 insertions(+), 273 deletions(-) > > The MFD series: > > 12 files changed, 882 insertions(+), 25 deletions(-) My preference in general is having less code in drivers/soc and more in subsystems of people that understand their stuff. Not every driver leans itself to being an MFD, but the more differnent functions it has, the better it is to have it split up. This was the original motivation of moving irqchip, clk, pinctrl, etc drivers out of arch/arm/mach-* into their own subsystems. Arnd
This series moves the BCM2835 WDT driver that controls a fraction of the PM block out to soc/ and adds most of the rest of its functionality. My motivation has been to have V3D be functional without firmware calls, probably improve its interactivity (since we'll be able to power on/off without RPC to the firmware that may be busy with other tasks), and (in a patch not submitted in this series) extend its binding to use the reset controller instead of trying to reset by toggling its power domain. I've tested V3D with a few hours of running a V3D test, sleep(1) (to trigger PM domain off); running a GPU hang job (to trigger reset); sleep(1). The non-hanging success-case job always passed, and dmesg had no complaints from bcm2835-pm. The other power domains are not tested, but I've done my best. This series will probably also be of interest to the https://github.com/christinaa/rpi-open-firmware project for enabling USB. Eric Anholt (8): watchdog: bcm2835: Move the driver to the soc/ directory. soc: bcm: bcm2835-pm: Rename the driver to its new "PM" name. soc: bcm: bcm2835-pm: Stop using _relaxed mmio accessors. soc: bcm: bcm2835-pm: Make some little accessor macros for the mmio area. dt-bindings: soc: Add a new binding for the BCM2835 PM node. soc: bcm: bcm2835-pm: Add support for power domains under a new binding. ARM: bcm283x: Extend the WDT DT node out to cover the whole PM block. ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware. .../bindings/soc/bcm/brcm,bcm2835-pm.txt | 42 + arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 - arch/arm/boot/dts/bcm283x.dtsi | 16 +- arch/arm/mach-bcm/Kconfig | 1 + drivers/soc/bcm/Makefile | 1 + drivers/soc/bcm/bcm2835-pm.c | 866 ++++++++++++++++++ drivers/watchdog/Kconfig | 11 - drivers/watchdog/Makefile | 1 - drivers/watchdog/bcm2835_wdt.c | 254 ----- include/dt-bindings/soc/bcm2835-pm.h | 28 + 10 files changed, 951 insertions(+), 273 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt create mode 100644 drivers/soc/bcm/bcm2835-pm.c delete mode 100644 drivers/watchdog/bcm2835_wdt.c create mode 100644 include/dt-bindings/soc/bcm2835-pm.h