2019-03-21 09:02:46

by Marek Szyprowski

Subject: [PATCH] PM / devfreq: exynos-bus: Suspend all devices on system shutdown

Force all Exynos buses to safe operation points before doing the system
reboot operation. There are board on which some aggressive power saving
operation points are behind the capabilities of the bootloader to properly
reset the hardware and boot the board. This way one can avoid board crash
early after reboot.

This fixes reboot issue on OdroidU3 board both with eMMC and SD boot.

Reported-by: Markus Reichl <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
This is an alternative to https://patchwork.kernel.org/patch/10781433/
limited only to Exynos-bus driver.
---
drivers/devfreq/exynos-bus.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index c25658b26598..486cc5b422f1 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -514,6 +514,13 @@ static int exynos_bus_probe(struct platform_device *pdev)
return ret;
}

+static void exynos_bus_shutdown(struct platform_device *pdev)
+{
+ struct exynos_bus *bus = dev_get_drvdata(&pdev->dev);
+
+ devfreq_suspend_device(bus->devfreq);
+}
+
#ifdef CONFIG_PM_SLEEP
static int exynos_bus_resume(struct device *dev)
{
@@ -556,6 +563,7 @@ MODULE_DEVICE_TABLE(of, exynos_bus_of_match);

static struct platform_driver exynos_bus_platdrv = {
.probe = exynos_bus_probe,
+ .shutdown = exynos_bus_shutdown,
.driver = {
.name = "exynos-bus",
.pm = &exynos_bus_pm,
--
2.17.1

2019-03-21 09:20:12

by Chanwoo Choi

Subject: Re: [PATCH] PM / devfreq: exynos-bus: Suspend all devices on system shutdown

Hi Marek,

On 19. 3. 21. 오후 6:01, Marek Szyprowski wrote:
> Force all Exynos buses to safe operation points before doing the system
> reboot operation. There are board on which some aggressive power saving
> operation points are behind the capabilities of the bootloader to properly
> reset the hardware and boot the board. This way one can avoid board crash
> early after reboot.
>
> This fixes reboot issue on OdroidU3 board both with eMMC and SD boot.
>
> Reported-by: Markus Reichl <[email protected]>
> Signed-off-by: Marek Szyprowski <[email protected]>
> ---
> This is an alternative to https://patchwork.kernel.org/patch/10781433/
> limited only to Exynos-bus driver.
> ---
> drivers/devfreq/exynos-bus.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index c25658b26598..486cc5b422f1 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -514,6 +514,13 @@ static int exynos_bus_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static void exynos_bus_shutdown(struct platform_device *pdev)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(&pdev->dev);
> +
> + devfreq_suspend_device(bus->devfreq);
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int exynos_bus_resume(struct device *dev)
> {
> @@ -556,6 +563,7 @@ MODULE_DEVICE_TABLE(of, exynos_bus_of_match);
>
> static struct platform_driver exynos_bus_platdrv = {
> .probe = exynos_bus_probe,
> + .shutdown = exynos_bus_shutdown,
> .driver = {
> .name = "exynos-bus",
> .pm = &exynos_bus_pm,
>

Actually, I already agreed the previous patch.
Also, it looks good to me.

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2019-03-21 09:30:33

by Marek Szyprowski

Subject: Re: [PATCH] PM / devfreq: exynos-bus: Suspend all devices on system shutdown

Hi Chanwoo,

On 2019-03-21 10:19, Chanwoo Choi wrote:
> On 19. 3. 21. 오후 6:01, Marek Szyprowski wrote:
>> Force all Exynos buses to safe operation points before doing the system
>> reboot operation. There are board on which some aggressive power saving
>> operation points are behind the capabilities of the bootloader to properly
>> reset the hardware and boot the board. This way one can avoid board crash
>> early after reboot.
>>
>> This fixes reboot issue on OdroidU3 board both with eMMC and SD boot.
>>
>> Reported-by: Markus Reichl <[email protected]>
>> Signed-off-by: Marek Szyprowski <[email protected]>
>> ---
>> This is an alternative to https://patchwork.kernel.org/patch/10781433/
>> limited only to Exynos-bus driver.
>> ---
>> drivers/devfreq/exynos-bus.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index c25658b26598..486cc5b422f1 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -514,6 +514,13 @@ static int exynos_bus_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> +static void exynos_bus_shutdown(struct platform_device *pdev)
>> +{
>> + struct exynos_bus *bus = dev_get_drvdata(&pdev->dev);
>> +
>> + devfreq_suspend_device(bus->devfreq);
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>> static int exynos_bus_resume(struct device *dev)
>> {
>> @@ -556,6 +563,7 @@ MODULE_DEVICE_TABLE(of, exynos_bus_of_match);
>>
>> static struct platform_driver exynos_bus_platdrv = {
>> .probe = exynos_bus_probe,
>> + .shutdown = exynos_bus_shutdown,
>> .driver = {
>> .name = "exynos-bus",
>> .pm = &exynos_bus_pm,
>>
> Actually, I already agreed the previous patch.
> Also, it looks good to me.

Yes, I know, but MyungJoo had some objections, that's why I prepared
alternative version.

> Acked-by: Chanwoo Choi <[email protected]>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2019-03-25 04:39:57

by MyungJoo Ham

Subject: RE: Re: [PATCH] PM / devfreq: exynos-bus: Suspend all devices on system shutdown

>Hi Chanwoo,
>
>On 2019-03-21 10:19, Chanwoo Choi wrote:
>> On 19. 3. 21. 오후 6:01, Marek Szyprowski wrote:
>>> Force all Exynos buses to safe operation points before doing the system
>>> reboot operation. There are board on which some aggressive power saving
>>> operation points are behind the capabilities of the bootloader to properly
>>> reset the hardware and boot the board. This way one can avoid board crash
>>> early after reboot.
>>>
>>> This fixes reboot issue on OdroidU3 board both with eMMC and SD boot.
>>>
>>> Reported-by: Markus Reichl <[email protected]>
>>> Signed-off-by: Marek Szyprowski <[email protected]>
>>> ---
>>> This is an alternative to https://patchwork.kernel.org/patch/10781433/
>>> limited only to Exynos-bus driver.
>>> ---
>>> drivers/devfreq/exynos-bus.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>> index c25658b26598..486cc5b422f1 100644
>>> --- a/drivers/devfreq/exynos-bus.c
>>> +++ b/drivers/devfreq/exynos-bus.c
>>> @@ -514,6 +514,13 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>> return ret;
>>> }
>>>
>>> +static void exynos_bus_shutdown(struct platform_device *pdev)
>>> +{
>>> + struct exynos_bus *bus = dev_get_drvdata(&pdev->dev);
>>> +
>>> + devfreq_suspend_device(bus->devfreq);
>>> +}
>>> +
>>> #ifdef CONFIG_PM_SLEEP
>>> static int exynos_bus_resume(struct device *dev)
>>> {
>>> @@ -556,6 +563,7 @@ MODULE_DEVICE_TABLE(of, exynos_bus_of_match);
>>>
>>> static struct platform_driver exynos_bus_platdrv = {
>>> .probe = exynos_bus_probe,
>>> + .shutdown = exynos_bus_shutdown,
>>> .driver = {
>>> .name = "exynos-bus",
>>> .pm = &exynos_bus_pm,
>>>
>> Actually, I already agreed the previous patch.
>> Also, it looks good to me.
>
>Yes, I know, but MyungJoo had some objections, that's why I prepared
>alternative version.
>
>> Acked-by: Chanwoo Choi <[email protected]>
>>

This has become looking good now :)


Acked-by: MyungJoo Ham <[email protected]>


Cheers,
MyungJoo