Switch to generic syscon regmap based drivers

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos3250 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
arch/arm/boot/dts/exynos3250.dtsi | 14 ++++++++++++++

  1 file changed, 14 insertions(+)

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos3250 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/boot/dts/exynos3250.dtsi +4 -0

index 033def482fc3..af5d9ad4c7b7 100644
@@ -152,6 +152,20 @@
 			interrupt-parent = <&gic>;
 		};
 
+		poweroff: syscon-poweroff {
+			compatible = "syscon-poweroff";
+			regmap = <&pmu_system_controller>;
+			offset = <0x330C>;

Hello Alim,

Quoted 20 lines · Alim Akhtar

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos3250 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
  arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++++
  1 file changed, 14 insertions(+)

arch/arm/boot/dts/exynos3250.dtsi +4 -0

index 033def482fc3..af5d9ad4c7b7 100644
@@ -152,6 +152,20 @@
  			interrupt-parent = <&gic>;
  		};
 
+		poweroff: syscon-poweroff {
+			compatible = "syscon-poweroff";
+			regmap = <&pmu_system_controller>;
+			offset = <0x330C>;

arch/arm/boot/dts/exynos3250.dtsi line 168

Quoted 10 lines

arch/arm/boot/dts/exynos3250.dtsi +10 -0

+			mask = <0x5200>;
+		};
+
+		reboot: syscon-reboot {
+			compatible = "syscon-reboot";
+			regmap = <&pmu_system_controller>;
+			offset = <0x0400>;
+			mask = <0x1>;
+		};
+

I don't have a Exynos3250 manual but I guess 0x330C is also named PS_HOLD_CONTROL and 0x400 is SWRESET as the other Exynos SoCs.

I wonder if a macro could be used instead of magic numbers or at least have a comment next to the offset field.

The patch looks good to me though and a comment can be added as a follow up so:

Reviewed-by Javier Martinez Canillas <[email protected]>

Best regards,

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos4 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
arch/arm/boot/dts/exynos4.dtsi | 14 ++++++++++++++

  1 file changed, 14 insertions(+)

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos4 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/boot/dts/exynos4.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/boot/dts/exynos4.dtsi +4 -0

index 98c0a368b777..79015320cdb6 100644
@@ -158,6 +158,20 @@
 		interrupt-parent = <&gic>;
 	};
 
+	poweroff: syscon-poweroff {
+		compatible = "syscon-poweroff";
+		regmap = <&pmu_system_controller>;
+		offset = <0x330C>;

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos5 SoCs.

Signed-off-by Alim Akhtar <[email protected]>

---
arch/arm/boot/dts/exynos5.dtsi | 14 ++++++++++++++

  1 file changed, 14 insertions(+)

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos5 SoCs.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/boot/dts/exynos5.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/boot/dts/exynos5.dtsi +4 -0

index 110dbd4fb884..1313777618b7 100644
@@ -88,6 +88,20 @@
 		status = "disabled";
 	};
 
+	poweroff: syscon-poweroff {
+		compatible = "syscon-poweroff";
+		regmap = <&pmu_system_controller>;
+		offset = <0x330C>;

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos5410 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
arch/arm/boot/dts/exynos5410.dtsi | 14 ++++++++++++++

  1 file changed, 14 insertions(+)

This patch adds syscon-{reboot, poweroff} nodes to allow the generic syscon-{reboot, poweroff} driver to reset/poweroff exynos5410 SoC.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/boot/dts/exynos5410.dtsi +4 -0

index 731eefd23fa9..b9bef8b5ffc9 100644
@@ -102,6 +102,20 @@
 			reg = <0x10040000 0x5000>;
 		};
 
+		poweroff: syscon-poweroff {
+			compatible = "syscon-poweroff";
+			regmap = <&pmu_system_controller>;
+			offset = <0x330C>;

make savedefconfig result in some difference, lets normalize the defconfig.

Signed-off-by Alim Akhtar <[email protected]>

---

 arch/arm/configs/exynos_defconfig |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

arch/arm/configs/exynos_defconfig +0 -0

index 1ff2bfa2e183..3349713e6c63 100644

arch/arm/configs/exynos_defconfig +0 -0

 CONFIG_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
 CONFIG_MFD_CROS_EC=y

@@ -127,14 +125,14 @@ CONFIG_REGULATOR_S2MPS11=y
 CONFIG_REGULATOR_S5M8767=y
 CONFIG_REGULATOR_TPS65090=y
 CONFIG_DRM=y

arch/arm/configs/exynos_defconfig +0 -1

 CONFIG_FB_SIMPLE=y
 CONFIG_EXYNOS_VIDEO=y
 CONFIG_EXYNOS_MIPI_DSI=y

@@ -175,7 +173,6 @@ CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_S3C=y
 CONFIG_DMADEVICES=y
 CONFIG_PL330_DMA=y
-CONFIG_CHROME_PLATFORMS=y
 CONFIG_CROS_EC_CHARDEV=y
 CONFIG_COMMON_CLK_MAX77686=y
 CONFIG_COMMON_CLK_MAX77802=y

Hello Alim,

Quoted 5 lines · Alim Akhtar

make savedefconfig result in some difference, lets normalize the defconfig.

Signed-off-by Alim Akhtar <[email protected]>

---

Did you make savedefconfig against v4.3-rc6 or tot linux-next?

I remember we had issues in the past due savedefconfig made against linux-next but then some of the patches in linux-next not making it to the next release. So I think it should be good to mention what's the base used for this patch.

Quoted 8 lines

 arch/arm/configs/exynos_defconfig |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

arch/arm/configs/exynos_defconfig +0 -0

index 1ff2bfa2e183..3349713e6c63 100644

arch/arm/configs/exynos_defconfig CONFIG_SENSORS_LM90=y · line 105

Quoted 7 lines

arch/arm/configs/exynos_defconfig +0 -2

@@ -99,10 +99,8 @@ CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_NTC_THERMISTOR=y
 CONFIG_SENSORS_PWM_FAN=y
 CONFIG_SENSORS_INA2XX=y
-CONFIG_THERMAL=y
 CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
-CONFIG_EXYNOS_THERMAL=y

I would prefer to split all the changes that removes symbols into a separate patch explaining why these are not needed anymore (i.e: it is selected now by symbol $foo).

Quoted 7 lines

arch/arm/configs/exynos_defconfig +0 -0

 CONFIG_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
 CONFIG_MFD_CROS_EC=y

@@ -127,14 +125,14 @@ CONFIG_REGULATOR_S2MPS11=y
 CONFIG_REGULATOR_S5M8767=y
 CONFIG_REGULATOR_TPS65090=y
 CONFIG_DRM=y

arch/arm/configs/exynos_defconfig CONFIG_REGULATOR_S2MPS11=y · line 135

Quoted 10 lines

arch/arm/configs/exynos_defconfig +2 -2

-CONFIG_DRM_NXP_PTN3460=y
-CONFIG_DRM_PARADE_PS8622=y
 CONFIG_DRM_EXYNOS=y
 CONFIG_DRM_EXYNOS_FIMD=y
 CONFIG_DRM_EXYNOS_DSI=y
 CONFIG_DRM_EXYNOS_HDMI=y
 CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0=y
+CONFIG_DRM_NXP_PTN3460=y
+CONFIG_DRM_PARADE_PS8622=y

Moving these DRM bridge symbols to match savedefconfig is ok IMHO.

Quoted 13 lines

arch/arm/configs/exynos_defconfig +0 -1

 CONFIG_FB_SIMPLE=y
 CONFIG_EXYNOS_VIDEO=y
 CONFIG_EXYNOS_MIPI_DSI=y

@@ -175,7 +173,6 @@ CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_S3C=y
 CONFIG_DMADEVICES=y
 CONFIG_PL330_DMA=y
-CONFIG_CHROME_PLATFORMS=y
 CONFIG_CROS_EC_CHARDEV=y
 CONFIG_COMMON_CLK_MAX77686=y
 CONFIG_COMMON_CLK_MAX77802=y

arch/arm/configs/exynos_defconfig CONFIG_PWM_SAMSUNG=y · line 192

Quoted 7 lines

arch/arm/configs/exynos_defconfig +0 -1

@@ -190,7 +187,6 @@ CONFIG_PWM_SAMSUNG=y
 CONFIG_PHY_EXYNOS5250_SATA=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
-CONFIG_EXT4_FS=y
 CONFIG_MSDOS_FS=y
 CONFIG_VFAT_FS=y
 CONFIG_TMPFS=y

Same comment than above, I would prefer these to be split.

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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

Hi Javier,

Quoted 11 lines · Javier Martinez Canillas

Hello Alim,

Quoted 5 lines · Alim Akhtar

make savedefconfig result in some difference, lets normalize the defconfig.

Signed-off-by Alim Akhtar <[email protected]>

---

Did you make savedefconfig against v4.3-rc6 or tot linux-next?

These are based on v4.3-rc5.

Quoted 5 lines

I remember we had issues in the past due savedefconfig made against linux-next but then some of the patches in linux-next not making it to the next release. So I think it should be good to mention what's the base used for this patch.

Ah, ok I didn't follow that, will check again on Linux-next as well as

on 4.3-rc5

Quoted 20 lines

Quoted 8 lines

 arch/arm/configs/exynos_defconfig |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

arch/arm/configs/exynos_defconfig +0 -0

index 1ff2bfa2e183..3349713e6c63 100644

arch/arm/configs/exynos_defconfig CONFIG_SENSORS_LM90=y · line 105

Quoted 7 lines

arch/arm/configs/exynos_defconfig +0 -2

@@ -99,10 +99,8 @@ CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_NTC_THERMISTOR=y
 CONFIG_SENSORS_PWM_FAN=y
 CONFIG_SENSORS_INA2XX=y
-CONFIG_THERMAL=y
 CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
-CONFIG_EXYNOS_THERMAL=y

I would prefer to split all the changes that removes symbols into a separate patch explaining why these are not needed anymore (i.e: it is selected now by symbol $foo).

Ok ..let me rebase my patches and check these config changes again....

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

arch/arm/mach-exynos/pmu.c +0 -43

index de68938ee6aa..393c04aa727e 100644
@@ -14,9 +14,6 @@
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
-
 
 #include "exynos-pmu.h"
 #include "regs-pmu.h"

@@ -681,23 +678,6 @@ static unsigned int const exynos5420_list_disable_pmu_reg[] = {
 	EXYNOS5420_CMU_RESET_FSYS_SYS_PWR_REG,
 };
 
-static void exynos_power_off(void)
-{
-	unsigned int tmp;
-
-	pr_info("Power down.\n");
-	tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
-	tmp ^= (1 << 8);
-	pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
-
-	/* Wait a little so we don't give a false warning below */
-	mdelay(100);
-
-	pr_err("Power down failed, please power off system manually.\n");
-	while (1)
-		;
-}
-
 static void exynos5420_powerdown_conf(enum sys_powerdown mode)
 {
 	u32 this_cluster;

@@ -875,14 +855,6 @@ static void exynos5420_pmu_init(void)
 	pr_info("EXYNOS5420 PMU initialized\n");
 }
 
-static int pmu_restart_notify(struct notifier_block *this,
-		unsigned long code, void *unused)
-{
-	pmu_raw_writel(0x1, EXYNOS_SWRESET);
-
-	return NOTIFY_DONE;
-}
-
 static const struct exynos_pmu_data exynos3250_pmu_data = {
 	.pmu_config	= exynos3250_pmu_config,
 	.pmu_init	= exynos3250_pmu_init,

@@ -940,20 +912,11 @@ static const struct of_device_id exynos_pmu_of_device_ids[] = {
 	{ /*sentinel*/ },
 };
 
-/*
- * Exynos PMU restart notifier, handles restart functionality
- */
-static struct notifier_block pmu_restart_handler = {
-	.notifier_call = pmu_restart_notify,
-	.priority = 128,
-};
-
 static int exynos_pmu_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
-	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pmu_base_addr = devm_ioremap_resource(dev, res);

@@ -978,12 +941,6 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pmu_context);
 
-	ret = register_restart_handler(&pmu_restart_handler);
-	if (ret)
-		dev_warn(dev, "can't register restart handler err=%d\n", ret);
-
-	pm_power_off = exynos_power_off;
-
 	dev_dbg(dev, "Exynos PMU Driver probe done\n");
 	return 0;
 }

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
  arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
  1 file changed, 43 deletions(-)

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Best regards,
Krzysztof

