* [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module @ 2019-09-30 10:35 Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 1/2] power: reset: gpio-poweroff: add force mode Oleksandr Suvorov ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Oleksandr Suvorov @ 2019-09-30 10:35 UTC (permalink / raw) To: linux-kernel Cc: Marcel Ziswiler, Jamie Lentin, linux-pm, Andrew Lunn, Igor Opaniuk, Oleksandr Suvorov, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland to register its own pm_power_off handler even if someone has registered this handler earlier. Useful to change a way to power off the system using DT files. Oleksandr Suvorov (2): power: reset: gpio-poweroff: add force mode dt-bindings: power: reset: gpio-poweroff: Add 'force-mode' property .../bindings/power/reset/gpio-poweroff.txt | 3 +++ drivers/power/reset/gpio-poweroff.c | 27 ++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] power: reset: gpio-poweroff: add force mode 2019-09-30 10:35 [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Oleksandr Suvorov @ 2019-09-30 10:35 ` Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 2/2] dt-bindings: power: reset: gpio-poweroff: Add 'force-mode' property Oleksandr Suvorov 2019-09-30 12:14 ` [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Andrew Lunn 2 siblings, 0 replies; 7+ messages in thread From: Oleksandr Suvorov @ 2019-09-30 10:35 UTC (permalink / raw) To: linux-kernel Cc: Marcel Ziswiler, Jamie Lentin, linux-pm, Andrew Lunn, Igor Opaniuk, Oleksandr Suvorov, Sebastian Reichel Property "force-mode" tells the driver to replace previously initialized power-off kernel hook and allows gpio-poweroff to probe and operate successfully in any case. Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> --- drivers/power/reset/gpio-poweroff.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/power/reset/gpio-poweroff.c b/drivers/power/reset/gpio-poweroff.c index 6a4bbb506551..cca9f36fdce6 100644 --- a/drivers/power/reset/gpio-poweroff.c +++ b/drivers/power/reset/gpio-poweroff.c @@ -24,6 +24,7 @@ static struct gpio_desc *reset_gpio; static u32 timeout = DEFAULT_TIMEOUT_MS; static u32 active_delay = 100; static u32 inactive_delay = 100; +static void *prev_pm_power_off; static void gpio_poweroff_do_poweroff(void) { @@ -49,14 +50,28 @@ static void gpio_poweroff_do_poweroff(void) static int gpio_poweroff_probe(struct platform_device *pdev) { bool input = false; + bool force = false; enum gpiod_flags flags; - /* If a pm_power_off function has already been added, leave it alone */ + + force = device_property_read_bool(&pdev->dev, "force-mode"); + + /* + * If a pm_power_off function has already been added, leave it alone, + * if force-mode is not enabled. + */ if (pm_power_off != NULL) { - dev_err(&pdev->dev, - "%s: pm_power_off function already registered", - __func__); - return -EBUSY; + if (force) { + dev_warn(&pdev->dev, + "%s: pm_power_off function replaced", + __func__); + prev_pm_power_off = pm_power_off; + } else { + dev_err(&pdev->dev, + "%s: pm_power_off function already registered", + __func__); + return -EBUSY; + } } input = device_property_read_bool(&pdev->dev, "input"); @@ -81,7 +96,7 @@ static int gpio_poweroff_probe(struct platform_device *pdev) static int gpio_poweroff_remove(struct platform_device *pdev) { if (pm_power_off == &gpio_poweroff_do_poweroff) - pm_power_off = NULL; + pm_power_off = prev_pm_power_off; return 0; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] dt-bindings: power: reset: gpio-poweroff: Add 'force-mode' property 2019-09-30 10:35 [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 1/2] power: reset: gpio-poweroff: add force mode Oleksandr Suvorov @ 2019-09-30 10:35 ` Oleksandr Suvorov 2019-09-30 12:14 ` [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Andrew Lunn 2 siblings, 0 replies; 7+ messages in thread From: Oleksandr Suvorov @ 2019-09-30 10:35 UTC (permalink / raw) To: linux-kernel Cc: Marcel Ziswiler, Jamie Lentin, linux-pm, Andrew Lunn, Igor Opaniuk, Oleksandr Suvorov, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland Add 'force-mode' property to allow the driver to load even if someone has registered the pm_power_off hook earlier. Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> --- .../devicetree/bindings/power/reset/gpio-poweroff.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt b/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt index 3e56c1b34a4c..2056e299a472 100644 --- a/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt +++ b/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt @@ -31,6 +31,9 @@ Optional properties: - inactive-delay-ms: Delay (default 100) to wait after driving gpio inactive - timeout-ms: Time to wait before asserting a WARN_ON(1). If nothing is specified, 3000 ms is used. +- force-mode: Force replacing pm_power_off kernel hook. + If this optional property is not specified, the driver will fail to + load if someone has registered the pm_power_off hook earlier. Examples: -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module 2019-09-30 10:35 [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 1/2] power: reset: gpio-poweroff: add force mode Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 2/2] dt-bindings: power: reset: gpio-poweroff: Add 'force-mode' property Oleksandr Suvorov @ 2019-09-30 12:14 ` Andrew Lunn 2019-09-30 14:11 ` Oleksandr Suvorov 2 siblings, 1 reply; 7+ messages in thread From: Andrew Lunn @ 2019-09-30 12:14 UTC (permalink / raw) To: Oleksandr Suvorov Cc: linux-kernel, Marcel Ziswiler, Jamie Lentin, linux-pm, Igor Opaniuk, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland On Mon, Sep 30, 2019 at 10:35:36AM +0000, Oleksandr Suvorov wrote: > to register its own pm_power_off handler even if someone has registered > this handler earlier. > Useful to change a way to power off the system using DT files. Hi Oleksandr I'm not sure this is a good idea. What happens when there are two drivers using forced mode? You then get which ever is register last. Non deterministic behaviour. What is the other driver which is causing you problems? How is it getting probed? DT? Thanks Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module 2019-09-30 12:14 ` [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Andrew Lunn @ 2019-09-30 14:11 ` Oleksandr Suvorov 2019-09-30 16:32 ` Andrew Lunn 0 siblings, 1 reply; 7+ messages in thread From: Oleksandr Suvorov @ 2019-09-30 14:11 UTC (permalink / raw) To: Andrew Lunn Cc: Oleksandr Suvorov, linux-kernel, Marcel Ziswiler, Jamie Lentin, linux-pm, Igor Opaniuk, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland Hi Andrew, On Mon, Sep 30, 2019 at 3:16 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Mon, Sep 30, 2019 at 10:35:36AM +0000, Oleksandr Suvorov wrote: > > to register its own pm_power_off handler even if someone has registered > > this handler earlier. > > Useful to change a way to power off the system using DT files. > > Hi Oleksandr > > I'm not sure this is a good idea. What happens when there are two > drivers using forced mode? You then get which ever is register last. > Non deterministic behaviour. You're right, we have to handle a case when gpio-poweroff fails to power the system off. Please look at the 2nd version of the patchset. There are 3 only drivers that forcibly register its own pm_power_off handler even if it has been registered before. drivers/firmware/efi/reboot.c - supports chained call of next pm_power_off handler if its own handler fails. arch/x86/platform/iris/iris.c, drivers/char/ipmi/ipmi_poweroff.c - don't support calling of next pm_power_off handler. Looks like these drivers should be fixed too. All other drivers don't change already initialized pm_power_off handler. > What is the other driver which is causing you problems? How is it > getting probed? DT? There are several PMUs, RTCs, watchdogs that register their own pm_power_off. Most of them, probably not all, are probed from DT. > > Thanks > Andrew -- Best regards Oleksandr Suvorov Toradex AG Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800 (main line) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module 2019-09-30 14:11 ` Oleksandr Suvorov @ 2019-09-30 16:32 ` Andrew Lunn 2019-09-30 19:42 ` Jamie Lentin 0 siblings, 1 reply; 7+ messages in thread From: Andrew Lunn @ 2019-09-30 16:32 UTC (permalink / raw) To: Oleksandr Suvorov Cc: linux-kernel, Marcel Ziswiler, Jamie Lentin, linux-pm, Igor Opaniuk, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland On Mon, Sep 30, 2019 at 02:11:59PM +0000, Oleksandr Suvorov wrote: > Hi Andrew, > > On Mon, Sep 30, 2019 at 3:16 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Mon, Sep 30, 2019 at 10:35:36AM +0000, Oleksandr Suvorov wrote: > > > to register its own pm_power_off handler even if someone has registered > > > this handler earlier. > > > Useful to change a way to power off the system using DT files. > > > > Hi Oleksandr > > > > I'm not sure this is a good idea. What happens when there are two > > drivers using forced mode? You then get which ever is register last. > > Non deterministic behaviour. > > You're right, we have to handle a case when gpio-poweroff fails to > power the system off. Please look at the > 2nd version of the patchset. > > There are 3 only drivers that forcibly register its own pm_power_off > handler even if it has been registered before. > > drivers/firmware/efi/reboot.c - supports chained call of next > pm_power_off handler if its own handler fails. > > arch/x86/platform/iris/iris.c, drivers/char/ipmi/ipmi_poweroff.c - > don't support calling of next pm_power_off handler. > Looks like these drivers should be fixed too. > > All other drivers don't change already initialized pm_power_off handler. > > > What is the other driver which is causing you problems? How is it > > getting probed? DT? > > There are several PMUs, RTCs, watchdogs that register their own pm_power_off. > Most of them, probably not all, are probed from DT. And which specific one is causing you problems. I don't like this forced parameter. No other driver is using it. Maybe we should change this driver to support chained pm_power_off handlers? Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module 2019-09-30 16:32 ` Andrew Lunn @ 2019-09-30 19:42 ` Jamie Lentin 0 siblings, 0 replies; 7+ messages in thread From: Jamie Lentin @ 2019-09-30 19:42 UTC (permalink / raw) To: Andrew Lunn Cc: Oleksandr Suvorov, linux-kernel, Marcel Ziswiler, linux-pm, Igor Opaniuk, devicetree, Sebastian Reichel, Rob Herring, Mark Rutland On Mon, 30 Sep 2019, Andrew Lunn wrote: > On Mon, Sep 30, 2019 at 02:11:59PM +0000, Oleksandr Suvorov wrote: >> Hi Andrew, >> >> On Mon, Sep 30, 2019 at 3:16 PM Andrew Lunn <andrew@lunn.ch> wrote: >>> >>> On Mon, Sep 30, 2019 at 10:35:36AM +0000, Oleksandr Suvorov wrote: >>>> to register its own pm_power_off handler even if someone has registered >>>> this handler earlier. >>>> Useful to change a way to power off the system using DT files. >>> >>> Hi Oleksandr >>> >>> I'm not sure this is a good idea. What happens when there are two >>> drivers using forced mode? You then get which ever is register last. >>> Non deterministic behaviour. >> >> You're right, we have to handle a case when gpio-poweroff fails to >> power the system off. Please look at the >> 2nd version of the patchset. >> >> There are 3 only drivers that forcibly register its own pm_power_off >> handler even if it has been registered before. >> >> drivers/firmware/efi/reboot.c - supports chained call of next >> pm_power_off handler if its own handler fails. >> >> arch/x86/platform/iris/iris.c, drivers/char/ipmi/ipmi_poweroff.c - >> don't support calling of next pm_power_off handler. >> Looks like these drivers should be fixed too. >> >> All other drivers don't change already initialized pm_power_off handler. >> >>> What is the other driver which is causing you problems? How is it >>> getting probed? DT? >> >> There are several PMUs, RTCs, watchdogs that register their own pm_power_off. >> Most of them, probably not all, are probed from DT. > > And which specific one is causing you problems. > > I don't like this forced parameter. No other driver is using it. > > Maybe we should change this driver to support chained pm_power_off > handlers? There's still scope for non-deterministic behaviour though, as the chaining would take place depending on the probe ordering. Admittedly if the gpio-poweroff works it's unlikely to be a problem, but still seems messy. Without knowing specifics, disabling the devices that can't turn the device off seems like a better bet. If they'd be otherwise useful, I see there's a of_device_is_system_power_controller(), see: /Documentation/devicetree/bindings/power/power-controller.txt https://elixir.bootlin.com/linux/latest/source/drivers/mfd/max77620.c#L566 ...maybe that can be added to the devices getting in the way? Cheers, [0] https://elixir.bootlin.com/linux/latest/source/drivers/watchdog/bcm2835_wdt.c#L152 (chosen at random) > > Andrew > -- Jamie Lentin ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-30 21:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-30 10:35 [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 1/2] power: reset: gpio-poweroff: add force mode Oleksandr Suvorov 2019-09-30 10:35 ` [PATCH 2/2] dt-bindings: power: reset: gpio-poweroff: Add 'force-mode' property Oleksandr Suvorov 2019-09-30 12:14 ` [PATCH 0/2] This patch introduces a feature to force gpio-poweroff module Andrew Lunn 2019-09-30 14:11 ` Oleksandr Suvorov 2019-09-30 16:32 ` Andrew Lunn 2019-09-30 19:42 ` Jamie Lentin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).