[PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Anand Moon]

Added .shutdown function to s2mps11 to help poweroff the board succefully.
The device drivers set the register to turn off the PMIC.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
Changes fixes the poweroff
root@odroidxu3:~# poweroff

Broadcast message from root@odroidxu3
        (/dev/ttySAC2) at 13:08 ...

The system is going down for power off NOW!
root@odroidxu3:~# wait-for-state stop/waiting
 * Stopping rsync daemon rsync                                           [ OK ]
 * Stopping RDP Session manager                                          [ OK ]
 * Stopping NTP server ntpd                                              [ OK ]
 * Asking all remaining processes to terminate...                        [ OK ]
 * All processes ended within 1 seconds...                               [ OK ]
nm-dispatcher.action: Caught signal 15, shutting down...
ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name

ModemManager[2134]: <info>  ModemManager is shut down

 * Unmounting temporary filesystems...                                   [ OK ]
 * Deactivating swap...                                                  [ OK ]
 * Unmounting local filesystems...                                       [ OK ]
 * Will now halt
[  209.020280] reboot: Power down
[  209.122039] Power down failed, please power off system manually.
---
 drivers/regulator/s2mps11.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index ff82811..871f7b8 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -1060,6 +1060,13 @@ out:
 	return ret;
 }
 
+static void s2mps11_pmic_shutdown(struct platform_device *pdev)
+{
+	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+
+	regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
+}
+
 static const struct platform_device_id s2mps11_pmic_id[] = {
 	{ "s2mps11-pmic", S2MPS11X},
 	{ "s2mps13-pmic", S2MPS13X},
@@ -1074,6 +1081,7 @@ static struct platform_driver s2mps11_pmic_driver = {
 		.name = "s2mps11-pmic",
 	},
 	.probe = s2mps11_pmic_probe,
+	.shutdown = s2mps11_pmic_shutdown,
 	.id_table = s2mps11_pmic_id,
 };
 
-- 
1.9.1

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Krzysztof Kozlowski]

2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
> Added .shutdown function to s2mps11 to help poweroff the board succefully.

s/succefully/successfully/

> The device drivers set the register to turn off the PMIC.

Driver or drivers?

>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> Changes fixes the poweroff
> root@odroidxu3:~# poweroff
>
> Broadcast message from root@odroidxu3
>         (/dev/ttySAC2) at 13:08 ...
>
> The system is going down for power off NOW!
> root@odroidxu3:~# wait-for-state stop/waiting
>  * Stopping rsync daemon rsync                                           [ OK ]
>  * Stopping RDP Session manager                                          [ OK ]
>  * Stopping NTP server ntpd                                              [ OK ]
>  * Asking all remaining processes to terminate...                        [ OK ]
>  * All processes ended within 1 seconds...                               [ OK ]
> nm-dispatcher.action: Caught signal 15, shutting down...
> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>
> ModemManager[2134]: <info>  ModemManager is shut down
>
>  * Unmounting temporary filesystems...                                   [ OK ]
>  * Deactivating swap...                                                  [ OK ]
>  * Unmounting local filesystems...                                       [ OK ]
>  * Will now halt
> [  209.020280] reboot: Power down
> [  209.122039] Power down failed, please power off system manually.
> ---
>  drivers/regulator/s2mps11.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index ff82811..871f7b8 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -1060,6 +1060,13 @@ out:
>         return ret;
>  }
>
> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
> +{
> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> +
> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);

This looks odd to me and interesting in the same time...
1. Why clearing all of the fields from the register? Don't you want to
clear only one of it?
2. What exactly you want to do here? What is expected behaviour?
3. How this relates to PWRHOLD coming from AP to the PMIC?
4. Why this is needed only for this driver and only for this board (Odroid XU3)?

Best regards,
Krzysztof

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Anand Moon]

Hi Krzysztof,

On 24 June 2015 at 13:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>> Added .shutdown function to s2mps11 to help poweroff the board succefully.
>
> s/succefully/successfully/
>
>> The device drivers set the register to turn off the PMIC.
>
> Driver or drivers?
>
>>
>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>> ---
>> Changes fixes the poweroff
>> root@odroidxu3:~# poweroff
>>
>> Broadcast message from root@odroidxu3
>>         (/dev/ttySAC2) at 13:08 ...
>>
>> The system is going down for power off NOW!
>> root@odroidxu3:~# wait-for-state stop/waiting
>>  * Stopping rsync daemon rsync                                           [ OK ]
>>  * Stopping RDP Session manager                                          [ OK ]
>>  * Stopping NTP server ntpd                                              [ OK ]
>>  * Asking all remaining processes to terminate...                        [ OK ]
>>  * All processes ended within 1 seconds...                               [ OK ]
>> nm-dispatcher.action: Caught signal 15, shutting down...
>> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>>
>> ModemManager[2134]: <info>  ModemManager is shut down
>>
>>  * Unmounting temporary filesystems...                                   [ OK ]
>>  * Deactivating swap...                                                  [ OK ]
>>  * Unmounting local filesystems...                                       [ OK ]
>>  * Will now halt
>> [  209.020280] reboot: Power down
>> [  209.122039] Power down failed, please power off system manually.
>> ---
>>  drivers/regulator/s2mps11.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
>> index ff82811..871f7b8 100644
>> --- a/drivers/regulator/s2mps11.c
>> +++ b/drivers/regulator/s2mps11.c
>> @@ -1060,6 +1060,13 @@ out:
>>         return ret;
>>  }
>>
>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>> +{
>> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>> +
>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
>
> This looks odd to me and interesting in the same time...
> 1. Why clearing all of the fields from the register? Don't you want to
> clear only one of it?

I have followed what the hardkernel source code point at this point.
I will look into which bit need to clear/set to power off successful.

> 2. What exactly you want to do here? What is expected behaviour?

When you power off the board dose not power off cleanly.
[  209.122039] Power down failed, please power off system manually.

After this changes Its power off the board. Leaving the board with
solid red led blowing.

> 3. How this relates to PWRHOLD coming from AP to the PMIC?

No Idea right now about this right now. but will update you it I have
some thing.

> 4. Why this is needed only for this driver and only for this board (Odroid XU3)?

Yes It could be generic not specific for the Odroid XU3 will correct
in next patch.

-Anand Moon
>
> Best regards,
> Krzysztof

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Krzysztof Kozlowski]

2015-06-24 19:32 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
> Hi Krzysztof,
>
> On 24 June 2015 at 13:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>> Added .shutdown function to s2mps11 to help poweroff the board succefully.
>>
>> s/succefully/successfully/
>>
>>> The device drivers set the register to turn off the PMIC.
>>
>> Driver or drivers?
>>
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>> Changes fixes the poweroff
>>> root@odroidxu3:~# poweroff
>>>
>>> Broadcast message from root@odroidxu3
>>>         (/dev/ttySAC2) at 13:08 ...
>>>
>>> The system is going down for power off NOW!
>>> root@odroidxu3:~# wait-for-state stop/waiting
>>>  * Stopping rsync daemon rsync                                           [ OK ]
>>>  * Stopping RDP Session manager                                          [ OK ]
>>>  * Stopping NTP server ntpd                                              [ OK ]
>>>  * Asking all remaining processes to terminate...                        [ OK ]
>>>  * All processes ended within 1 seconds...                               [ OK ]
>>> nm-dispatcher.action: Caught signal 15, shutting down...
>>> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>>>
>>> ModemManager[2134]: <info>  ModemManager is shut down
>>>
>>>  * Unmounting temporary filesystems...                                   [ OK ]
>>>  * Deactivating swap...                                                  [ OK ]
>>>  * Unmounting local filesystems...                                       [ OK ]
>>>  * Will now halt
>>> [  209.020280] reboot: Power down
>>> [  209.122039] Power down failed, please power off system manually.
>>> ---
>>>  drivers/regulator/s2mps11.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
>>> index ff82811..871f7b8 100644
>>> --- a/drivers/regulator/s2mps11.c
>>> +++ b/drivers/regulator/s2mps11.c
>>> @@ -1060,6 +1060,13 @@ out:
>>>         return ret;
>>>  }
>>>
>>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>>> +{
>>> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>>> +
>>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
>>
>> This looks odd to me and interesting in the same time...
>> 1. Why clearing all of the fields from the register? Don't you want to
>> clear only one of it?
>
> I have followed what the hardkernel source code point at this point.
> I will look into which bit need to clear/set to power off successful.

Following other tree is not enough, even when it solves some one
problem. Drivers could be used on different boards, where hardkernel
patches were not tested. This driver is used on many boards so please
explain exactly what is the cause of problem, what have to be done and
how patch achieves this. Such explanation helps understanding impact
on other boards.

>
>> 2. What exactly you want to do here? What is expected behaviour?
>
> When you power off the board dose not power off cleanly.
> [  209.122039] Power down failed, please power off system manually.
>
> After this changes Its power off the board. Leaving the board with
> solid red led blowing.

You described observable issue, which is nice, but I am asking for
technical details. What exactly do you want to achieve here?
Technically. What is expected behaviour. In technical details, not
only observable. I am asking for these technical details not only
because they are important but also because I do not know them.

>
>> 3. How this relates to PWRHOLD coming from AP to the PMIC?
>
> No Idea right now about this right now. but will update you it I have
> some thing.

The PMIC receives two signals - PWREN and PWRHOLD. It seems strange
that PMIC must override PWRHOLD... maybe there is an issue in
mach-exynos code?

>
>> 4. Why this is needed only for this driver and only for this board (Odroid XU3)?
>
> Yes It could be generic not specific for the Odroid XU3 will correct
> in next patch.

Thanks,
Best regards,
Krzysztof

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Anand Moon]

Hi Krzysztof,

On 24 June 2015 at 16:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> 2015-06-24 19:32 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>> Hi Krzysztof,
>>
>> On 24 June 2015 at 13:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>>> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>> Added .shutdown function to s2mps11 to help poweroff the board succefully.
>>>
>>> s/succefully/successfully/
>>>
>>>> The device drivers set the register to turn off the PMIC.
>>>
>>> Driver or drivers?
>>>
>>>>
>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>> ---
>>>> Changes fixes the poweroff
>>>> root@odroidxu3:~# poweroff
>>>>
>>>> Broadcast message from root@odroidxu3
>>>>         (/dev/ttySAC2) at 13:08 ...
>>>>
>>>> The system is going down for power off NOW!
>>>> root@odroidxu3:~# wait-for-state stop/waiting
>>>>  * Stopping rsync daemon rsync                                           [ OK ]
>>>>  * Stopping RDP Session manager                                          [ OK ]
>>>>  * Stopping NTP server ntpd                                              [ OK ]
>>>>  * Asking all remaining processes to terminate...                        [ OK ]
>>>>  * All processes ended within 1 seconds...                               [ OK ]
>>>> nm-dispatcher.action: Caught signal 15, shutting down...
>>>> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>>>>
>>>> ModemManager[2134]: <info>  ModemManager is shut down
>>>>
>>>>  * Unmounting temporary filesystems...                                   [ OK ]
>>>>  * Deactivating swap...                                                  [ OK ]
>>>>  * Unmounting local filesystems...                                       [ OK ]
>>>>  * Will now halt
>>>> [  209.020280] reboot: Power down
>>>> [  209.122039] Power down failed, please power off system manually.
>>>> ---
>>>>  drivers/regulator/s2mps11.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
>>>> index ff82811..871f7b8 100644
>>>> --- a/drivers/regulator/s2mps11.c
>>>> +++ b/drivers/regulator/s2mps11.c
>>>> @@ -1060,6 +1060,13 @@ out:
>>>>         return ret;
>>>>  }
>>>>
>>>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>>>> +{
>>>> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>>>> +
>>>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
>>>
>>> This looks odd to me and interesting in the same time...
>>> 1. Why clearing all of the fields from the register? Don't you want to
>>> clear only one of it?
>>
>> I have followed what the hardkernel source code point at this point.
>> I will look into which bit need to clear/set to power off successful.
>
> Following other tree is not enough, even when it solves some one
> problem. Drivers could be used on different boards, where hardkernel
> patches were not tested. This driver is used on many boards so please
> explain exactly what is the cause of problem, what have to be done and
> how patch achieves this. Such explanation helps understanding impact
> on other boards.
>
>>
>>> 2. What exactly you want to do here? What is expected behaviour?
>>
>> When you power off the board dose not power off cleanly.
>> [  209.122039] Power down failed, please power off system manually.
>>
>> After this changes Its power off the board. Leaving the board with
>> solid red led blowing.
>
> You described observable issue, which is nice, but I am asking for
> technical details. What exactly do you want to achieve here?
> Technically. What is expected behaviour. In technical details, not
> only observable. I am asking for these technical details not only
> because they are important but also because I do not know them.
>
>>
>>> 3. How this relates to PWRHOLD coming from AP to the PMIC?
>>
>> No Idea right now about this right now. but will update you it I have
>> some thing.
>
> The PMIC receives two signals - PWREN and PWRHOLD. It seems strange
> that PMIC must override PWRHOLD... maybe there is an issue in
> mach-exynos code?
>
>>
>>> 4. Why this is needed only for this driver and only for this board (Odroid XU3)?
>>
>> Yes It could be generic not specific for the Odroid XU3 will correct
>> in next patch.
>
> Thanks,
> Best regards,
> Krzysztof

I don't have much technical knowlegde on the arch side PMIC.

Problem that I am trying to addressed is unclean shutdown. i.e. CPU is
still running in busy loop.
which leads external HDD not able to clean umount or power-off of the board.

[   27.427485] reboot: Power down
[   27.529236] Power down failed, please power off system manually.

My investigation lead to the following.
Reading the control register S2MPS11_REG_CTRL1 of s2mps11-pmic shown below.

[   27.411231] s2mps11-pmic s2mps11-pmic: reg value
16:00000000000000000000000000010000

clearing the bit of the control register S2MPS11_REG_CTRL1 which lead
to proper shutdown of board.

Please share you thought on this.

-Anand Moon

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Krzysztof Kozlowski]

+CC Marek


W dniu 11.07.2015 o 16:12, Anand Moon pisze:
> Hi Krzysztof,
> 
> On 24 June 2015 at 16:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>> 2015-06-24 19:32 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>> Hi Krzysztof,
>>>
>>> On 24 June 2015 at 13:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>>>> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>> Added .shutdown function to s2mps11 to help poweroff the board succefully.
>>>>
>>>> s/succefully/successfully/
>>>>
>>>>> The device drivers set the register to turn off the PMIC.
>>>>
>>>> Driver or drivers?
>>>>
>>>>>
>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>> ---
>>>>> Changes fixes the poweroff
>>>>> root@odroidxu3:~# poweroff
>>>>>
>>>>> Broadcast message from root@odroidxu3
>>>>>         (/dev/ttySAC2) at 13:08 ...
>>>>>
>>>>> The system is going down for power off NOW!
>>>>> root@odroidxu3:~# wait-for-state stop/waiting
>>>>>  * Stopping rsync daemon rsync                                           [ OK ]
>>>>>  * Stopping RDP Session manager                                          [ OK ]
>>>>>  * Stopping NTP server ntpd                                              [ OK ]
>>>>>  * Asking all remaining processes to terminate...                        [ OK ]
>>>>>  * All processes ended within 1 seconds...                               [ OK ]
>>>>> nm-dispatcher.action: Caught signal 15, shutting down...
>>>>> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>>>>>
>>>>> ModemManager[2134]: <info>  ModemManager is shut down
>>>>>
>>>>>  * Unmounting temporary filesystems...                                   [ OK ]
>>>>>  * Deactivating swap...                                                  [ OK ]
>>>>>  * Unmounting local filesystems...                                       [ OK ]
>>>>>  * Will now halt
>>>>> [  209.020280] reboot: Power down
>>>>> [  209.122039] Power down failed, please power off system manually.
>>>>> ---
>>>>>  drivers/regulator/s2mps11.c | 8 ++++++++
>>>>>  1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
>>>>> index ff82811..871f7b8 100644
>>>>> --- a/drivers/regulator/s2mps11.c
>>>>> +++ b/drivers/regulator/s2mps11.c
>>>>> @@ -1060,6 +1060,13 @@ out:
>>>>>         return ret;
>>>>>  }
>>>>>
>>>>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>>>>> +{
>>>>> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>>>>> +
>>>>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
>>>>
>>>> This looks odd to me and interesting in the same time...
>>>> 1. Why clearing all of the fields from the register? Don't you want to
>>>> clear only one of it?
>>>
>>> I have followed what the hardkernel source code point at this point.
>>> I will look into which bit need to clear/set to power off successful.
>>
>> Following other tree is not enough, even when it solves some one
>> problem. Drivers could be used on different boards, where hardkernel
>> patches were not tested. This driver is used on many boards so please
>> explain exactly what is the cause of problem, what have to be done and
>> how patch achieves this. Such explanation helps understanding impact
>> on other boards.
>>
>>>
>>>> 2. What exactly you want to do here? What is expected behaviour?
>>>
>>> When you power off the board dose not power off cleanly.
>>> [  209.122039] Power down failed, please power off system manually.
>>>
>>> After this changes Its power off the board. Leaving the board with
>>> solid red led blowing.
>>
>> You described observable issue, which is nice, but I am asking for
>> technical details. What exactly do you want to achieve here?
>> Technically. What is expected behaviour. In technical details, not
>> only observable. I am asking for these technical details not only
>> because they are important but also because I do not know them.
>>
>>>
>>>> 3. How this relates to PWRHOLD coming from AP to the PMIC?
>>>
>>> No Idea right now about this right now. but will update you it I have
>>> some thing.
>>
>> The PMIC receives two signals - PWREN and PWRHOLD. It seems strange
>> that PMIC must override PWRHOLD... maybe there is an issue in
>> mach-exynos code?
>>
>>>
>>>> 4. Why this is needed only for this driver and only for this board (Odroid XU3)?
>>>
>>> Yes It could be generic not specific for the Odroid XU3 will correct
>>> in next patch.
>>
>> Thanks,
>> Best regards,
>> Krzysztof
> 
> I don't have much technical knowlegde on the arch side PMIC.
> 
> Problem that I am trying to addressed is unclean shutdown. i.e. CPU is
> still running in busy loop.
> which leads external HDD not able to clean umount or power-off of the board.
> 
> [   27.427485] reboot: Power down
> [   27.529236] Power down failed, please power off system manually.
> 
> My investigation lead to the following.
> Reading the control register S2MPS11_REG_CTRL1 of s2mps11-pmic shown below.
> 
> [   27.411231] s2mps11-pmic s2mps11-pmic: reg value
> 16:00000000000000000000000000010000

This means the PWRHOLD bit is high.

> 
> clearing the bit of the control register S2MPS11_REG_CTRL1 which lead
> to proper shutdown of board.
> 
> Please share you thought on this.

The power off should be initiated from application processor, not PMIC.
The AP should set PWRHOLD pin (XPSHOLD on AP) to low which will turn off
the power.

This works for other Exynos SoCs.

Power off should not be performed by driver's shutdown callback because
there is no guarantee that this is the last shutdown callback. Maybe
other shutdowns would be executed after it?

The power should go off from exynos_power_off() method in pmu.c. Please
start with that. Why disabling PS_HOLD does not turn off the power?


Marek (cc-ed), do you have any ideas why Odroid XU3 fails to power down?


Best regards,
Krzysztof

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Marek Szyprowski]

Hello,

On 2015-07-11 09:57, Krzysztof Kozlowski wrote:
> +CC Marek
>
>
> W dniu 11.07.2015 o 16:12, Anand Moon pisze:
>> Hi Krzysztof,
>>
>> On 24 June 2015 at 16:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>>> 2015-06-24 19:32 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>> Hi Krzysztof,
>>>>
>>>> On 24 June 2015 at 13:27, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>>>>> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>>> Added .shutdown function to s2mps11 to help poweroff the board succefully.
>>>>> s/succefully/successfully/
>>>>>
>>>>>> The device drivers set the register to turn off the PMIC.
>>>>> Driver or drivers?
>>>>>
>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>> ---
>>>>>> Changes fixes the poweroff
>>>>>> root@odroidxu3:~# poweroff
>>>>>>
>>>>>> Broadcast message from root@odroidxu3
>>>>>>          (/dev/ttySAC2) at 13:08 ...
>>>>>>
>>>>>> The system is going down for power off NOW!
>>>>>> root@odroidxu3:~# wait-for-state stop/waiting
>>>>>>   * Stopping rsync daemon rsync                                           [ OK ]
>>>>>>   * Stopping RDP Session manager                                          [ OK ]
>>>>>>   * Stopping NTP server ntpd                                              [ OK ]
>>>>>>   * Asking all remaining processes to terminate...                        [ OK ]
>>>>>>   * All processes ended within 1 seconds...                               [ OK ]
>>>>>> nm-dispatcher.action: Caught signal 15, shutting down...
>>>>>> ModemManager[2134]: <warn>  Could not acquire the 'org.freedesktop.ModemManager1' service name
>>>>>>
>>>>>> ModemManager[2134]: <info>  ModemManager is shut down
>>>>>>
>>>>>>   * Unmounting temporary filesystems...                                   [ OK ]
>>>>>>   * Deactivating swap...                                                  [ OK ]
>>>>>>   * Unmounting local filesystems...                                       [ OK ]
>>>>>>   * Will now halt
>>>>>> [  209.020280] reboot: Power down
>>>>>> [  209.122039] Power down failed, please power off system manually.
>>>>>> ---
>>>>>>   drivers/regulator/s2mps11.c | 8 ++++++++
>>>>>>   1 file changed, 8 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
>>>>>> index ff82811..871f7b8 100644
>>>>>> --- a/drivers/regulator/s2mps11.c
>>>>>> +++ b/drivers/regulator/s2mps11.c
>>>>>> @@ -1060,6 +1060,13 @@ out:
>>>>>>          return ret;
>>>>>>   }
>>>>>>
>>>>>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>>>>>> +{
>>>>>> +       struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>>>>>> +
>>>>>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1, 0xff, 0x00);
>>>>> This looks odd to me and interesting in the same time...
>>>>> 1. Why clearing all of the fields from the register? Don't you want to
>>>>> clear only one of it?
>>>> I have followed what the hardkernel source code point at this point.
>>>> I will look into which bit need to clear/set to power off successful.
>>> Following other tree is not enough, even when it solves some one
>>> problem. Drivers could be used on different boards, where hardkernel
>>> patches were not tested. This driver is used on many boards so please
>>> explain exactly what is the cause of problem, what have to be done and
>>> how patch achieves this. Such explanation helps understanding impact
>>> on other boards.
>>>
>>>>> 2. What exactly you want to do here? What is expected behaviour?
>>>> When you power off the board dose not power off cleanly.
>>>> [  209.122039] Power down failed, please power off system manually.
>>>>
>>>> After this changes Its power off the board. Leaving the board with
>>>> solid red led blowing.
>>> You described observable issue, which is nice, but I am asking for
>>> technical details. What exactly do you want to achieve here?
>>> Technically. What is expected behaviour. In technical details, not
>>> only observable. I am asking for these technical details not only
>>> because they are important but also because I do not know them.
>>>
>>>>> 3. How this relates to PWRHOLD coming from AP to the PMIC?
>>>> No Idea right now about this right now. but will update you it I have
>>>> some thing.
>>> The PMIC receives two signals - PWREN and PWRHOLD. It seems strange
>>> that PMIC must override PWRHOLD... maybe there is an issue in
>>> mach-exynos code?
>>>
>>>>> 4. Why this is needed only for this driver and only for this board (Odroid XU3)?
>>>> Yes It could be generic not specific for the Odroid XU3 will correct
>>>> in next patch.
>>> Thanks,
>>> Best regards,
>>> Krzysztof
>> I don't have much technical knowlegde on the arch side PMIC.
>>
>> Problem that I am trying to addressed is unclean shutdown. i.e. CPU is
>> still running in busy loop.
>> which leads external HDD not able to clean umount or power-off of the board.
>>
>> [   27.427485] reboot: Power down
>> [   27.529236] Power down failed, please power off system manually.
>>
>> My investigation lead to the following.
>> Reading the control register S2MPS11_REG_CTRL1 of s2mps11-pmic shown below.
>>
>> [   27.411231] s2mps11-pmic s2mps11-pmic: reg value
>> 16:00000000000000000000000000010000
> This means the PWRHOLD bit is high.
>
>> clearing the bit of the control register S2MPS11_REG_CTRL1 which lead
>> to proper shutdown of board.
>>
>> Please share you thought on this.
> The power off should be initiated from application processor, not PMIC.
> The AP should set PWRHOLD pin (XPSHOLD on AP) to low which will turn off
> the power.
>
> This works for other Exynos SoCs.
>
> Power off should not be performed by driver's shutdown callback because
> there is no guarantee that this is the last shutdown callback. Maybe
> other shutdowns would be executed after it?
>
> The power should go off from exynos_power_off() method in pmu.c. Please
> start with that. Why disabling PS_HOLD does not turn off the power?
>
>
> Marek (cc-ed), do you have any ideas why Odroid XU3 fails to power down?

I have no idea why XU3 doesn't power down. I think it should be safe to
implement reset handler in PMIC driver and do all needed stuff there instead
of device .shutdown callback.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] regulator: s2mps11: Added shutdown function to poweroff Odroid-XU3

[Krzysztof Kozlowski]

On 14.07.2015 18:33, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-07-11 09:57, Krzysztof Kozlowski wrote:
>> +CC Marek
>>
>>
>> W dniu 11.07.2015 o 16:12, Anand Moon pisze:
>>> Hi Krzysztof,
>>>
>>> On 24 June 2015 at 16:27, Krzysztof Kozlowski
>>> <k.kozlowski@samsung.com> wrote:
>>>> 2015-06-24 19:32 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>> Hi Krzysztof,
>>>>>
>>>>> On 24 June 2015 at 13:27, Krzysztof Kozlowski
>>>>> <k.kozlowski@samsung.com> wrote:
>>>>>> 2015-06-24 16:16 GMT+09:00 Anand Moon <linux.amoon@gmail.com>:
>>>>>>> Added .shutdown function to s2mps11 to help poweroff the board
>>>>>>> succefully.
>>>>>> s/succefully/successfully/
>>>>>>
>>>>>>> The device drivers set the register to turn off the PMIC.
>>>>>> Driver or drivers?
>>>>>>
>>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>>> ---
>>>>>>> Changes fixes the poweroff
>>>>>>> root@odroidxu3:~# poweroff
>>>>>>>
>>>>>>> Broadcast message from root@odroidxu3
>>>>>>>          (/dev/ttySAC2) at 13:08 ...
>>>>>>>
>>>>>>> The system is going down for power off NOW!
>>>>>>> root@odroidxu3:~# wait-for-state stop/waiting
>>>>>>>   * Stopping rsync daemon
>>>>>>> rsync                                           [ OK ]
>>>>>>>   * Stopping RDP Session
>>>>>>> manager                                          [ OK ]
>>>>>>>   * Stopping NTP server
>>>>>>> ntpd                                              [ OK ]
>>>>>>>   * Asking all remaining processes to
>>>>>>> terminate...                        [ OK ]
>>>>>>>   * All processes ended within 1
>>>>>>> seconds...                               [ OK ]
>>>>>>> nm-dispatcher.action: Caught signal 15, shutting down...
>>>>>>> ModemManager[2134]: <warn>  Could not acquire the
>>>>>>> 'org.freedesktop.ModemManager1' service name
>>>>>>>
>>>>>>> ModemManager[2134]: <info>  ModemManager is shut down
>>>>>>>
>>>>>>>   * Unmounting temporary
>>>>>>> filesystems...                                   [ OK ]
>>>>>>>   * Deactivating
>>>>>>> swap...                                                  [ OK ]
>>>>>>>   * Unmounting local
>>>>>>> filesystems...                                       [ OK ]
>>>>>>>   * Will now halt
>>>>>>> [  209.020280] reboot: Power down
>>>>>>> [  209.122039] Power down failed, please power off system manually.
>>>>>>> ---
>>>>>>>   drivers/regulator/s2mps11.c | 8 ++++++++
>>>>>>>   1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/regulator/s2mps11.c
>>>>>>> b/drivers/regulator/s2mps11.c
>>>>>>> index ff82811..871f7b8 100644
>>>>>>> --- a/drivers/regulator/s2mps11.c
>>>>>>> +++ b/drivers/regulator/s2mps11.c
>>>>>>> @@ -1060,6 +1060,13 @@ out:
>>>>>>>          return ret;
>>>>>>>   }
>>>>>>>
>>>>>>> +static void s2mps11_pmic_shutdown(struct platform_device *pdev)
>>>>>>> +{
>>>>>>> +       struct sec_pmic_dev *iodev =
>>>>>>> dev_get_drvdata(pdev->dev.parent);
>>>>>>> +
>>>>>>> +       regmap_update_bits(iodev->regmap_pmic, S2MPS11_REG_CTRL1,
>>>>>>> 0xff, 0x00);
>>>>>> This looks odd to me and interesting in the same time...
>>>>>> 1. Why clearing all of the fields from the register? Don't you
>>>>>> want to
>>>>>> clear only one of it?
>>>>> I have followed what the hardkernel source code point at this point.
>>>>> I will look into which bit need to clear/set to power off successful.
>>>> Following other tree is not enough, even when it solves some one
>>>> problem. Drivers could be used on different boards, where hardkernel
>>>> patches were not tested. This driver is used on many boards so please
>>>> explain exactly what is the cause of problem, what have to be done and
>>>> how patch achieves this. Such explanation helps understanding impact
>>>> on other boards.
>>>>
>>>>>> 2. What exactly you want to do here? What is expected behaviour?
>>>>> When you power off the board dose not power off cleanly.
>>>>> [  209.122039] Power down failed, please power off system manually.
>>>>>
>>>>> After this changes Its power off the board. Leaving the board with
>>>>> solid red led blowing.
>>>> You described observable issue, which is nice, but I am asking for
>>>> technical details. What exactly do you want to achieve here?
>>>> Technically. What is expected behaviour. In technical details, not
>>>> only observable. I am asking for these technical details not only
>>>> because they are important but also because I do not know them.
>>>>
>>>>>> 3. How this relates to PWRHOLD coming from AP to the PMIC?
>>>>> No Idea right now about this right now. but will update you it I have
>>>>> some thing.
>>>> The PMIC receives two signals - PWREN and PWRHOLD. It seems strange
>>>> that PMIC must override PWRHOLD... maybe there is an issue in
>>>> mach-exynos code?
>>>>
>>>>>> 4. Why this is needed only for this driver and only for this board
>>>>>> (Odroid XU3)?
>>>>> Yes It could be generic not specific for the Odroid XU3 will correct
>>>>> in next patch.
>>>> Thanks,
>>>> Best regards,
>>>> Krzysztof
>>> I don't have much technical knowlegde on the arch side PMIC.
>>>
>>> Problem that I am trying to addressed is unclean shutdown. i.e. CPU is
>>> still running in busy loop.
>>> which leads external HDD not able to clean umount or power-off of the
>>> board.
>>>
>>> [   27.427485] reboot: Power down
>>> [   27.529236] Power down failed, please power off system manually.
>>>
>>> My investigation lead to the following.
>>> Reading the control register S2MPS11_REG_CTRL1 of s2mps11-pmic shown
>>> below.
>>>
>>> [   27.411231] s2mps11-pmic s2mps11-pmic: reg value
>>> 16:00000000000000000000000000010000
>> This means the PWRHOLD bit is high.
>>
>>> clearing the bit of the control register S2MPS11_REG_CTRL1 which lead
>>> to proper shutdown of board.
>>>
>>> Please share you thought on this.
>> The power off should be initiated from application processor, not PMIC.
>> The AP should set PWRHOLD pin (XPSHOLD on AP) to low which will turn off
>> the power.
>>
>> This works for other Exynos SoCs.
>>
>> Power off should not be performed by driver's shutdown callback because
>> there is no guarantee that this is the last shutdown callback. Maybe
>> other shutdowns would be executed after it?
>>
>> The power should go off from exynos_power_off() method in pmu.c. Please
>> start with that. Why disabling PS_HOLD does not turn off the power?
>>
>>
>> Marek (cc-ed), do you have any ideas why Odroid XU3 fails to power down?
> 
> I have no idea why XU3 doesn't power down. I think it should be safe to
> implement reset handler in PMIC driver and do all needed stuff there
> instead
> of device .shutdown callback.

Thanks for the hints. I'll try to find some spare time to compare the
XU3 with other boards using this PMIC family (eg. Gear 2). Maybe it is
connected in a different way or - as you said to me other time -
bootloader configures it differently. I am using bootloader from Hardkernel.

Best regards,
Krzysztof