[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pavel Fedin]

Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart
code into pmu driver") PMU support is required in order for the reboot to
work. Unfortunately, there is currently no PMU support for 5410.

This patch adds exynos5410-pmu to the list of recognized devices. It is
okay for the code to have empty struct exynos_pmu_data, in this case only
minimal support is enabled. This is enough for SWRESET to work.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 arch/arm/mach-exynos/pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index de68938..7b9e33c 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
 	.powerdown_conf	= exynos5_powerdown_conf,
 };
 
+static struct exynos_pmu_data exynos5410_pmu_data = {
+	/* Only SWRESET is currently supported on 5410 */
+};
+
 static struct exynos_pmu_data exynos5420_pmu_data = {
 	.pmu_config	= exynos5420_pmu_config,
 	.pmu_init	= exynos5420_pmu_init,
@@ -934,6 +938,9 @@ static const struct of_device_id exynos_pmu_of_device_ids[] = {
 		.compatible = "samsung,exynos5250-pmu",
 		.data = &exynos5250_pmu_data,
 	}, {
+		.compatible = "samsung,exynos5410-pmu",
+		.data = &exynos5410_pmu_data,
+	}, {
 		.compatible = "samsung,exynos5420-pmu",
 		.data = &exynos5420_pmu_data,
 	},
-- 
2.4.4

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Krzysztof Kozlowski]

On 28.10.2015 18:36, Pavel Fedin wrote:
> Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart

This should be sufficient (although full is okay as well):
$ git config core.abbrev 12

> code into pmu driver") PMU support is required in order for the reboot to
> work. Unfortunately, there is currently no PMU support for 5410.
> 
> This patch adds exynos5410-pmu to the list of recognized devices. It is
> okay for the code to have empty struct exynos_pmu_data, in this case only
> minimal support is enabled. This is enough for SWRESET to work.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>

This definitely looks like a fix for a regression, so:
Cc: <stable@vger.kernel.org>
Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")

> ---
>  arch/arm/mach-exynos/pmu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index de68938..7b9e33c 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
>  	.powerdown_conf	= exynos5_powerdown_conf,
>  };
>  
> +static struct exynos_pmu_data exynos5410_pmu_data = {
> +	/* Only SWRESET is currently supported on 5410 */
> +};

static const struct

With these changes:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


Best regards,
Krzysztof

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pankaj Dubey]

Hi Pavel,

On 29 October 2015 at 07:11, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 28.10.2015 18:36, Pavel Fedin wrote:
>> Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart
>
> This should be sufficient (although full is okay as well):
> $ git config core.abbrev 12
>
>> code into pmu driver") PMU support is required in order for the reboot to
>> work. Unfortunately, there is currently no PMU support for 5410.
>>
>> This patch adds exynos5410-pmu to the list of recognized devices. It is
>> okay for the code to have empty struct exynos_pmu_data, in this case only
>> minimal support is enabled. This is enough for SWRESET to work.
>>
>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>
> This definitely looks like a fix for a regression, so:
> Cc: <stable@vger.kernel.org>
> Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")
>
>> ---
>>  arch/arm/mach-exynos/pmu.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
>> index de68938..7b9e33c 100644
>> --- a/arch/arm/mach-exynos/pmu.c
>> +++ b/arch/arm/mach-exynos/pmu.c
>> @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
>>       .powerdown_conf = exynos5_powerdown_conf,
>>  };
>>
>> +static struct exynos_pmu_data exynos5410_pmu_data = {
>> +     /* Only SWRESET is currently supported on 5410 */
>> +};
>
> static const struct
>
> With these changes:
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
>

Patch looks ok, but after Alim's patch series [1] to handle poweroff
and reboot via generic syscon based driver gets merged, this will
become redundant entry into PMU driver in the absence of any PMU  data
for Exynos5410. Alim has taken care of adding power-off and reboot
device node in exynos5410.dtsi [2].

[1]: http://www.spinics.net/lists/devicetree/msg98858.html
[2]: http://www.spinics.net/lists/devicetree/msg98862.html

Thanks,
Pankaj Dubey

> Best regards,
> Krzysztof
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Krzysztof Kozlowski]

On 29.10.2015 11:06, Pankaj Dubey wrote:
> Hi Pavel,
> 
> On 29 October 2015 at 07:11, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> On 28.10.2015 18:36, Pavel Fedin wrote:
>>> Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart
>>
>> This should be sufficient (although full is okay as well):
>> $ git config core.abbrev 12
>>
>>> code into pmu driver") PMU support is required in order for the reboot to
>>> work. Unfortunately, there is currently no PMU support for 5410.
>>>
>>> This patch adds exynos5410-pmu to the list of recognized devices. It is
>>> okay for the code to have empty struct exynos_pmu_data, in this case only
>>> minimal support is enabled. This is enough for SWRESET to work.
>>>
>>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>>
>> This definitely looks like a fix for a regression, so:
>> Cc: <stable@vger.kernel.org>
>> Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")
>>
>>> ---
>>>  arch/arm/mach-exynos/pmu.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
>>> index de68938..7b9e33c 100644
>>> --- a/arch/arm/mach-exynos/pmu.c
>>> +++ b/arch/arm/mach-exynos/pmu.c
>>> @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
>>>       .powerdown_conf = exynos5_powerdown_conf,
>>>  };
>>>
>>> +static struct exynos_pmu_data exynos5410_pmu_data = {
>>> +     /* Only SWRESET is currently supported on 5410 */
>>> +};
>>
>> static const struct
>>
>> With these changes:
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>>
> 
> Patch looks ok, but after Alim's patch series [1] to handle poweroff
> and reboot via generic syscon based driver gets merged, this will
> become redundant entry into PMU driver in the absence of any PMU  data
> for Exynos5410. Alim has taken care of adding power-off and reboot
> device node in exynos5410.dtsi [2].
> 
> [1]: http://www.spinics.net/lists/devicetree/msg98858.html
> [2]: http://www.spinics.net/lists/devicetree/msg98862.html
> 

Yes, current patch makes sense only if it gets into fixes for current RC
and then backported to stable kernels. Alim's patchset won't be backported.

Best regards,
Krzysztof

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pavel Fedin]

 Hello!

> Patch looks ok, but after Alim's patch series [1] to handle poweroff
> and reboot via generic syscon based driver gets merged, this will
> become redundant entry into PMU driver in the absence of any PMU  data
> for Exynos5410. Alim has taken care of adding power-off and reboot
> device node in exynos5410.dtsi [2].
> 
> [1]: http://www.spinics.net/lists/devicetree/msg98858.html
> [2]: http://www.spinics.net/lists/devicetree/msg98862.html

 I looked at those patches, IMHO they are too large for doing too small thing. What they do is basically describing two registers one-by-one and adding a separate per-register driver. Isn't it too much just for pushing a reset pin?
 May be we should just invent some generic "compatible" string, like "samsung,exynos-pmu" (or "Samsung,exynos-pmu-generic", if you want to be more specific), which would do the same as my patch currently does? This would be only several lines of code, achieving the same result.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pankaj Dubey]

Hi Pavel,

On Thursday 29 October 2015 12:40 PM, Pavel Fedin wrote:
>   Hello!
>
>> Patch looks ok, but after Alim's patch series [1] to handle poweroff
>> and reboot via generic syscon based driver gets merged, this will
>> become redundant entry into PMU driver in the absence of any PMU  data
>> for Exynos5410. Alim has taken care of adding power-off and reboot
>> device node in exynos5410.dtsi [2].
>>
>> [1]: http://www.spinics.net/lists/devicetree/msg98858.html
>> [2]: http://www.spinics.net/lists/devicetree/msg98862.html
>
>   I looked at those patches, IMHO they are too large for doing too small thing. What they do is basically describing two registers one-by-one and adding a separate per-register driver. Isn't it too much just for pushing a reset pin?
>   May be we should just invent some generic "compatible" string, like "samsung,exynos-pmu" (or "Samsung,exynos-pmu-generic", if you want to be more specific), which would do the same as my patch currently does? This would be only several lines of code, achieving the same result.
>

These patches are not adding any new code, except DT entry. Infact 
restart handler will be removed from PMU driver and we will be using 
generic syscon based implementation for reboot, which is already present 
in Linux kernel. Thus effectively reducing LoC. Only we need to add 
device node entries in respective DT files, and this is being done for 
all Exynos SoCs including exynos5410.

Thanks,
Pankaj Dubey
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pavel Fedin]

 Hello!

> These patches are not adding any new code, except DT entry. Infact
> restart handler will be removed from PMU driver and we will be using
> generic syscon based implementation for reboot, which is already present
> in Linux kernel.

 Ah, got it. So syscon is a very generic thing, allowing to describe whatever reset register for whatever board, since we are specifying both address and value to write there. Is my understanding correct? If so, then, OK, you can do this too.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH] ARM: exynos: Bring back reboot on Exynos5410

[Pankaj Dubey]

On Thursday 29 October 2015 02:13 PM, Pavel Fedin wrote:
>   Hello!
>
>> These patches are not adding any new code, except DT entry. Infact
>> restart handler will be removed from PMU driver and we will be using
>> generic syscon based implementation for reboot, which is already present
>> in Linux kernel.
>
>   Ah, got it. So syscon is a very generic thing, allowing to describe whatever reset register for whatever board, since we are specifying both address and value to write there. Is my understanding correct? If so, then, OK, you can do this too.
>

Yes, syscon-reboot does exactly same. We just need to provide 
"syscon-provider" in our case it's PMU device node, register/offset and 
value to be written, and syscon-reboot driver will take care from 
registered restart-handler.

Thanks,
Pankaj Dubey
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>