Quoted 89 lines

arch/arm/mach-exynos/pmu.c +0 -43

index de68938ee6aa..393c04aa727e 100644
@@ -14,9 +14,6 @@
  #include <linux/of_address.h>
  #include <linux/platform_device.h>
  #include <linux/delay.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
-
 
  #include "exynos-pmu.h"
  #include "regs-pmu.h"

@@ -681,23 +678,6 @@ static unsigned int const exynos5420_list_disable_pmu_reg[] = {
  	EXYNOS5420_CMU_RESET_FSYS_SYS_PWR_REG,
  };
 
-static void exynos_power_off(void)
-{
-	unsigned int tmp;
-
-	pr_info("Power down.\n");
-	tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
-	tmp ^= (1 << 8);
-	pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
-
-	/* Wait a little so we don't give a false warning below */
-	mdelay(100);
-
-	pr_err("Power down failed, please power off system manually.\n");
-	while (1)
-		;
-}
-
  static void exynos5420_powerdown_conf(enum sys_powerdown mode)
  {
  	u32 this_cluster;

@@ -875,14 +855,6 @@ static void exynos5420_pmu_init(void)
  	pr_info("EXYNOS5420 PMU initialized\n");
  }
 
-static int pmu_restart_notify(struct notifier_block *this,
-		unsigned long code, void *unused)
-{
-	pmu_raw_writel(0x1, EXYNOS_SWRESET);
-
-	return NOTIFY_DONE;
-}
-
  static const struct exynos_pmu_data exynos3250_pmu_data = {
  	.pmu_config	= exynos3250_pmu_config,
  	.pmu_init	= exynos3250_pmu_init,

@@ -940,20 +912,11 @@ static const struct of_device_id exynos_pmu_of_device_ids[] = {
  	{ /*sentinel*/ },
  };
 
-/*
- * Exynos PMU restart notifier, handles restart functionality
- */
-static struct notifier_block pmu_restart_handler = {
-	.notifier_call = pmu_restart_notify,
-	.priority = 128,
-};
-
  static int exynos_pmu_probe(struct platform_device *pdev)
  {
  	const struct of_device_id *match;
  	struct device *dev = &pdev->dev;
  	struct resource *res;
-	int ret;
 
  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  	pmu_base_addr = devm_ioremap_resource(dev, res);

@@ -978,12 +941,6 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 
  	platform_set_drvdata(pdev, pmu_context);
 
-	ret = register_restart_handler(&pmu_restart_handler);
-	if (ret)
-		dev_warn(dev, "can't register restart handler err=%d\n", ret);
-
-	pm_power_off = exynos_power_off;
-
  	dev_dbg(dev, "Exynos PMU Driver probe done\n");
  	return 0;
  }

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Best regards,
Krzysztof

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
  arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
  1 file changed, 43 deletions(-)

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

Quoted 5 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Quoted 2 lines

Any ideas how to solve that?

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Quoted 3 lines

Best regards,
Krzysztof

Best regards,

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

Quoted 6 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Quoted 2 lines

Any ideas how to solve that?

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

2015-10-19 18:56 GMT+09:00 Javier Martinez Canillas <[email protected]>:

Quoted 13 lines

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

Quoted 8 lines

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

I am not sure whether this is ABI break issue. There was no compatible mentioning that "reset works" which now would be replaced. The existing PMU compatible (like samsung,exynos4412-pmu) does not mention "reset" as a feature coming with this compatible.

So no ABI break.

Quoted 9 lines

Quoted 8 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Quoted 4 lines

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Selecting from defconfig is not sufficient... since I do not have other idea than selecting then ovak, but Alim please check it whether it does not create circular dependencies on various configs.

Best regards,
Krzysztof

Hello Krzysztof,

Quoted 23 lines · Krzysztof Kozlowski

2015-10-19 18:56 GMT+09:00 Javier Martinez Canillas <[email protected]>:

Quoted 13 lines

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

Quoted 8 lines

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

Quoted 8 lines

I am not sure whether this is ABI break issue. There was no compatible mentioning that "reset works" which now would be replaced. The existing PMU compatible (like samsung,exynos4412-pmu) does not mention "reset" as a feature coming with this compatible.

So no ABI break.

I deliberately didn't use the "DT ABI break" expression since as you said is not part of the documented DT bindings. But what I said is that this change will break old DTBs with newer kernels since reboot and power off will stop working after $SUBJECT.

I'm not a particular fan of the stable DT idea since in practice it seems to do more harm than good but since that was decided, the expectation for users is that booting a new kernel with an old DT should not cause any regression.

So I think that at least a comment in the commit message is needed so if there are people really using old DTs with newer kernels on Exynos boards, they can know that the commit causes such an issue instead of having to figure it out themselves.

Quoted 14 lines

Quoted 13 lines

Quoted 8 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Quoted 4 lines

Selecting from defconfig is not sufficient... since I do not have other idea than selecting then ovak, but Alim please check it whether it does not create circular dependencies on various configs.

Agreed, Kconfig circular dependencies is the reason why select is avoided. Fortunately now the 0-day bot analyzes even posted patches so it's possible that such an issue could be found even before these patches are merged.

Quoted 3 lines

Best regards,
Krzysztof

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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

Hi Javier,

Quoted 44 lines · Javier Martinez Canillas

Hello Krzysztof,

Quoted 23 lines · Krzysztof Kozlowski

2015-10-19 18:56 GMT+09:00 Javier Martinez Canillas <[email protected]>:

Quoted 13 lines

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

Quoted 8 lines

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

Quoted 8 lines

I am not sure whether this is ABI break issue. There was no compatible mentioning that "reset works" which now would be replaced. The existing PMU compatible (like samsung,exynos4412-pmu) does not mention "reset" as a feature coming with this compatible.

So no ABI break.

I deliberately didn't use the "DT ABI break" expression since as you said is not part of the documented DT bindings. But what I said is that this change will break old DTBs with newer kernels since reboot and power off will stop working after $SUBJECT.

I'm not a particular fan of the stable DT idea since in practice it seems to do more harm than good but since that was decided, the expectation for users is that booting a new kernel with an old DT should not cause any regression.

Quoted 5 lines

So I think that at least a comment in the commit message is needed so if there are people really using old DTs with newer kernels on Exynos boards, they can know that the commit causes such an issue instead of having to figure it out themselves.

Agree, will add a comment about this in commit message.

Quoted 19 lines

Quoted 18 lines

Quoted 6 lines

Quoted 6 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Quoted 7 lines

Quoted 2 lines

Any ideas how to solve that?

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Selecting from defconfig is not sufficient... since I do not have other idea than selecting then ovak, but Alim please check it whether it does not create circular dependencies on various configs.

Quoted 4 lines

Agreed, Kconfig circular dependencies is the reason why select is avoided. Fortunately now the 0-day bot analyzes even posted patches so it's possible that such an issue could be found even before these patches are merged.

ok..will select it from mach-exynos/Kconfig.
Thanks.

2015-10-19 18:56 GMT+09:00 Javier Martinez Canillas <[email protected]>:

Quoted 13 lines

Hello,

Quoted 10 lines · Krzysztof Kozlowski

Quoted 8 lines · Alim Akhtar

Now we can use the generic syscon-{reboot/poweroff} drivers, so we don't need special handling for reboot/poweroff in exynos pmu driver. This patch remove the same.

Signed-off-by Alim Akhtar <[email protected]>

---
 arch/arm/mach-exynos/pmu.c |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

Quoted 8 lines

Quoted 5 lines

I think that removal of this stuff will effectively remove the restart/poweroff handlers from:
1. Other defconfigs, like multi_v7
2. Custom configs.

This will also break old DTBs that don't have a "syscon-poweroff" device node that contains the necessary PMU regmap, offset and mask information.

I am not sure whether this is ABI break issue. There was no compatible mentioning that "reset works" which now would be replaced. The existing PMU compatible (like samsung,exynos4412-pmu) does not mention "reset" as a feature coming with this compatible.

So no ABI break.

Quoted 10 lines

Quoted 9 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Quoted 4 lines

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Selecting from defconfig is not sufficient... since I do not have other idea than selecting then ovak, but Alim please check it whether it does not create circular dependencies on various configs.

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

Quoted 14 lines

Quoted 13 lines

Quoted 8 lines

Previously this code was always compiled in for ARCH_EXYNOS. Now it is not so I am thinking about selecting necessary drivers from main exynos Kconfig symbol. That could be tricky though, because "select" should be used only for non-visible symbols.

Any ideas how to solve that?

Is true that select should only be used for non-visible symbols but there are others user visible symbols that are selected by ARCH_EXYNOS such as EXYNOS_THERMAL. So I think selecting the regmap syscon reset stuff there is a sensible option.

Quoted 4 lines

Selecting from defconfig is not sufficient... since I do not have other idea than selecting then ovak, but Alim please check it whether it does not create circular dependencies on various configs.

I checked in multi_v7_defconfig SYSCON_RESET is already enabled by default. so I doubt we are going to run into any circular dependencies, will check again..thanks.

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

On 19.10.2015 15:03, Alim Akhtar wrote:
...

Hi,

I am happy to see more contributions from your side! Keep it up. :)

However this empty space in your cover letter is the place for description of the patchset. What do you want to achieve here? What kind of problem are you solving? What features are you adding? Any dependencies?

Best regards,
Krzysztof

Quoted 18 lines

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

  arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
  arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
  arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
  arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
  arch/arm/configs/exynos_defconfig |   11 +++++-----
  arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
  6 files changed, 61 insertions(+), 49 deletions(-)

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

Hello Alim,

I tested the Exynos4 bits on a Odroid-X2 and I can confirm that poweroff/reboot (still) work.

With best wishes,
Tobias

Quoted 10 lines · Alim Akhtar

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

Quoted 8 lines

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

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

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 18 lines · Alim Akhtar

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

  arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
  arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
  arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
  arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
  arch/arm/configs/exynos_defconfig |   11 +++++-----
  arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
  6 files changed, 61 insertions(+), 49 deletions(-)

Quoted 6 lines

--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Feel free to add Acked-By: Moritz Fischer <[email protected]> to the patches in your series

Moritz

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

Am 19.10.2015 um 08:03 schrieb Alim Akhtar:

Quoted 10 lines

Alim Akhtar (7):
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos3250 SoCs
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos4
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5
arm: dts: Add syscon-{reboot, poweroff} nodes for exynos5410 SoC
ARM: exynos_defconfig: Normalize exynos defconfig
ARM: exynos_defconfig: Enable generic syscon-{reboot, poweroff}
drivers
ARM: EXYNOS: Remove code for restart and poweroff for exynos SoCs

Quoted 8 lines

 arch/arm/boot/dts/exynos3250.dtsi |   14 ++++++++++++
 arch/arm/boot/dts/exynos4.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5.dtsi    |   14 ++++++++++++
 arch/arm/boot/dts/exynos5410.dtsi |   14 ++++++++++++
 arch/arm/configs/exynos_defconfig |   11 +++++-----
 arch/arm/mach-exynos/pmu.c        |   43 -------------------------------------
 6 files changed, 61 insertions(+), 49 deletions(-)

Hi,

I have tested this patch on my Odroid U3 and U3+ using 4.3.0-rc6 from Tobias' repo at https://github.com/tobiasjakobi/linux-odroid/tree/odroid-4.3.y

and his debug config:
https://raw.githubusercontent.com/tobiasjakobi/odroid-environment/master/sourcecode/system/vanilla-4.3-debug.conf

halt -p works (power goes down to 0W)

reboot does not work, stuck at 0.5W, right after
"systemd-shutdown[1]: rebooting".

reboot did work with 4.3.0-rc3 from above repo (without Alim's patch).