Messages in this thread | | | Date | Sat, 5 Nov 2022 14:18:48 -0500 | From | Samuel Holland <> | Subject | Re: [BISECTED] Allwinner A33 tablet does not fully power off |
| |
On 11/5/22 03:23, Ivaylo Dimitrov wrote: > Hi Samuel, > > On 5.11.22 г. 4:21 ч., Samuel Holland wrote: >> Hi Ivo, >> >> On 10/29/22 10:23, Ivaylo Dimitrov wrote: >>> After commit 843107498f91e57d1d4b22cd8787112726fdaeb4 (bus: sunxi-rsb: >>> Implement suspend/resume/shutdown callbacks) Q8 A33 tablet I have here >>> cannot be powered-on after power-off, it needs press-and-hold of the >>> power button for 10 seconds (I guess some HW assisted power down >>> happens) before it can be powered-on again. >>> >>> The following patch makes it behave correctly: >>> >>> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c >>> index 60b082fe2ed0..30016d62044c 100644 >>> --- a/drivers/bus/sunxi-rsb.c >>> +++ b/drivers/bus/sunxi-rsb.c >>> @@ -818,10 +818,7 @@ static int sunxi_rsb_remove(struct platform_device >>> *pdev) >>> >>> static void sunxi_rsb_shutdown(struct platform_device *pdev) >>> { >>> - struct sunxi_rsb *rsb = platform_get_drvdata(pdev); >>> - >>> pm_runtime_disable(&pdev->dev); >>> - sunxi_rsb_hw_exit(rsb); >>> } >>> >>> static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = { >>> >>> >>> I guess the issue comes from the fact that by the time 'power off' >>> command to the power management IC has to be send, the bus it lives on >>> is already down, so the device is left in semi-powered down state. Ofc >>> this is a wild guess, however, preventing the bus being turned off on >>> shutdown fixes the issue. >> >> Your guess is correct. The controller gets shut down in >> >> kernel_power_off() >> kernel_shutdown_prepare() >> device_shutdown() >> >> but the PMIC communication needs to happen later in >> >> kernel_power_off() >> machine_power_off() >> pm_power_off() >> >>> Please LMK if the above is the correct approach so I will send a proper >>> patch or something else shall be fixed. >> >> Yes, this is exactly the right approach. The whole sunxi_rsb_shutdown() > > Don't we need pm_runtime_disable() on shutdown? As IIUC, the controller > might be suspended and we have to resume it to put it in state to accept > commands later on(in pm_power_off()).
sunxi_rsb_write() takes care of resuming the controller, so the controller being suspended prior to pm_power_off() is fine. pm_runtime_disable() would actually prevent resuming the controller later in sunxi_rsb_write().
>> function should be removed. When you send a patch, please add a Fixes: >> tag referencing the commit that you bisected to.
I found a couple of other issues as well, so I'll send out some fixes with you CC'd.
Regards, Samuel
| |