Re: [PATCH 3/9] watchdog: s3c2410_wdt: update to use new exynos_pmu_*() apis

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/22/24 14:57, Peter Griffin wrote:
Instead of obtaining the PMU regmap directly use the new exynos_pmu_*()
APIs. The exynos_pmu_ APIs allow support of newer Exynos SoCs that have
atomic set/clear bit hardware and platforms where the PMU registers can
only be accessed via SMC call.


Not really sure about using a direect API instead of regmap. I personally
think that regmap is more generic and like the idea of abstracting hardware
accesses this way. Since that is POV, I won't argue about it. However,

As all platforms that have PMU registers use these new APIs, remove the
syscon regmap lookup code, as it is now redundant.


if syscon is now no longer needed, why keep selecting MFD_SYSCON below,
and why are linux/mfd/syscon.h and linux/regmap.h still included ?

Also, the driver did not previously only support ARCH_EXYNOS but also
ARCH_S3C64XX and ARCH_S5PV210. It is not entirely (actually, not at all)
clear to me if and how those platforms are now supported. EXYNOS_PMU
still seems to depend on ARCH_EXYNOS. How can the driver select
EXYNOS_PMU if ARCH_EXYNOS=n ?

Also, ARCH_EXYNOS already selects EXYNOS_PMU, so a conditional
"select EXYNOS_PMU if ARCH_EXYNOS" would not make sense (or be required)
either.

Please explain all the above.

Thanks,
Guenter

Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
---
  drivers/watchdog/Kconfig       |  1 +
  drivers/watchdog/s3c2410_wdt.c | 25 +++++++++----------------
  2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7d22051b15a2..b3e90e1ddf14 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -513,6 +513,7 @@ config S3C2410_WATCHDOG
  	depends on ARCH_S3C64XX || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
  	select WATCHDOG_CORE
  	select MFD_SYSCON if ARCH_EXYNOS
+	select EXYNOS_PMU
  	help
  	  Watchdog timer block in the Samsung S3C64xx, S5Pv210 and Exynos
  	  SoCs. This will reboot the system when the timer expires with
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 349d30462c8c..fd3a9ce870a0 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -28,6 +28,8 @@
  #include <linux/regmap.h>
  #include <linux/delay.h>
+#include <linux/soc/samsung/exynos-pmu.h>
+
  #define S3C2410_WTCON		0x00
  #define S3C2410_WTDAT		0x04
  #define S3C2410_WTCNT		0x08
@@ -187,7 +189,6 @@ struct s3c2410_wdt {
  	struct watchdog_device	wdt_device;
  	struct notifier_block	freq_transition;
  	const struct s3c2410_wdt_variant *drv_data;
-	struct regmap *pmureg;
  };
static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
@@ -355,8 +356,8 @@ static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
  	const u32 val = mask ? mask_val : 0;
  	int ret;
- ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->disable_reg,
-				 mask_val, val);
+	ret = exynos_pmu_update(wdt->drv_data->disable_reg,
+				mask_val, val);
  	if (ret < 0)
  		dev_err(wdt->dev, "failed to update reg(%d)\n", ret);
@@ -370,8 +371,8 @@ static int s3c2410wdt_mask_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
  	const u32 val = (mask ^ val_inv) ? mask_val : 0;
  	int ret;
- ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->mask_reset_reg,
-				 mask_val, val);
+	ret = exynos_pmu_update(wdt->drv_data->mask_reset_reg,
+				mask_val, val);
  	if (ret < 0)
  		dev_err(wdt->dev, "failed to update reg(%d)\n", ret);
@@ -384,8 +385,8 @@ static int s3c2410wdt_enable_counter(struct s3c2410_wdt *wdt, bool en)
  	const u32 val = en ? mask_val : 0;
  	int ret;
- ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->cnt_en_reg,
-				 mask_val, val);
+	ret = exynos_pmu_update(wdt->drv_data->cnt_en_reg,
+				mask_val, val);
  	if (ret < 0)
  		dev_err(wdt->dev, "failed to update reg(%d)\n", ret);
@@ -617,7 +618,7 @@ static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
  	if (!(wdt->drv_data->quirks & QUIRK_HAS_PMU_RST_STAT))
  		return 0;
- ret = regmap_read(wdt->pmureg, wdt->drv_data->rst_stat_reg, &rst_stat);
+	ret = exynos_pmu_read(wdt->drv_data->rst_stat_reg, &rst_stat);
  	if (ret)
  		dev_warn(wdt->dev, "Couldn't get RST_STAT register\n");
  	else if (rst_stat & BIT(wdt->drv_data->rst_stat_bit))
@@ -698,14 +699,6 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
  	if (ret)
  		return ret;
- if (wdt->drv_data->quirks & QUIRKS_HAVE_PMUREG) {
-		wdt->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
-						"samsung,syscon-phandle");
-		if (IS_ERR(wdt->pmureg))
-			return dev_err_probe(dev, PTR_ERR(wdt->pmureg),
-					     "syscon regmap lookup failed.\n");
-	}
-
  	wdt_irq = platform_get_irq(pdev, 0);
  	if (wdt_irq < 0)
  		return wdt_irq;





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux