Skip to contentSkip to conversation

ARM: exynos: Bring back reboot on Exynos5410

PATCH Superseded Pavel Fedin 3 participants 8 messages Oct 28, 2015 – Oct 29, 2015

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm Samsung Exynos

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Pavel Fedin

author
#0 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

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 <[email protected]>
---
 arch/arm/mach-exynos/pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-exynos/pmu.c +7 -0
index de68938..7b9e33c 100644
@@ -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,
 	},
Signature · 1 line
2.4.4
#1 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Quoted 1 line · Pavel Fedin

Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart

This should be sufficient (although full is okay as well):

$ git config core.abbrev 12
Quoted 8 lines

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 <[email protected]>

This definitely looks like a fix for a regression, so:

Cc <[email protected]>
Fixes 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")
Quoted 9 lines
---
 arch/arm/mach-exynos/pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-exynos/pmu.c +0 -0
index de68938..7b9e33c 100644

arch/arm/mach-exynos/pmu.c static const struct exynos_pmu_data exynos5250_pmu_data = { · line 913

Quoted 6 lines
arch/arm/mach-exynos/pmu.c +3 -0
@@ -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 <[email protected]>

Best regards,
Krzysztof

#2 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Hi Pavel,

Quoted 31 lines · Krzysztof Kozlowski
Quoted 1 line · Pavel Fedin

Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart

This should be sufficient (although full is okay as well):

$ git config core.abbrev 12
Quoted 8 lines

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 <[email protected]>

This definitely looks like a fix for a regression, so:

Cc <[email protected]>
Fixes 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")
Quoted 12 lines
---
 arch/arm/mach-exynos/pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-exynos/pmu.c +0 -0
index de68938..7b9e33c 100644
@@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
      .powerdown_conf = exynos5_powerdown_conf,
 };
Quoted 10 lines

arch/arm/mach-exynos/pmu.c static const struct exynos_pmu_data exynos5250_pmu_data = { · line 913

Quoted 3 lines
arch/arm/mach-exynos/pmu.c +3 -0
+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 <[email protected]>

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

Quoted 7 lines

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

#3 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Quoted 46 lines · Pankaj Dubey

Hi Pavel,

Quoted 31 lines · Krzysztof Kozlowski
Quoted 1 line · Pavel Fedin

Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart

This should be sufficient (although full is okay as well):

$ git config core.abbrev 12
Quoted 8 lines

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 <[email protected]>

This definitely looks like a fix for a regression, so:

Cc <[email protected]>
Fixes 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver")
Quoted 12 lines
---
 arch/arm/mach-exynos/pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-exynos/pmu.c +0 -0
index de68938..7b9e33c 100644
@@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = {
      .powerdown_conf = exynos5_powerdown_conf,
 };
Quoted 10 lines

arch/arm/mach-exynos/pmu.c static const struct exynos_pmu_data exynos5250_pmu_data = { · line 913

Quoted 3 lines
arch/arm/mach-exynos/pmu.c +3 -0
+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 <[email protected]>
Quoted 9 lines

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

author
#4 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

In reply to Pankaj Dubey

Hello!

Quoted 8 lines

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

#5 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Hi Pavel,

Quoted 8 lines · Pavel Fedin

Hello!

Quoted 6 lines

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].

Quoted 6 lines
Quoted 2 lines

[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

Quoted 7 lines

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

author
#6 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Hello!

Quoted 4 lines

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

#7 lore raw

PATCH 1/1 ARM: exynos: Bring back reboot on Exynos5410

Quoted 9 lines · Pavel Fedin

Hello!

Quoted 4 lines

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

Quoted 7 lines

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