* [PATCH v1 2/5] watchdog: intel-mid_wdt: Postpone IRQ handler registration till SCU is ready
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
@ 2021-01-22 12:31 ` Andy Shevchenko
2021-01-22 14:17 ` Guenter Roeck
2021-01-22 12:31 ` [PATCH v1 3/5] platform/x86: intel_scu_wdt: Move driver from arch/x86 Andy Shevchenko
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 12:31 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck
Cc: Andy Shevchenko, Linus Walleij, Mika Westerberg
When SCU is not ready and CONFIG_DEBUG_SHIRQ=y we got deferred probe followed
by fired test IRQ which immediately makes kernel panic. Fix this by delaying
IRQ handler registration till SCU is ready.
Fixes: 80ae679b8f86 ("watchdog: intel-mid_wdt: Convert to use new SCU IPC API")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/watchdog/intel-mid_wdt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
index 1ae03b64ef8b..9b2173f765c8 100644
--- a/drivers/watchdog/intel-mid_wdt.c
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -154,6 +154,10 @@ static int mid_wdt_probe(struct platform_device *pdev)
watchdog_set_nowayout(wdt_dev, WATCHDOG_NOWAYOUT);
watchdog_set_drvdata(wdt_dev, mid);
+ mid->scu = devm_intel_scu_ipc_dev_get(dev);
+ if (!mid->scu)
+ return -EPROBE_DEFER;
+
ret = devm_request_irq(dev, pdata->irq, mid_wdt_irq,
IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
wdt_dev);
@@ -162,10 +166,6 @@ static int mid_wdt_probe(struct platform_device *pdev)
return ret;
}
- mid->scu = devm_intel_scu_ipc_dev_get(dev);
- if (!mid->scu)
- return -EPROBE_DEFER;
-
/*
* The firmware followed by U-Boot leaves the watchdog running
* with the default threshold which may vary. When we get here
--
2.29.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 2/5] watchdog: intel-mid_wdt: Postpone IRQ handler registration till SCU is ready
2021-01-22 12:31 ` [PATCH v1 2/5] watchdog: intel-mid_wdt: Postpone IRQ handler registration till SCU is ready Andy Shevchenko
@ 2021-01-22 14:17 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2021-01-22 14:17 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck
Cc: Linus Walleij, Mika Westerberg
On 1/22/21 4:31 AM, Andy Shevchenko wrote:
> When SCU is not ready and CONFIG_DEBUG_SHIRQ=y we got deferred probe followed
> by fired test IRQ which immediately makes kernel panic. Fix this by delaying
> IRQ handler registration till SCU is ready.
>
> Fixes: 80ae679b8f86 ("watchdog: intel-mid_wdt: Convert to use new SCU IPC API")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/intel-mid_wdt.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
> index 1ae03b64ef8b..9b2173f765c8 100644
> --- a/drivers/watchdog/intel-mid_wdt.c
> +++ b/drivers/watchdog/intel-mid_wdt.c
> @@ -154,6 +154,10 @@ static int mid_wdt_probe(struct platform_device *pdev)
> watchdog_set_nowayout(wdt_dev, WATCHDOG_NOWAYOUT);
> watchdog_set_drvdata(wdt_dev, mid);
>
> + mid->scu = devm_intel_scu_ipc_dev_get(dev);
> + if (!mid->scu)
> + return -EPROBE_DEFER;
> +
> ret = devm_request_irq(dev, pdata->irq, mid_wdt_irq,
> IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
> wdt_dev);
> @@ -162,10 +166,6 @@ static int mid_wdt_probe(struct platform_device *pdev)
> return ret;
> }
>
> - mid->scu = devm_intel_scu_ipc_dev_get(dev);
> - if (!mid->scu)
> - return -EPROBE_DEFER;
> -
> /*
> * The firmware followed by U-Boot leaves the watchdog running
> * with the default threshold which may vary. When we get here
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 3/5] platform/x86: intel_scu_wdt: Move driver from arch/x86
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
2021-01-22 12:31 ` [PATCH v1 2/5] watchdog: intel-mid_wdt: Postpone IRQ handler registration till SCU is ready Andy Shevchenko
@ 2021-01-22 12:31 ` Andy Shevchenko
2021-01-22 14:21 ` Guenter Roeck
2021-01-22 12:32 ` [PATCH v1 4/5] platform/x86: intel_scu_wdt: Drop SCU notification Andy Shevchenko
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 12:31 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck
Cc: Andy Shevchenko, Hans de Goede, Linus Walleij
The ACPI-enabled Intel MID platforms neither have WDAT table nor proper IDs
to instantiate watchdog device. In order to keep them working move the board
code from arch/x86 to drivers/platform/x86.
Note, the complete SFI support is going to be removed, that's why PDx86
has been chosen as a new home for it. This is the only device which needs
additional code so far.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/x86/platform/intel-mid/device_libs/Makefile | 1 -
drivers/platform/x86/Kconfig | 8 ++++++++
drivers/platform/x86/Makefile | 1 +
.../platform/x86/intel_scu_wdt.c | 0
4 files changed, 9 insertions(+), 1 deletion(-)
rename arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c => drivers/platform/x86/intel_scu_wdt.c (100%)
diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 480fed21cc7d..918edac9ab9a 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -30,4 +30,3 @@ obj-$(subst m,y,$(CONFIG_GPIO_PCA953X)) += platform_tca6416.o
obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
obj-$(subst m,y,$(CONFIG_INTEL_MID_POWER_BUTTON)) += platform_mrfld_power_btn.o
obj-$(subst m,y,$(CONFIG_RTC_DRV_CMOS)) += platform_mrfld_rtc.o
-obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_mrfld_wdt.o
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 4a5798a0ce0c..0bb85eabace1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1424,6 +1424,14 @@ config INTEL_SCU_PLATFORM
and SCU (sometimes called PMC as well). The driver currently
supports Intel Elkhart Lake and compatible platforms.
+config INTEL_SCU_WDT
+ bool
+ default INTEL_SCU_PCI
+ depends on INTEL_MID_WATCHDOG
+ help
+ This is a specific platform code to instantiate watchdog device
+ on ACPI-based Intel MID platforms.
+
config INTEL_SCU_IPC_UTIL
tristate "Intel SCU IPC utility driver"
depends on INTEL_SCU
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 728ccc226a29..19306450d791 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -146,6 +146,7 @@ obj-$(CONFIG_INTEL_PUNIT_IPC) += intel_punit_ipc.o
obj-$(CONFIG_INTEL_SCU_IPC) += intel_scu_ipc.o
obj-$(CONFIG_INTEL_SCU_PCI) += intel_scu_pcidrv.o
obj-$(CONFIG_INTEL_SCU_PLATFORM) += intel_scu_pltdrv.o
+obj-$(CONFIG_INTEL_SCU_WDT) += intel_scu_wdt.o
obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o
obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \
intel_telemetry_pltdrv.o \
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
similarity index 100%
rename from arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c
rename to drivers/platform/x86/intel_scu_wdt.c
--
2.29.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 3/5] platform/x86: intel_scu_wdt: Move driver from arch/x86
2021-01-22 12:31 ` [PATCH v1 3/5] platform/x86: intel_scu_wdt: Move driver from arch/x86 Andy Shevchenko
@ 2021-01-22 14:21 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2021-01-22 14:21 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck
Cc: Hans de Goede, Linus Walleij
On 1/22/21 4:31 AM, Andy Shevchenko wrote:
> The ACPI-enabled Intel MID platforms neither have WDAT table nor proper IDs
> to instantiate watchdog device. In order to keep them working move the board
> code from arch/x86 to drivers/platform/x86.
>
> Note, the complete SFI support is going to be removed, that's why PDx86
> has been chosen as a new home for it. This is the only device which needs
> additional code so far.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Not sure if I like the new name (seems to be a bit misleading), but still
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> arch/x86/platform/intel-mid/device_libs/Makefile | 1 -
> drivers/platform/x86/Kconfig | 8 ++++++++
> drivers/platform/x86/Makefile | 1 +
> .../platform/x86/intel_scu_wdt.c | 0
> 4 files changed, 9 insertions(+), 1 deletion(-)
> rename arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c => drivers/platform/x86/intel_scu_wdt.c (100%)
>
> diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> index 480fed21cc7d..918edac9ab9a 100644
> --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> @@ -30,4 +30,3 @@ obj-$(subst m,y,$(CONFIG_GPIO_PCA953X)) += platform_tca6416.o
> obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> obj-$(subst m,y,$(CONFIG_INTEL_MID_POWER_BUTTON)) += platform_mrfld_power_btn.o
> obj-$(subst m,y,$(CONFIG_RTC_DRV_CMOS)) += platform_mrfld_rtc.o
> -obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_mrfld_wdt.o
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 4a5798a0ce0c..0bb85eabace1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1424,6 +1424,14 @@ config INTEL_SCU_PLATFORM
> and SCU (sometimes called PMC as well). The driver currently
> supports Intel Elkhart Lake and compatible platforms.
>
> +config INTEL_SCU_WDT
> + bool
> + default INTEL_SCU_PCI
> + depends on INTEL_MID_WATCHDOG
> + help
> + This is a specific platform code to instantiate watchdog device
> + on ACPI-based Intel MID platforms.
> +
> config INTEL_SCU_IPC_UTIL
> tristate "Intel SCU IPC utility driver"
> depends on INTEL_SCU
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 728ccc226a29..19306450d791 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -146,6 +146,7 @@ obj-$(CONFIG_INTEL_PUNIT_IPC) += intel_punit_ipc.o
> obj-$(CONFIG_INTEL_SCU_IPC) += intel_scu_ipc.o
> obj-$(CONFIG_INTEL_SCU_PCI) += intel_scu_pcidrv.o
> obj-$(CONFIG_INTEL_SCU_PLATFORM) += intel_scu_pltdrv.o
> +obj-$(CONFIG_INTEL_SCU_WDT) += intel_scu_wdt.o
> obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o
> obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \
> intel_telemetry_pltdrv.o \
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
> similarity index 100%
> rename from arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c
> rename to drivers/platform/x86/intel_scu_wdt.c
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 4/5] platform/x86: intel_scu_wdt: Drop SCU notification
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
2021-01-22 12:31 ` [PATCH v1 2/5] watchdog: intel-mid_wdt: Postpone IRQ handler registration till SCU is ready Andy Shevchenko
2021-01-22 12:31 ` [PATCH v1 3/5] platform/x86: intel_scu_wdt: Move driver from arch/x86 Andy Shevchenko
@ 2021-01-22 12:32 ` Andy Shevchenko
2021-01-22 14:21 ` Guenter Roeck
2021-01-22 12:32 ` [PATCH v1 5/5] platform/x86: intel_scu_wdt: Get rid of custom x86 model comparison Andy Shevchenko
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 12:32 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck
Cc: Andy Shevchenko, Hans de Goede, Linus Walleij
Since SCU code along with the Intel MID watchdog driver has been refactored
in a way that latter will be probed only after the former has been come
to live, the notification code is bogus and not needed. Remove it for good.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/platform/x86/intel_scu_wdt.c | 31 +++++++---------------------
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/platform/x86/intel_scu_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
index 227218a8f98e..19f7686a3c19 100644
--- a/drivers/platform/x86/intel_scu_wdt.c
+++ b/drivers/platform/x86/intel_scu_wdt.c
@@ -12,7 +12,6 @@
#include <linux/platform_data/intel-mid_wdt.h>
#include <asm/intel-mid.h>
-#include <asm/intel_scu_ipc.h>
#include <asm/io_apic.h>
#include <asm/hw_irq.h>
@@ -49,34 +48,18 @@ static struct intel_mid_wdt_pdata tangier_pdata = {
.probe = tangier_probe,
};
-static int wdt_scu_status_change(struct notifier_block *nb,
- unsigned long code, void *data)
-{
- if (code == SCU_DOWN) {
- platform_device_unregister(&wdt_dev);
- return 0;
- }
-
- return platform_device_register(&wdt_dev);
-}
-
-static struct notifier_block wdt_scu_notifier = {
- .notifier_call = wdt_scu_status_change,
-};
-
static int __init register_mid_wdt(void)
{
if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
return -ENODEV;
wdt_dev.dev.platform_data = &tangier_pdata;
-
- /*
- * We need to be sure that the SCU IPC is ready before watchdog device
- * can be registered:
- */
- intel_scu_notifier_add(&wdt_scu_notifier);
-
- return 0;
+ return platform_device_register(&wdt_dev);
}
arch_initcall(register_mid_wdt);
+
+static void __exit unregister_mid_wdt(void)
+{
+ platform_device_unregister(&wdt_dev);
+}
+__exitcall(unregister_mid_wdt);
--
2.29.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 4/5] platform/x86: intel_scu_wdt: Drop SCU notification
2021-01-22 12:32 ` [PATCH v1 4/5] platform/x86: intel_scu_wdt: Drop SCU notification Andy Shevchenko
@ 2021-01-22 14:21 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2021-01-22 14:21 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck
Cc: Hans de Goede, Linus Walleij
On 1/22/21 4:32 AM, Andy Shevchenko wrote:
> Since SCU code along with the Intel MID watchdog driver has been refactored
> in a way that latter will be probed only after the former has been come
> to live, the notification code is bogus and not needed. Remove it for good.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/platform/x86/intel_scu_wdt.c | 31 +++++++---------------------
> 1 file changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_scu_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
> index 227218a8f98e..19f7686a3c19 100644
> --- a/drivers/platform/x86/intel_scu_wdt.c
> +++ b/drivers/platform/x86/intel_scu_wdt.c
> @@ -12,7 +12,6 @@
> #include <linux/platform_data/intel-mid_wdt.h>
>
> #include <asm/intel-mid.h>
> -#include <asm/intel_scu_ipc.h>
> #include <asm/io_apic.h>
> #include <asm/hw_irq.h>
>
> @@ -49,34 +48,18 @@ static struct intel_mid_wdt_pdata tangier_pdata = {
> .probe = tangier_probe,
> };
>
> -static int wdt_scu_status_change(struct notifier_block *nb,
> - unsigned long code, void *data)
> -{
> - if (code == SCU_DOWN) {
> - platform_device_unregister(&wdt_dev);
> - return 0;
> - }
> -
> - return platform_device_register(&wdt_dev);
> -}
> -
> -static struct notifier_block wdt_scu_notifier = {
> - .notifier_call = wdt_scu_status_change,
> -};
> -
> static int __init register_mid_wdt(void)
> {
> if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
> return -ENODEV;
>
> wdt_dev.dev.platform_data = &tangier_pdata;
> -
> - /*
> - * We need to be sure that the SCU IPC is ready before watchdog device
> - * can be registered:
> - */
> - intel_scu_notifier_add(&wdt_scu_notifier);
> -
> - return 0;
> + return platform_device_register(&wdt_dev);
> }
> arch_initcall(register_mid_wdt);
> +
> +static void __exit unregister_mid_wdt(void)
> +{
> + platform_device_unregister(&wdt_dev);
> +}
> +__exitcall(unregister_mid_wdt);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 5/5] platform/x86: intel_scu_wdt: Get rid of custom x86 model comparison
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
` (2 preceding siblings ...)
2021-01-22 12:32 ` [PATCH v1 4/5] platform/x86: intel_scu_wdt: Drop SCU notification Andy Shevchenko
@ 2021-01-22 12:32 ` Andy Shevchenko
2021-01-22 14:23 ` Guenter Roeck
2021-01-22 14:16 ` [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Guenter Roeck
2021-01-22 14:59 ` Hans de Goede
5 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 12:32 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck
Cc: Andy Shevchenko
Switch the platform code to use x86_id_table and accompanying API
instead of custom comparison against x86 CPU model.
This is one of the last users of custom API for that and following
changes will remove it for the good.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/platform/x86/intel_scu_wdt.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel_scu_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
index 19f7686a3c19..85ee85ca2215 100644
--- a/drivers/platform/x86/intel_scu_wdt.c
+++ b/drivers/platform/x86/intel_scu_wdt.c
@@ -11,6 +11,8 @@
#include <linux/platform_device.h>
#include <linux/platform_data/intel-mid_wdt.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
#include <asm/intel-mid.h>
#include <asm/io_apic.h>
#include <asm/hw_irq.h>
@@ -48,12 +50,20 @@ static struct intel_mid_wdt_pdata tangier_pdata = {
.probe = tangier_probe,
};
+static const struct x86_cpu_id intel_mid_cpu_ids[] = {
+ X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, &tangier_pdata),
+ {}
+};
+
static int __init register_mid_wdt(void)
{
- if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
+ const struct x86_cpu_id *id;
+
+ id = x86_match_cpu(intel_mid_cpu_ids);
+ if (!id)
return -ENODEV;
- wdt_dev.dev.platform_data = &tangier_pdata;
+ wdt_dev.dev.platform_data = (const struct intel_mid_wdt_pdata *)id->driver_data;
return platform_device_register(&wdt_dev);
}
arch_initcall(register_mid_wdt);
--
2.29.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 5/5] platform/x86: intel_scu_wdt: Get rid of custom x86 model comparison
2021-01-22 12:32 ` [PATCH v1 5/5] platform/x86: intel_scu_wdt: Get rid of custom x86 model comparison Andy Shevchenko
@ 2021-01-22 14:23 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2021-01-22 14:23 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck
On 1/22/21 4:32 AM, Andy Shevchenko wrote:
> Switch the platform code to use x86_id_table and accompanying API
> instead of custom comparison against x86 CPU model.
>
> This is one of the last users of custom API for that and following
> changes will remove it for the good.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/platform/x86/intel_scu_wdt.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_scu_wdt.c b/drivers/platform/x86/intel_scu_wdt.c
> index 19f7686a3c19..85ee85ca2215 100644
> --- a/drivers/platform/x86/intel_scu_wdt.c
> +++ b/drivers/platform/x86/intel_scu_wdt.c
> @@ -11,6 +11,8 @@
> #include <linux/platform_device.h>
> #include <linux/platform_data/intel-mid_wdt.h>
>
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> #include <asm/intel-mid.h>
> #include <asm/io_apic.h>
> #include <asm/hw_irq.h>
> @@ -48,12 +50,20 @@ static struct intel_mid_wdt_pdata tangier_pdata = {
> .probe = tangier_probe,
> };
>
> +static const struct x86_cpu_id intel_mid_cpu_ids[] = {
> + X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, &tangier_pdata),
> + {}
> +};
> +
> static int __init register_mid_wdt(void)
> {
> - if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
> + const struct x86_cpu_id *id;
> +
> + id = x86_match_cpu(intel_mid_cpu_ids);
> + if (!id)
> return -ENODEV;
>
> - wdt_dev.dev.platform_data = &tangier_pdata;
> + wdt_dev.dev.platform_data = (const struct intel_mid_wdt_pdata *)id->driver_data;
> return platform_device_register(&wdt_dev);
> }
> arch_initcall(register_mid_wdt);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
` (3 preceding siblings ...)
2021-01-22 12:32 ` [PATCH v1 5/5] platform/x86: intel_scu_wdt: Get rid of custom x86 model comparison Andy Shevchenko
@ 2021-01-22 14:16 ` Guenter Roeck
2021-01-22 14:59 ` Hans de Goede
5 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2021-01-22 14:16 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck
Cc: Linus Walleij
On 1/22/21 4:31 AM, Andy Shevchenko wrote:
> Intel Moorestown and Medfield are quite old Intel Atom based
> 32-bit platforms, which were in limited use in some Android phones,
> tablets and consumer electronics more than eight years ago.
>
> There are no bugs or problems ever reported outside from Intel
> for breaking any of that platforms for years. It seems no real
> users exists who run more or less fresh kernel on it. The commit
> 05f4434bc130 ("ASoC: Intel: remove mfld_machine") also in align
> with this theory.
>
> Due to above and to reduce a burden of supporting outdated drivers
> we remove the support of outdated platforms completely.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/Kconfig | 9 -
> drivers/watchdog/Makefile | 1 -
> drivers/watchdog/intel_scu_watchdog.c | 533 --------------------------
> drivers/watchdog/intel_scu_watchdog.h | 50 ---
> 4 files changed, 593 deletions(-)
> delete mode 100644 drivers/watchdog/intel_scu_watchdog.c
> delete mode 100644 drivers/watchdog/intel_scu_watchdog.h
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 7ff941e71b79..6b9e93d8532b 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1219,15 +1219,6 @@ config IE6XX_WDT
> To compile this driver as a module, choose M here: the
> module will be called ie6xx_wdt.
>
> -config INTEL_SCU_WATCHDOG
> - bool "Intel SCU Watchdog for Mobile Platforms"
> - depends on X86_INTEL_MID
> - help
> - Hardware driver for the watchdog time built into the Intel SCU
> - for Intel Mobile Platforms.
> -
> - To compile this driver as a module, choose M here.
> -
> config INTEL_MID_WATCHDOG
> tristate "Intel MID Watchdog Timer"
> depends on X86_INTEL_MID
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c74ee19d441..74f61e4105d8 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -140,7 +140,6 @@ obj-$(CONFIG_W83877F_WDT) += w83877f_wdt.o
> obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
> obj-$(CONFIG_MACHZ_WDT) += machzwd.o
> obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
> -obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
> obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
> obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
> obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
> diff --git a/drivers/watchdog/intel_scu_watchdog.c b/drivers/watchdog/intel_scu_watchdog.c
> deleted file mode 100644
> index 804e35940983..000000000000
> --- a/drivers/watchdog/intel_scu_watchdog.c
> +++ /dev/null
> @@ -1,533 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Intel_SCU 0.2: An Intel SCU IOH Based Watchdog Device
> - * for Intel part #(s):
> - * - AF82MP20 PCH
> - *
> - * Copyright (C) 2009-2010 Intel Corporation. All rights reserved.
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/compiler.h>
> -#include <linux/kernel.h>
> -#include <linux/moduleparam.h>
> -#include <linux/types.h>
> -#include <linux/miscdevice.h>
> -#include <linux/watchdog.h>
> -#include <linux/fs.h>
> -#include <linux/notifier.h>
> -#include <linux/reboot.h>
> -#include <linux/init.h>
> -#include <linux/jiffies.h>
> -#include <linux/uaccess.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
> -#include <linux/interrupt.h>
> -#include <linux/delay.h>
> -#include <linux/sched.h>
> -#include <linux/signal.h>
> -#include <linux/sfi.h>
> -#include <asm/irq.h>
> -#include <linux/atomic.h>
> -#include <asm/intel_scu_ipc.h>
> -#include <asm/apb_timer.h>
> -#include <asm/intel-mid.h>
> -
> -#include "intel_scu_watchdog.h"
> -
> -/* Bounds number of times we will retry loading time count */
> -/* This retry is a work around for a silicon bug. */
> -#define MAX_RETRY 16
> -
> -#define IPC_SET_WATCHDOG_TIMER 0xF8
> -
> -static int timer_margin = DEFAULT_SOFT_TO_HARD_MARGIN;
> -module_param(timer_margin, int, 0);
> -MODULE_PARM_DESC(timer_margin,
> - "Watchdog timer margin"
> - "Time between interrupt and resetting the system"
> - "The range is from 1 to 160"
> - "This is the time for all keep alives to arrive");
> -
> -static int timer_set = DEFAULT_TIME;
> -module_param(timer_set, int, 0);
> -MODULE_PARM_DESC(timer_set,
> - "Default Watchdog timer setting"
> - "Complete cycle time"
> - "The range is from 1 to 170"
> - "This is the time for all keep alives to arrive");
> -
> -/* After watchdog device is closed, check force_boot. If:
> - * force_boot == 0, then force boot on next watchdog interrupt after close,
> - * force_boot == 1, then force boot immediately when device is closed.
> - */
> -static int force_boot;
> -module_param(force_boot, int, 0);
> -MODULE_PARM_DESC(force_boot,
> - "A value of 1 means that the driver will reboot"
> - "the system immediately if the /dev/watchdog device is closed"
> - "A value of 0 means that when /dev/watchdog device is closed"
> - "the watchdog timer will be refreshed for one more interval"
> - "of length: timer_set. At the end of this interval, the"
> - "watchdog timer will reset the system."
> - );
> -
> -/* there is only one device in the system now; this can be made into
> - * an array in the future if we have more than one device */
> -
> -static struct intel_scu_watchdog_dev watchdog_device;
> -
> -/* Forces restart, if force_reboot is set */
> -static void watchdog_fire(void)
> -{
> - if (force_boot) {
> - pr_crit("Initiating system reboot\n");
> - emergency_restart();
> - pr_crit("Reboot didn't ?????\n");
> - }
> -
> - else {
> - pr_crit("Immediate Reboot Disabled\n");
> - pr_crit("System will reset when watchdog timer times out!\n");
> - }
> -}
> -
> -static int check_timer_margin(int new_margin)
> -{
> - if ((new_margin < MIN_TIME_CYCLE) ||
> - (new_margin > MAX_TIME - timer_set)) {
> - pr_debug("value of new_margin %d is out of the range %d to %d\n",
> - new_margin, MIN_TIME_CYCLE, MAX_TIME - timer_set);
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -/*
> - * IPC operations
> - */
> -static int watchdog_set_ipc(int soft_threshold, int threshold)
> -{
> - u32 *ipc_wbuf;
> - u8 cbuf[16] = { '\0' };
> - int ipc_ret = 0;
> -
> - ipc_wbuf = (u32 *)&cbuf;
> - ipc_wbuf[0] = soft_threshold;
> - ipc_wbuf[1] = threshold;
> -
> - ipc_ret = intel_scu_ipc_command(
> - IPC_SET_WATCHDOG_TIMER,
> - 0,
> - ipc_wbuf,
> - 2,
> - NULL,
> - 0);
> -
> - if (ipc_ret != 0)
> - pr_err("Error setting SCU watchdog timer: %x\n", ipc_ret);
> -
> - return ipc_ret;
> -};
> -
> -/*
> - * Intel_SCU operations
> - */
> -
> -/* timer interrupt handler */
> -static irqreturn_t watchdog_timer_interrupt(int irq, void *dev_id)
> -{
> - int int_status;
> - int_status = ioread32(watchdog_device.timer_interrupt_status_addr);
> -
> - pr_debug("irq, int_status: %x\n", int_status);
> -
> - if (int_status != 0)
> - return IRQ_NONE;
> -
> - /* has the timer been started? If not, then this is spurious */
> - if (watchdog_device.timer_started == 0) {
> - pr_debug("spurious interrupt received\n");
> - return IRQ_HANDLED;
> - }
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* set the timer to the threshold */
> - iowrite32(watchdog_device.threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* allow the timer to run */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static int intel_scu_keepalive(void)
> -{
> -
> - /* read eoi register - clears interrupt */
> - ioread32(watchdog_device.timer_clear_interrupt_addr);
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* set the timer to the soft_threshold */
> - iowrite32(watchdog_device.soft_threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* allow the timer to run */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - return 0;
> -}
> -
> -static int intel_scu_stop(void)
> -{
> - iowrite32(0, watchdog_device.timer_control_addr);
> - return 0;
> -}
> -
> -static int intel_scu_set_heartbeat(u32 t)
> -{
> - int ipc_ret;
> - int retry_count;
> - u32 soft_value;
> - u32 hw_value;
> -
> - watchdog_device.timer_set = t;
> - watchdog_device.threshold =
> - timer_margin * watchdog_device.timer_tbl_ptr->freq_hz;
> - watchdog_device.soft_threshold =
> - (watchdog_device.timer_set - timer_margin)
> - * watchdog_device.timer_tbl_ptr->freq_hz;
> -
> - pr_debug("set_heartbeat: timer freq is %d\n",
> - watchdog_device.timer_tbl_ptr->freq_hz);
> - pr_debug("set_heartbeat: timer_set is %x (hex)\n",
> - watchdog_device.timer_set);
> - pr_debug("set_heartbeat: timer_margin is %x (hex)\n", timer_margin);
> - pr_debug("set_heartbeat: threshold is %x (hex)\n",
> - watchdog_device.threshold);
> - pr_debug("set_heartbeat: soft_threshold is %x (hex)\n",
> - watchdog_device.soft_threshold);
> -
> - /* Adjust thresholds by FREQ_ADJUSTMENT factor, to make the */
> - /* watchdog timing come out right. */
> - watchdog_device.threshold =
> - watchdog_device.threshold / FREQ_ADJUSTMENT;
> - watchdog_device.soft_threshold =
> - watchdog_device.soft_threshold / FREQ_ADJUSTMENT;
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* send the threshold and soft_threshold via IPC to the processor */
> - ipc_ret = watchdog_set_ipc(watchdog_device.soft_threshold,
> - watchdog_device.threshold);
> -
> - if (ipc_ret != 0) {
> - /* Make sure the watchdog timer is stopped */
> - intel_scu_stop();
> - return ipc_ret;
> - }
> -
> - /* Soft Threshold set loop. Early versions of silicon did */
> - /* not always set this count correctly. This loop checks */
> - /* the value and retries if it was not set correctly. */
> -
> - retry_count = 0;
> - soft_value = watchdog_device.soft_threshold & 0xFFFF0000;
> - do {
> -
> - /* Make sure timer is stopped */
> - intel_scu_stop();
> -
> - if (MAX_RETRY < retry_count++) {
> - /* Unable to set timer value */
> - pr_err("Unable to set timer\n");
> - return -ENODEV;
> - }
> -
> - /* set the timer to the soft threshold */
> - iowrite32(watchdog_device.soft_threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* read count value before starting timer */
> - ioread32(watchdog_device.timer_load_count_addr);
> -
> - /* Start the timer */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - /* read the value the time loaded into its count reg */
> - hw_value = ioread32(watchdog_device.timer_load_count_addr);
> - hw_value = hw_value & 0xFFFF0000;
> -
> -
> - } while (soft_value != hw_value);
> -
> - watchdog_device.timer_started = 1;
> -
> - return 0;
> -}
> -
> -/*
> - * /dev/watchdog handling
> - */
> -
> -static int intel_scu_open(struct inode *inode, struct file *file)
> -{
> -
> - /* Set flag to indicate that watchdog device is open */
> - if (test_and_set_bit(0, &watchdog_device.driver_open))
> - return -EBUSY;
> -
> - /* Check for reopen of driver. Reopens are not allowed */
> - if (watchdog_device.driver_closed)
> - return -EPERM;
> -
> - return stream_open(inode, file);
> -}
> -
> -static int intel_scu_release(struct inode *inode, struct file *file)
> -{
> - /*
> - * This watchdog should not be closed, after the timer
> - * is started with the WDIPC_SETTIMEOUT ioctl
> - * If force_boot is set watchdog_fire() will cause an
> - * immediate reset. If force_boot is not set, the watchdog
> - * timer is refreshed for one more interval. At the end
> - * of that interval, the watchdog timer will reset the system.
> - */
> -
> - if (!test_and_clear_bit(0, &watchdog_device.driver_open)) {
> - pr_debug("intel_scu_release, without open\n");
> - return -ENOTTY;
> - }
> -
> - if (!watchdog_device.timer_started) {
> - /* Just close, since timer has not been started */
> - pr_debug("closed, without starting timer\n");
> - return 0;
> - }
> -
> - pr_crit("Unexpected close of /dev/watchdog!\n");
> -
> - /* Since the timer was started, prevent future reopens */
> - watchdog_device.driver_closed = 1;
> -
> - /* Refresh the timer for one more interval */
> - intel_scu_keepalive();
> -
> - /* Reboot system (if force_boot is set) */
> - watchdog_fire();
> -
> - /* We should only reach this point if force_boot is not set */
> - return 0;
> -}
> -
> -static ssize_t intel_scu_write(struct file *file,
> - char const *data,
> - size_t len,
> - loff_t *ppos)
> -{
> -
> - if (watchdog_device.timer_started)
> - /* Watchdog already started, keep it alive */
> - intel_scu_keepalive();
> - else
> - /* Start watchdog with timer value set by init */
> - intel_scu_set_heartbeat(watchdog_device.timer_set);
> -
> - return len;
> -}
> -
> -static long intel_scu_ioctl(struct file *file,
> - unsigned int cmd,
> - unsigned long arg)
> -{
> - void __user *argp = (void __user *)arg;
> - u32 __user *p = argp;
> - u32 new_margin;
> -
> -
> - static const struct watchdog_info ident = {
> - .options = WDIOF_SETTIMEOUT
> - | WDIOF_KEEPALIVEPING,
> - .firmware_version = 0, /* @todo Get from SCU via
> - ipc_get_scu_fw_version()? */
> - .identity = "Intel_SCU IOH Watchdog" /* len < 32 */
> - };
> -
> - switch (cmd) {
> - case WDIOC_GETSUPPORT:
> - return copy_to_user(argp,
> - &ident,
> - sizeof(ident)) ? -EFAULT : 0;
> - case WDIOC_GETSTATUS:
> - case WDIOC_GETBOOTSTATUS:
> - return put_user(0, p);
> - case WDIOC_KEEPALIVE:
> - intel_scu_keepalive();
> -
> - return 0;
> - case WDIOC_SETTIMEOUT:
> - if (get_user(new_margin, p))
> - return -EFAULT;
> -
> - if (check_timer_margin(new_margin))
> - return -EINVAL;
> -
> - if (intel_scu_set_heartbeat(new_margin))
> - return -EINVAL;
> - return 0;
> - case WDIOC_GETTIMEOUT:
> - return put_user(watchdog_device.soft_threshold, p);
> -
> - default:
> - return -ENOTTY;
> - }
> -}
> -
> -/*
> - * Notifier for system down
> - */
> -static int intel_scu_notify_sys(struct notifier_block *this,
> - unsigned long code,
> - void *another_unused)
> -{
> - if (code == SYS_DOWN || code == SYS_HALT)
> - /* Turn off the watchdog timer. */
> - intel_scu_stop();
> - return NOTIFY_DONE;
> -}
> -
> -/*
> - * Kernel Interfaces
> - */
> -static const struct file_operations intel_scu_fops = {
> - .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .write = intel_scu_write,
> - .unlocked_ioctl = intel_scu_ioctl,
> - .compat_ioctl = compat_ptr_ioctl,
> - .open = intel_scu_open,
> - .release = intel_scu_release,
> -};
> -
> -static int __init intel_scu_watchdog_init(void)
> -{
> - int ret;
> - u32 __iomem *tmp_addr;
> -
> - /*
> - * We don't really need to check this as the SFI timer get will fail
> - * but if we do so we can exit with a clearer reason and no noise.
> - *
> - * If it isn't an intel MID device then it doesn't have this watchdog
> - */
> - if (!intel_mid_identify_cpu())
> - return -ENODEV;
> -
> - /* Check boot parameters to verify that their initial values */
> - /* are in range. */
> - /* Check value of timer_set boot parameter */
> - if ((timer_set < MIN_TIME_CYCLE) ||
> - (timer_set > MAX_TIME - MIN_TIME_CYCLE)) {
> - pr_err("value of timer_set %x (hex) is out of range from %x to %x (hex)\n",
> - timer_set, MIN_TIME_CYCLE, MAX_TIME - MIN_TIME_CYCLE);
> - return -EINVAL;
> - }
> -
> - /* Check value of timer_margin boot parameter */
> - if (check_timer_margin(timer_margin))
> - return -EINVAL;
> -
> - watchdog_device.timer_tbl_ptr = sfi_get_mtmr(sfi_mtimer_num-1);
> -
> - if (watchdog_device.timer_tbl_ptr == NULL) {
> - pr_debug("timer is not available\n");
> - return -ENODEV;
> - }
> - /* make sure the timer exists */
> - if (watchdog_device.timer_tbl_ptr->phys_addr == 0) {
> - pr_debug("timer %d does not have valid physical memory\n",
> - sfi_mtimer_num);
> - return -ENODEV;
> - }
> -
> - if (watchdog_device.timer_tbl_ptr->irq == 0) {
> - pr_debug("timer %d invalid irq\n", sfi_mtimer_num);
> - return -ENODEV;
> - }
> -
> - tmp_addr = ioremap(watchdog_device.timer_tbl_ptr->phys_addr,
> - 20);
> -
> - if (tmp_addr == NULL) {
> - pr_debug("timer unable to ioremap\n");
> - return -ENOMEM;
> - }
> -
> - watchdog_device.timer_load_count_addr = tmp_addr++;
> - watchdog_device.timer_current_value_addr = tmp_addr++;
> - watchdog_device.timer_control_addr = tmp_addr++;
> - watchdog_device.timer_clear_interrupt_addr = tmp_addr++;
> - watchdog_device.timer_interrupt_status_addr = tmp_addr++;
> -
> - /* Set the default time values in device structure */
> -
> - watchdog_device.timer_set = timer_set;
> - watchdog_device.threshold =
> - timer_margin * watchdog_device.timer_tbl_ptr->freq_hz;
> - watchdog_device.soft_threshold =
> - (watchdog_device.timer_set - timer_margin)
> - * watchdog_device.timer_tbl_ptr->freq_hz;
> -
> -
> - watchdog_device.intel_scu_notifier.notifier_call =
> - intel_scu_notify_sys;
> -
> - ret = register_reboot_notifier(&watchdog_device.intel_scu_notifier);
> - if (ret) {
> - pr_err("cannot register notifier %d)\n", ret);
> - goto register_reboot_error;
> - }
> -
> - watchdog_device.miscdev.minor = WATCHDOG_MINOR;
> - watchdog_device.miscdev.name = "watchdog";
> - watchdog_device.miscdev.fops = &intel_scu_fops;
> -
> - ret = misc_register(&watchdog_device.miscdev);
> - if (ret) {
> - pr_err("cannot register miscdev %d err =%d\n",
> - WATCHDOG_MINOR, ret);
> - goto misc_register_error;
> - }
> -
> - ret = request_irq((unsigned int)watchdog_device.timer_tbl_ptr->irq,
> - watchdog_timer_interrupt,
> - IRQF_SHARED, "watchdog",
> - &watchdog_device.timer_load_count_addr);
> - if (ret) {
> - pr_err("error requesting irq %d\n", ret);
> - goto request_irq_error;
> - }
> - /* Make sure timer is disabled before returning */
> - intel_scu_stop();
> - return 0;
> -
> -/* error cleanup */
> -
> -request_irq_error:
> - misc_deregister(&watchdog_device.miscdev);
> -misc_register_error:
> - unregister_reboot_notifier(&watchdog_device.intel_scu_notifier);
> -register_reboot_error:
> - intel_scu_stop();
> - iounmap(watchdog_device.timer_load_count_addr);
> - return ret;
> -}
> -late_initcall(intel_scu_watchdog_init);
> diff --git a/drivers/watchdog/intel_scu_watchdog.h b/drivers/watchdog/intel_scu_watchdog.h
> deleted file mode 100644
> index fb12a25ee417..000000000000
> --- a/drivers/watchdog/intel_scu_watchdog.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Intel_SCU 0.2: An Intel SCU IOH Based Watchdog Device
> - * for Intel part #(s):
> - * - AF82MP20 PCH
> - *
> - * Copyright (C) 2009-2010 Intel Corporation. All rights reserved.
> - */
> -
> -#ifndef __INTEL_SCU_WATCHDOG_H
> -#define __INTEL_SCU_WATCHDOG_H
> -
> -#define WDT_VER "0.3"
> -
> -/* minimum time between interrupts */
> -#define MIN_TIME_CYCLE 1
> -
> -/* Time from warning to reboot is 2 seconds */
> -#define DEFAULT_SOFT_TO_HARD_MARGIN 2
> -
> -#define MAX_TIME 170
> -
> -#define DEFAULT_TIME 5
> -
> -#define MAX_SOFT_TO_HARD_MARGIN (MAX_TIME-MIN_TIME_CYCLE)
> -
> -/* Ajustment to clock tick frequency to make timing come out right */
> -#define FREQ_ADJUSTMENT 8
> -
> -struct intel_scu_watchdog_dev {
> - ulong driver_open;
> - ulong driver_closed;
> - u32 timer_started;
> - u32 timer_set;
> - u32 threshold;
> - u32 soft_threshold;
> - u32 __iomem *timer_load_count_addr;
> - u32 __iomem *timer_current_value_addr;
> - u32 __iomem *timer_control_addr;
> - u32 __iomem *timer_clear_interrupt_addr;
> - u32 __iomem *timer_interrupt_status_addr;
> - struct sfi_timer_table_entry *timer_tbl_ptr;
> - struct notifier_block intel_scu_notifier;
> - struct miscdevice miscdev;
> -};
> -
> -extern int sfi_mtimer_num;
> -
> -/* extern struct sfi_timer_table_entry *sfi_get_mtmr(int hint); */
> -#endif /* __INTEL_SCU_WATCHDOG_H */
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 12:31 [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Andy Shevchenko
` (4 preceding siblings ...)
2021-01-22 14:16 ` [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform Guenter Roeck
@ 2021-01-22 14:59 ` Hans de Goede
2021-01-22 15:22 ` Andy Shevchenko
5 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2021-01-22 14:59 UTC (permalink / raw)
To: Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, platform-driver-x86, Mark Gross,
linux-watchdog, Wim Van Sebroeck, Guenter Roeck
Cc: Linus Walleij
Hi,
What is the plan for merging this series ?
It touches files under:
arch/x86
drivers/watchdog
drivers/platform/x86
It is probably best if the entire series is merged through a single
tree. I don't expect this to cause any conflicts with current / upcoming
changes under drivers/platform/x86, so I'm fine with this being merged
through another tree.
Or if I can get an ack for that from the x86 and watchdog maintainers
I can merge the entire series through the pdx86 tree.
Regards,
Hans
On 1/22/21 1:31 PM, Andy Shevchenko wrote:
> Intel Moorestown and Medfield are quite old Intel Atom based
> 32-bit platforms, which were in limited use in some Android phones,
> tablets and consumer electronics more than eight years ago.
>
> There are no bugs or problems ever reported outside from Intel
> for breaking any of that platforms for years. It seems no real
> users exists who run more or less fresh kernel on it. The commit
> 05f4434bc130 ("ASoC: Intel: remove mfld_machine") also in align
> with this theory.
>
> Due to above and to reduce a burden of supporting outdated drivers
> we remove the support of outdated platforms completely.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/watchdog/Kconfig | 9 -
> drivers/watchdog/Makefile | 1 -
> drivers/watchdog/intel_scu_watchdog.c | 533 --------------------------
> drivers/watchdog/intel_scu_watchdog.h | 50 ---
> 4 files changed, 593 deletions(-)
> delete mode 100644 drivers/watchdog/intel_scu_watchdog.c
> delete mode 100644 drivers/watchdog/intel_scu_watchdog.h
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 7ff941e71b79..6b9e93d8532b 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1219,15 +1219,6 @@ config IE6XX_WDT
> To compile this driver as a module, choose M here: the
> module will be called ie6xx_wdt.
>
> -config INTEL_SCU_WATCHDOG
> - bool "Intel SCU Watchdog for Mobile Platforms"
> - depends on X86_INTEL_MID
> - help
> - Hardware driver for the watchdog time built into the Intel SCU
> - for Intel Mobile Platforms.
> -
> - To compile this driver as a module, choose M here.
> -
> config INTEL_MID_WATCHDOG
> tristate "Intel MID Watchdog Timer"
> depends on X86_INTEL_MID
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c74ee19d441..74f61e4105d8 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -140,7 +140,6 @@ obj-$(CONFIG_W83877F_WDT) += w83877f_wdt.o
> obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
> obj-$(CONFIG_MACHZ_WDT) += machzwd.o
> obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
> -obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
> obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
> obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
> obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
> diff --git a/drivers/watchdog/intel_scu_watchdog.c b/drivers/watchdog/intel_scu_watchdog.c
> deleted file mode 100644
> index 804e35940983..000000000000
> --- a/drivers/watchdog/intel_scu_watchdog.c
> +++ /dev/null
> @@ -1,533 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Intel_SCU 0.2: An Intel SCU IOH Based Watchdog Device
> - * for Intel part #(s):
> - * - AF82MP20 PCH
> - *
> - * Copyright (C) 2009-2010 Intel Corporation. All rights reserved.
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/compiler.h>
> -#include <linux/kernel.h>
> -#include <linux/moduleparam.h>
> -#include <linux/types.h>
> -#include <linux/miscdevice.h>
> -#include <linux/watchdog.h>
> -#include <linux/fs.h>
> -#include <linux/notifier.h>
> -#include <linux/reboot.h>
> -#include <linux/init.h>
> -#include <linux/jiffies.h>
> -#include <linux/uaccess.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
> -#include <linux/interrupt.h>
> -#include <linux/delay.h>
> -#include <linux/sched.h>
> -#include <linux/signal.h>
> -#include <linux/sfi.h>
> -#include <asm/irq.h>
> -#include <linux/atomic.h>
> -#include <asm/intel_scu_ipc.h>
> -#include <asm/apb_timer.h>
> -#include <asm/intel-mid.h>
> -
> -#include "intel_scu_watchdog.h"
> -
> -/* Bounds number of times we will retry loading time count */
> -/* This retry is a work around for a silicon bug. */
> -#define MAX_RETRY 16
> -
> -#define IPC_SET_WATCHDOG_TIMER 0xF8
> -
> -static int timer_margin = DEFAULT_SOFT_TO_HARD_MARGIN;
> -module_param(timer_margin, int, 0);
> -MODULE_PARM_DESC(timer_margin,
> - "Watchdog timer margin"
> - "Time between interrupt and resetting the system"
> - "The range is from 1 to 160"
> - "This is the time for all keep alives to arrive");
> -
> -static int timer_set = DEFAULT_TIME;
> -module_param(timer_set, int, 0);
> -MODULE_PARM_DESC(timer_set,
> - "Default Watchdog timer setting"
> - "Complete cycle time"
> - "The range is from 1 to 170"
> - "This is the time for all keep alives to arrive");
> -
> -/* After watchdog device is closed, check force_boot. If:
> - * force_boot == 0, then force boot on next watchdog interrupt after close,
> - * force_boot == 1, then force boot immediately when device is closed.
> - */
> -static int force_boot;
> -module_param(force_boot, int, 0);
> -MODULE_PARM_DESC(force_boot,
> - "A value of 1 means that the driver will reboot"
> - "the system immediately if the /dev/watchdog device is closed"
> - "A value of 0 means that when /dev/watchdog device is closed"
> - "the watchdog timer will be refreshed for one more interval"
> - "of length: timer_set. At the end of this interval, the"
> - "watchdog timer will reset the system."
> - );
> -
> -/* there is only one device in the system now; this can be made into
> - * an array in the future if we have more than one device */
> -
> -static struct intel_scu_watchdog_dev watchdog_device;
> -
> -/* Forces restart, if force_reboot is set */
> -static void watchdog_fire(void)
> -{
> - if (force_boot) {
> - pr_crit("Initiating system reboot\n");
> - emergency_restart();
> - pr_crit("Reboot didn't ?????\n");
> - }
> -
> - else {
> - pr_crit("Immediate Reboot Disabled\n");
> - pr_crit("System will reset when watchdog timer times out!\n");
> - }
> -}
> -
> -static int check_timer_margin(int new_margin)
> -{
> - if ((new_margin < MIN_TIME_CYCLE) ||
> - (new_margin > MAX_TIME - timer_set)) {
> - pr_debug("value of new_margin %d is out of the range %d to %d\n",
> - new_margin, MIN_TIME_CYCLE, MAX_TIME - timer_set);
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -/*
> - * IPC operations
> - */
> -static int watchdog_set_ipc(int soft_threshold, int threshold)
> -{
> - u32 *ipc_wbuf;
> - u8 cbuf[16] = { '\0' };
> - int ipc_ret = 0;
> -
> - ipc_wbuf = (u32 *)&cbuf;
> - ipc_wbuf[0] = soft_threshold;
> - ipc_wbuf[1] = threshold;
> -
> - ipc_ret = intel_scu_ipc_command(
> - IPC_SET_WATCHDOG_TIMER,
> - 0,
> - ipc_wbuf,
> - 2,
> - NULL,
> - 0);
> -
> - if (ipc_ret != 0)
> - pr_err("Error setting SCU watchdog timer: %x\n", ipc_ret);
> -
> - return ipc_ret;
> -};
> -
> -/*
> - * Intel_SCU operations
> - */
> -
> -/* timer interrupt handler */
> -static irqreturn_t watchdog_timer_interrupt(int irq, void *dev_id)
> -{
> - int int_status;
> - int_status = ioread32(watchdog_device.timer_interrupt_status_addr);
> -
> - pr_debug("irq, int_status: %x\n", int_status);
> -
> - if (int_status != 0)
> - return IRQ_NONE;
> -
> - /* has the timer been started? If not, then this is spurious */
> - if (watchdog_device.timer_started == 0) {
> - pr_debug("spurious interrupt received\n");
> - return IRQ_HANDLED;
> - }
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* set the timer to the threshold */
> - iowrite32(watchdog_device.threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* allow the timer to run */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static int intel_scu_keepalive(void)
> -{
> -
> - /* read eoi register - clears interrupt */
> - ioread32(watchdog_device.timer_clear_interrupt_addr);
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* set the timer to the soft_threshold */
> - iowrite32(watchdog_device.soft_threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* allow the timer to run */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - return 0;
> -}
> -
> -static int intel_scu_stop(void)
> -{
> - iowrite32(0, watchdog_device.timer_control_addr);
> - return 0;
> -}
> -
> -static int intel_scu_set_heartbeat(u32 t)
> -{
> - int ipc_ret;
> - int retry_count;
> - u32 soft_value;
> - u32 hw_value;
> -
> - watchdog_device.timer_set = t;
> - watchdog_device.threshold =
> - timer_margin * watchdog_device.timer_tbl_ptr->freq_hz;
> - watchdog_device.soft_threshold =
> - (watchdog_device.timer_set - timer_margin)
> - * watchdog_device.timer_tbl_ptr->freq_hz;
> -
> - pr_debug("set_heartbeat: timer freq is %d\n",
> - watchdog_device.timer_tbl_ptr->freq_hz);
> - pr_debug("set_heartbeat: timer_set is %x (hex)\n",
> - watchdog_device.timer_set);
> - pr_debug("set_heartbeat: timer_margin is %x (hex)\n", timer_margin);
> - pr_debug("set_heartbeat: threshold is %x (hex)\n",
> - watchdog_device.threshold);
> - pr_debug("set_heartbeat: soft_threshold is %x (hex)\n",
> - watchdog_device.soft_threshold);
> -
> - /* Adjust thresholds by FREQ_ADJUSTMENT factor, to make the */
> - /* watchdog timing come out right. */
> - watchdog_device.threshold =
> - watchdog_device.threshold / FREQ_ADJUSTMENT;
> - watchdog_device.soft_threshold =
> - watchdog_device.soft_threshold / FREQ_ADJUSTMENT;
> -
> - /* temporarily disable the timer */
> - iowrite32(0x00000002, watchdog_device.timer_control_addr);
> -
> - /* send the threshold and soft_threshold via IPC to the processor */
> - ipc_ret = watchdog_set_ipc(watchdog_device.soft_threshold,
> - watchdog_device.threshold);
> -
> - if (ipc_ret != 0) {
> - /* Make sure the watchdog timer is stopped */
> - intel_scu_stop();
> - return ipc_ret;
> - }
> -
> - /* Soft Threshold set loop. Early versions of silicon did */
> - /* not always set this count correctly. This loop checks */
> - /* the value and retries if it was not set correctly. */
> -
> - retry_count = 0;
> - soft_value = watchdog_device.soft_threshold & 0xFFFF0000;
> - do {
> -
> - /* Make sure timer is stopped */
> - intel_scu_stop();
> -
> - if (MAX_RETRY < retry_count++) {
> - /* Unable to set timer value */
> - pr_err("Unable to set timer\n");
> - return -ENODEV;
> - }
> -
> - /* set the timer to the soft threshold */
> - iowrite32(watchdog_device.soft_threshold,
> - watchdog_device.timer_load_count_addr);
> -
> - /* read count value before starting timer */
> - ioread32(watchdog_device.timer_load_count_addr);
> -
> - /* Start the timer */
> - iowrite32(0x00000003, watchdog_device.timer_control_addr);
> -
> - /* read the value the time loaded into its count reg */
> - hw_value = ioread32(watchdog_device.timer_load_count_addr);
> - hw_value = hw_value & 0xFFFF0000;
> -
> -
> - } while (soft_value != hw_value);
> -
> - watchdog_device.timer_started = 1;
> -
> - return 0;
> -}
> -
> -/*
> - * /dev/watchdog handling
> - */
> -
> -static int intel_scu_open(struct inode *inode, struct file *file)
> -{
> -
> - /* Set flag to indicate that watchdog device is open */
> - if (test_and_set_bit(0, &watchdog_device.driver_open))
> - return -EBUSY;
> -
> - /* Check for reopen of driver. Reopens are not allowed */
> - if (watchdog_device.driver_closed)
> - return -EPERM;
> -
> - return stream_open(inode, file);
> -}
> -
> -static int intel_scu_release(struct inode *inode, struct file *file)
> -{
> - /*
> - * This watchdog should not be closed, after the timer
> - * is started with the WDIPC_SETTIMEOUT ioctl
> - * If force_boot is set watchdog_fire() will cause an
> - * immediate reset. If force_boot is not set, the watchdog
> - * timer is refreshed for one more interval. At the end
> - * of that interval, the watchdog timer will reset the system.
> - */
> -
> - if (!test_and_clear_bit(0, &watchdog_device.driver_open)) {
> - pr_debug("intel_scu_release, without open\n");
> - return -ENOTTY;
> - }
> -
> - if (!watchdog_device.timer_started) {
> - /* Just close, since timer has not been started */
> - pr_debug("closed, without starting timer\n");
> - return 0;
> - }
> -
> - pr_crit("Unexpected close of /dev/watchdog!\n");
> -
> - /* Since the timer was started, prevent future reopens */
> - watchdog_device.driver_closed = 1;
> -
> - /* Refresh the timer for one more interval */
> - intel_scu_keepalive();
> -
> - /* Reboot system (if force_boot is set) */
> - watchdog_fire();
> -
> - /* We should only reach this point if force_boot is not set */
> - return 0;
> -}
> -
> -static ssize_t intel_scu_write(struct file *file,
> - char const *data,
> - size_t len,
> - loff_t *ppos)
> -{
> -
> - if (watchdog_device.timer_started)
> - /* Watchdog already started, keep it alive */
> - intel_scu_keepalive();
> - else
> - /* Start watchdog with timer value set by init */
> - intel_scu_set_heartbeat(watchdog_device.timer_set);
> -
> - return len;
> -}
> -
> -static long intel_scu_ioctl(struct file *file,
> - unsigned int cmd,
> - unsigned long arg)
> -{
> - void __user *argp = (void __user *)arg;
> - u32 __user *p = argp;
> - u32 new_margin;
> -
> -
> - static const struct watchdog_info ident = {
> - .options = WDIOF_SETTIMEOUT
> - | WDIOF_KEEPALIVEPING,
> - .firmware_version = 0, /* @todo Get from SCU via
> - ipc_get_scu_fw_version()? */
> - .identity = "Intel_SCU IOH Watchdog" /* len < 32 */
> - };
> -
> - switch (cmd) {
> - case WDIOC_GETSUPPORT:
> - return copy_to_user(argp,
> - &ident,
> - sizeof(ident)) ? -EFAULT : 0;
> - case WDIOC_GETSTATUS:
> - case WDIOC_GETBOOTSTATUS:
> - return put_user(0, p);
> - case WDIOC_KEEPALIVE:
> - intel_scu_keepalive();
> -
> - return 0;
> - case WDIOC_SETTIMEOUT:
> - if (get_user(new_margin, p))
> - return -EFAULT;
> -
> - if (check_timer_margin(new_margin))
> - return -EINVAL;
> -
> - if (intel_scu_set_heartbeat(new_margin))
> - return -EINVAL;
> - return 0;
> - case WDIOC_GETTIMEOUT:
> - return put_user(watchdog_device.soft_threshold, p);
> -
> - default:
> - return -ENOTTY;
> - }
> -}
> -
> -/*
> - * Notifier for system down
> - */
> -static int intel_scu_notify_sys(struct notifier_block *this,
> - unsigned long code,
> - void *another_unused)
> -{
> - if (code == SYS_DOWN || code == SYS_HALT)
> - /* Turn off the watchdog timer. */
> - intel_scu_stop();
> - return NOTIFY_DONE;
> -}
> -
> -/*
> - * Kernel Interfaces
> - */
> -static const struct file_operations intel_scu_fops = {
> - .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .write = intel_scu_write,
> - .unlocked_ioctl = intel_scu_ioctl,
> - .compat_ioctl = compat_ptr_ioctl,
> - .open = intel_scu_open,
> - .release = intel_scu_release,
> -};
> -
> -static int __init intel_scu_watchdog_init(void)
> -{
> - int ret;
> - u32 __iomem *tmp_addr;
> -
> - /*
> - * We don't really need to check this as the SFI timer get will fail
> - * but if we do so we can exit with a clearer reason and no noise.
> - *
> - * If it isn't an intel MID device then it doesn't have this watchdog
> - */
> - if (!intel_mid_identify_cpu())
> - return -ENODEV;
> -
> - /* Check boot parameters to verify that their initial values */
> - /* are in range. */
> - /* Check value of timer_set boot parameter */
> - if ((timer_set < MIN_TIME_CYCLE) ||
> - (timer_set > MAX_TIME - MIN_TIME_CYCLE)) {
> - pr_err("value of timer_set %x (hex) is out of range from %x to %x (hex)\n",
> - timer_set, MIN_TIME_CYCLE, MAX_TIME - MIN_TIME_CYCLE);
> - return -EINVAL;
> - }
> -
> - /* Check value of timer_margin boot parameter */
> - if (check_timer_margin(timer_margin))
> - return -EINVAL;
> -
> - watchdog_device.timer_tbl_ptr = sfi_get_mtmr(sfi_mtimer_num-1);
> -
> - if (watchdog_device.timer_tbl_ptr == NULL) {
> - pr_debug("timer is not available\n");
> - return -ENODEV;
> - }
> - /* make sure the timer exists */
> - if (watchdog_device.timer_tbl_ptr->phys_addr == 0) {
> - pr_debug("timer %d does not have valid physical memory\n",
> - sfi_mtimer_num);
> - return -ENODEV;
> - }
> -
> - if (watchdog_device.timer_tbl_ptr->irq == 0) {
> - pr_debug("timer %d invalid irq\n", sfi_mtimer_num);
> - return -ENODEV;
> - }
> -
> - tmp_addr = ioremap(watchdog_device.timer_tbl_ptr->phys_addr,
> - 20);
> -
> - if (tmp_addr == NULL) {
> - pr_debug("timer unable to ioremap\n");
> - return -ENOMEM;
> - }
> -
> - watchdog_device.timer_load_count_addr = tmp_addr++;
> - watchdog_device.timer_current_value_addr = tmp_addr++;
> - watchdog_device.timer_control_addr = tmp_addr++;
> - watchdog_device.timer_clear_interrupt_addr = tmp_addr++;
> - watchdog_device.timer_interrupt_status_addr = tmp_addr++;
> -
> - /* Set the default time values in device structure */
> -
> - watchdog_device.timer_set = timer_set;
> - watchdog_device.threshold =
> - timer_margin * watchdog_device.timer_tbl_ptr->freq_hz;
> - watchdog_device.soft_threshold =
> - (watchdog_device.timer_set - timer_margin)
> - * watchdog_device.timer_tbl_ptr->freq_hz;
> -
> -
> - watchdog_device.intel_scu_notifier.notifier_call =
> - intel_scu_notify_sys;
> -
> - ret = register_reboot_notifier(&watchdog_device.intel_scu_notifier);
> - if (ret) {
> - pr_err("cannot register notifier %d)\n", ret);
> - goto register_reboot_error;
> - }
> -
> - watchdog_device.miscdev.minor = WATCHDOG_MINOR;
> - watchdog_device.miscdev.name = "watchdog";
> - watchdog_device.miscdev.fops = &intel_scu_fops;
> -
> - ret = misc_register(&watchdog_device.miscdev);
> - if (ret) {
> - pr_err("cannot register miscdev %d err =%d\n",
> - WATCHDOG_MINOR, ret);
> - goto misc_register_error;
> - }
> -
> - ret = request_irq((unsigned int)watchdog_device.timer_tbl_ptr->irq,
> - watchdog_timer_interrupt,
> - IRQF_SHARED, "watchdog",
> - &watchdog_device.timer_load_count_addr);
> - if (ret) {
> - pr_err("error requesting irq %d\n", ret);
> - goto request_irq_error;
> - }
> - /* Make sure timer is disabled before returning */
> - intel_scu_stop();
> - return 0;
> -
> -/* error cleanup */
> -
> -request_irq_error:
> - misc_deregister(&watchdog_device.miscdev);
> -misc_register_error:
> - unregister_reboot_notifier(&watchdog_device.intel_scu_notifier);
> -register_reboot_error:
> - intel_scu_stop();
> - iounmap(watchdog_device.timer_load_count_addr);
> - return ret;
> -}
> -late_initcall(intel_scu_watchdog_init);
> diff --git a/drivers/watchdog/intel_scu_watchdog.h b/drivers/watchdog/intel_scu_watchdog.h
> deleted file mode 100644
> index fb12a25ee417..000000000000
> --- a/drivers/watchdog/intel_scu_watchdog.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Intel_SCU 0.2: An Intel SCU IOH Based Watchdog Device
> - * for Intel part #(s):
> - * - AF82MP20 PCH
> - *
> - * Copyright (C) 2009-2010 Intel Corporation. All rights reserved.
> - */
> -
> -#ifndef __INTEL_SCU_WATCHDOG_H
> -#define __INTEL_SCU_WATCHDOG_H
> -
> -#define WDT_VER "0.3"
> -
> -/* minimum time between interrupts */
> -#define MIN_TIME_CYCLE 1
> -
> -/* Time from warning to reboot is 2 seconds */
> -#define DEFAULT_SOFT_TO_HARD_MARGIN 2
> -
> -#define MAX_TIME 170
> -
> -#define DEFAULT_TIME 5
> -
> -#define MAX_SOFT_TO_HARD_MARGIN (MAX_TIME-MIN_TIME_CYCLE)
> -
> -/* Ajustment to clock tick frequency to make timing come out right */
> -#define FREQ_ADJUSTMENT 8
> -
> -struct intel_scu_watchdog_dev {
> - ulong driver_open;
> - ulong driver_closed;
> - u32 timer_started;
> - u32 timer_set;
> - u32 threshold;
> - u32 soft_threshold;
> - u32 __iomem *timer_load_count_addr;
> - u32 __iomem *timer_current_value_addr;
> - u32 __iomem *timer_control_addr;
> - u32 __iomem *timer_clear_interrupt_addr;
> - u32 __iomem *timer_interrupt_status_addr;
> - struct sfi_timer_table_entry *timer_tbl_ptr;
> - struct notifier_block intel_scu_notifier;
> - struct miscdevice miscdev;
> -};
> -
> -extern int sfi_mtimer_num;
> -
> -/* extern struct sfi_timer_table_entry *sfi_get_mtmr(int hint); */
> -#endif /* __INTEL_SCU_WATCHDOG_H */
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 14:59 ` Hans de Goede
@ 2021-01-22 15:22 ` Andy Shevchenko
2021-01-22 15:45 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 15:22 UTC (permalink / raw)
To: Hans de Goede
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck, Linus Walleij
On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
> Hi,
>
> What is the plan for merging this series ?
>
> It touches files under:
> arch/x86
> drivers/watchdog
> drivers/platform/x86
>
> It is probably best if the entire series is merged through a single
> tree. I don't expect this to cause any conflicts with current / upcoming
> changes under drivers/platform/x86, so I'm fine with this being merged
> through another tree.
>
> Or if I can get an ack for that from the x86 and watchdog maintainers
> I can merge the entire series through the pdx86 tree.
For time being I'm collecting tags and comments.
As of today this series is independent per se, but other (later) clean up
patches may rely on this.
One strategy can be providing an immutable branch for anybody who wants it and
merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
are more patches touching other stuff, though independently, in this matters).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 15:22 ` Andy Shevchenko
@ 2021-01-22 15:45 ` Andy Shevchenko
2021-01-22 21:04 ` Hans de Goede
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-22 15:45 UTC (permalink / raw)
To: Hans de Goede
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
+Cc: Arnd, RTC maintainers, Lee
On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
> > Hi,
> >
> > What is the plan for merging this series ?
> >
> > It touches files under:
> > arch/x86
> > drivers/watchdog
> > drivers/platform/x86
> >
> > It is probably best if the entire series is merged through a single
> > tree. I don't expect this to cause any conflicts with current / upcoming
> > changes under drivers/platform/x86, so I'm fine with this being merged
> > through another tree.
> >
> > Or if I can get an ack for that from the x86 and watchdog maintainers
> > I can merge the entire series through the pdx86 tree.
>
> For time being I'm collecting tags and comments.
> As of today this series is independent per se, but other (later) clean up
> patches may rely on this.
>
> One strategy can be providing an immutable branch for anybody who wants it and
> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
> are more patches touching other stuff, though independently, in this matters).
Yes, I think one more time and guess the best is:
- get tags from x86 / watchdog
- get tag from RTC
- collect everything in PDx86 immutable branch
- part 2 can be merged thru Lee's MFD tree with taking above mentioned branch
as a base
- part 3 (SFI removal) can be done based on Lee's branch and via tip tree
in this case we won't wait all maintainers to give a tag in a short period of
time (like couple of weeks from now on).
The full series (all three parts) are located in [1].
[1]: https://gitlab.com/andy-shev/next/-/tree/topic/mid-removal-next
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 15:45 ` Andy Shevchenko
@ 2021-01-22 21:04 ` Hans de Goede
2021-01-23 0:27 ` Guenter Roeck
0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2021-01-22 21:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Guenter Roeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
Hi,
On 1/22/21 4:45 PM, Andy Shevchenko wrote:
> +Cc: Arnd, RTC maintainers, Lee
>
> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> What is the plan for merging this series ?
>>>
>>> It touches files under:
>>> arch/x86
>>> drivers/watchdog
>>> drivers/platform/x86
>>>
>>> It is probably best if the entire series is merged through a single
>>> tree. I don't expect this to cause any conflicts with current / upcoming
>>> changes under drivers/platform/x86, so I'm fine with this being merged
>>> through another tree.
>>>
>>> Or if I can get an ack for that from the x86 and watchdog maintainers
>>> I can merge the entire series through the pdx86 tree.
>>
>> For time being I'm collecting tags and comments.
>> As of today this series is independent per se, but other (later) clean up
>> patches may rely on this.
>>
>> One strategy can be providing an immutable branch for anybody who wants it and
>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
>> are more patches touching other stuff, though independently, in this matters).
>
> Yes, I think one more time and guess the best is:
> - get tags from x86 / watchdog
> - get tag from RTC
> - collect everything in PDx86 immutable branch
Ok, that works for me. I assume that it is best for the other subsystems
if I create an immutable branch for this based on 5.11-rc1
(and then merge that into pdx86/for-next) ?
Guenter, I believe you have already reviewed all the watchdog changes,
are you ok with me creating an immutable branch for the entire series
and then sending you a pull-req for that ?
And x86 folks, can I get an Ack for merging the:
rename arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c => drivers/platform/x86/intel_scu_wdt.c
Change through an PDx86 immutable branch (for which you will then
get a pull-req) ?
Regards,
Hans
> - part 2 can be merged thru Lee's MFD tree with taking above mentioned branch
> as a base
> - part 3 (SFI removal) can be done based on Lee's branch and via tip tree
>
> in this case we won't wait all maintainers to give a tag in a short period of
> time (like couple of weeks from now on).
>
> The full series (all three parts) are located in [1].
>
> [1]: https://gitlab.com/andy-shev/next/-/tree/topic/mid-removal-next
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-22 21:04 ` Hans de Goede
@ 2021-01-23 0:27 ` Guenter Roeck
2021-01-23 12:05 ` Hans de Goede
0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2021-01-23 0:27 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
On 1/22/21 1:04 PM, Hans de Goede wrote:
> Hi,
>
> On 1/22/21 4:45 PM, Andy Shevchenko wrote:
>> +Cc: Arnd, RTC maintainers, Lee
>>
>> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
>>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> What is the plan for merging this series ?
>>>>
>>>> It touches files under:
>>>> arch/x86
>>>> drivers/watchdog
>>>> drivers/platform/x86
>>>>
>>>> It is probably best if the entire series is merged through a single
>>>> tree. I don't expect this to cause any conflicts with current / upcoming
>>>> changes under drivers/platform/x86, so I'm fine with this being merged
>>>> through another tree.
>>>>
>>>> Or if I can get an ack for that from the x86 and watchdog maintainers
>>>> I can merge the entire series through the pdx86 tree.
>>>
>>> For time being I'm collecting tags and comments.
>>> As of today this series is independent per se, but other (later) clean up
>>> patches may rely on this.
>>>
>>> One strategy can be providing an immutable branch for anybody who wants it and
>>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
>>> are more patches touching other stuff, though independently, in this matters).
>>
>> Yes, I think one more time and guess the best is:
>> - get tags from x86 / watchdog
>> - get tag from RTC
>> - collect everything in PDx86 immutable branch
>
> Ok, that works for me. I assume that it is best for the other subsystems
> if I create an immutable branch for this based on 5.11-rc1
> (and then merge that into pdx86/for-next) ?
>
> Guenter, I believe you have already reviewed all the watchdog changes,
> are you ok with me creating an immutable branch for the entire series
> and then sending you a pull-req for that ?
>
Wim handles watchdog pull requests, and he usually creates a branch
for Linus to pull from either during or shortly before a commit window.
Also, I don't immediately see why the watchdog tree should carry the other
patches of this series, immutable or not; I would not expect a dependency
against those.
Anyway, I am fine if the the wdt driver removal is submitted through some
other branch.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-23 0:27 ` Guenter Roeck
@ 2021-01-23 12:05 ` Hans de Goede
2021-01-25 11:15 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2021-01-23 12:05 UTC (permalink / raw)
To: Guenter Roeck, Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, platform-driver-x86, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
HI,
On 1/23/21 1:27 AM, Guenter Roeck wrote:
> On 1/22/21 1:04 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 1/22/21 4:45 PM, Andy Shevchenko wrote:
>>> +Cc: Arnd, RTC maintainers, Lee
>>>
>>> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
>>>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> What is the plan for merging this series ?
>>>>>
>>>>> It touches files under:
>>>>> arch/x86
>>>>> drivers/watchdog
>>>>> drivers/platform/x86
>>>>>
>>>>> It is probably best if the entire series is merged through a single
>>>>> tree. I don't expect this to cause any conflicts with current / upcoming
>>>>> changes under drivers/platform/x86, so I'm fine with this being merged
>>>>> through another tree.
>>>>>
>>>>> Or if I can get an ack for that from the x86 and watchdog maintainers
>>>>> I can merge the entire series through the pdx86 tree.
>>>>
>>>> For time being I'm collecting tags and comments.
>>>> As of today this series is independent per se, but other (later) clean up
>>>> patches may rely on this.
>>>>
>>>> One strategy can be providing an immutable branch for anybody who wants it and
>>>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
>>>> are more patches touching other stuff, though independently, in this matters).
>>>
>>> Yes, I think one more time and guess the best is:
>>> - get tags from x86 / watchdog
>>> - get tag from RTC
>>> - collect everything in PDx86 immutable branch
>>
>> Ok, that works for me. I assume that it is best for the other subsystems
>> if I create an immutable branch for this based on 5.11-rc1
>> (and then merge that into pdx86/for-next) ?
>>
>> Guenter, I believe you have already reviewed all the watchdog changes,
>> are you ok with me creating an immutable branch for the entire series
>> and then sending you a pull-req for that ?
>>
>
> Wim handles watchdog pull requests,
Ah, sorry I was under the impression you would do that because you were
doing the reviews (which I'm sure Wim appreciates).
> and he usually creates a branch
> for Linus to pull from either during or shortly before a commit window.
> Also, I don't immediately see why the watchdog tree should carry the other
> patches of this series, immutable or not; I would not expect a dependency
> against those.
>
> Anyway, I am fine if the the wdt driver removal is submitted through some
> other branch.
Hmm, looking at the code again it indeed seems that patch 1/5 + 2/5 could
be simply merged through the watchdog tree, as they don't appear to
have any interdependencies with the arch/x86 and drivers/platform/x86
changes. Andy ?
Regards,
Hans
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-23 12:05 ` Hans de Goede
@ 2021-01-25 11:15 ` Andy Shevchenko
2021-01-25 11:23 ` Hans de Goede
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-25 11:15 UTC (permalink / raw)
To: Hans de Goede
Cc: Guenter Roeck, Andy Shevchenko, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Platform Driver, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
On Sat, Jan 23, 2021 at 2:08 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 1/23/21 1:27 AM, Guenter Roeck wrote:
> > On 1/22/21 1:04 PM, Hans de Goede wrote:
> >> On 1/22/21 4:45 PM, Andy Shevchenko wrote:
> >>> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
> >>>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
> >>>>> What is the plan for merging this series ?
> >>>>>
> >>>>> It touches files under:
> >>>>> arch/x86
> >>>>> drivers/watchdog
> >>>>> drivers/platform/x86
> >>>>>
> >>>>> It is probably best if the entire series is merged through a single
> >>>>> tree. I don't expect this to cause any conflicts with current / upcoming
> >>>>> changes under drivers/platform/x86, so I'm fine with this being merged
> >>>>> through another tree.
> >>>>>
> >>>>> Or if I can get an ack for that from the x86 and watchdog maintainers
> >>>>> I can merge the entire series through the pdx86 tree.
> >>>>
> >>>> For time being I'm collecting tags and comments.
> >>>> As of today this series is independent per se, but other (later) clean up
> >>>> patches may rely on this.
> >>>>
> >>>> One strategy can be providing an immutable branch for anybody who wants it and
> >>>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
> >>>> are more patches touching other stuff, though independently, in this matters).
> >>>
> >>> Yes, I think one more time and guess the best is:
> >>> - get tags from x86 / watchdog
> >>> - get tag from RTC
> >>> - collect everything in PDx86 immutable branch
> >>
> >> Ok, that works for me. I assume that it is best for the other subsystems
> >> if I create an immutable branch for this based on 5.11-rc1
> >> (and then merge that into pdx86/for-next) ?
> >>
> >> Guenter, I believe you have already reviewed all the watchdog changes,
> >> are you ok with me creating an immutable branch for the entire series
> >> and then sending you a pull-req for that ?
> >
> > Wim handles watchdog pull requests,
Wim, can you Ack the watchdog changes?
> Ah, sorry I was under the impression you would do that because you were
> doing the reviews (which I'm sure Wim appreciates).
>
> > and he usually creates a branch
> > for Linus to pull from either during or shortly before a commit window.
> > Also, I don't immediately see why the watchdog tree should carry the other
> > patches of this series, immutable or not; I would not expect a dependency
> > against those.
> >
> > Anyway, I am fine if the the wdt driver removal is submitted through some
> > other branch.
>
> Hmm, looking at the code again it indeed seems that patch 1/5 + 2/5 could
> be simply merged through the watchdog tree, as they don't appear to
> have any interdependencies with the arch/x86 and drivers/platform/x86
> changes. Andy ?
The first patch is a dependency to the later (that are not published yet).
I was considering gathering all dependent patches in immutable
branches and doing in three parts / phases.
I can prepare a branch with a first batch (with Acks from respective
maintainers) and send a PR to you.
Would it work?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-25 11:15 ` Andy Shevchenko
@ 2021-01-25 11:23 ` Hans de Goede
2021-01-25 13:52 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2021-01-25 11:23 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Guenter Roeck, Andy Shevchenko, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Platform Driver, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
Hi,
On 1/25/21 12:15 PM, Andy Shevchenko wrote:
> On Sat, Jan 23, 2021 at 2:08 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 1/23/21 1:27 AM, Guenter Roeck wrote:
>>> On 1/22/21 1:04 PM, Hans de Goede wrote:
>>>> On 1/22/21 4:45 PM, Andy Shevchenko wrote:
>>>>> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
>>>>>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
>
>>>>>>> What is the plan for merging this series ?
>>>>>>>
>>>>>>> It touches files under:
>>>>>>> arch/x86
>>>>>>> drivers/watchdog
>>>>>>> drivers/platform/x86
>>>>>>>
>>>>>>> It is probably best if the entire series is merged through a single
>>>>>>> tree. I don't expect this to cause any conflicts with current / upcoming
>>>>>>> changes under drivers/platform/x86, so I'm fine with this being merged
>>>>>>> through another tree.
>>>>>>>
>>>>>>> Or if I can get an ack for that from the x86 and watchdog maintainers
>>>>>>> I can merge the entire series through the pdx86 tree.
>>>>>>
>>>>>> For time being I'm collecting tags and comments.
>>>>>> As of today this series is independent per se, but other (later) clean up
>>>>>> patches may rely on this.
>>>>>>
>>>>>> One strategy can be providing an immutable branch for anybody who wants it and
>>>>>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
>>>>>> are more patches touching other stuff, though independently, in this matters).
>>>>>
>>>>> Yes, I think one more time and guess the best is:
>>>>> - get tags from x86 / watchdog
>>>>> - get tag from RTC
>>>>> - collect everything in PDx86 immutable branch
>>>>
>>>> Ok, that works for me. I assume that it is best for the other subsystems
>>>> if I create an immutable branch for this based on 5.11-rc1
>>>> (and then merge that into pdx86/for-next) ?
>>>>
>>>> Guenter, I believe you have already reviewed all the watchdog changes,
>>>> are you ok with me creating an immutable branch for the entire series
>>>> and then sending you a pull-req for that ?
>>>
>>> Wim handles watchdog pull requests,
>
> Wim, can you Ack the watchdog changes?
>
>> Ah, sorry I was under the impression you would do that because you were
>> doing the reviews (which I'm sure Wim appreciates).
>>
>>> and he usually creates a branch
>>> for Linus to pull from either during or shortly before a commit window.
>>> Also, I don't immediately see why the watchdog tree should carry the other
>>> patches of this series, immutable or not; I would not expect a dependency
>>> against those.
>>>
>>> Anyway, I am fine if the the wdt driver removal is submitted through some
>>> other branch.
>>
>> Hmm, looking at the code again it indeed seems that patch 1/5 + 2/5 could
>> be simply merged through the watchdog tree, as they don't appear to
>> have any interdependencies with the arch/x86 and drivers/platform/x86
>> changes. Andy ?
>
> The first patch is a dependency to the later (that are not published yet).
> I was considering gathering all dependent patches in immutable
> branches and doing in three parts / phases.
>
> I can prepare a branch with a first batch (with Acks from respective
> maintainers) and send a PR to you.
> Would it work?
I guess you would then publish this as an immutable branch yourself and
send a PR to all involved subsys-maintainers ?
You have commit rights to the pdx86/platform-drivers-x86 git repo if
I'm not mistaken, so you could publish the immutable branch there if
you want.
That actually sounds like a good plan to me, assuming the other maintainers
are ok with it too.
Regards,
Hans
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v1 1/5] watchdog: intel_scu_watchdog: Remove driver for deprecated platform
2021-01-25 11:23 ` Hans de Goede
@ 2021-01-25 13:52 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2021-01-25 13:52 UTC (permalink / raw)
To: Hans de Goede
Cc: Guenter Roeck, Andy Shevchenko, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Platform Driver, Mark Gross, linux-watchdog,
Wim Van Sebroeck, Linus Walleij, Alessandro Zummo,
Alexandre Belloni, Arnd Bergmann, Lee Jones
On Mon, Jan 25, 2021 at 1:23 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 1/25/21 12:15 PM, Andy Shevchenko wrote:
> > On Sat, Jan 23, 2021 at 2:08 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 1/23/21 1:27 AM, Guenter Roeck wrote:
> >>> On 1/22/21 1:04 PM, Hans de Goede wrote:
> >>>> On 1/22/21 4:45 PM, Andy Shevchenko wrote:
> >>>>> On Fri, Jan 22, 2021 at 05:22:25PM +0200, Andy Shevchenko wrote:
> >>>>>> On Fri, Jan 22, 2021 at 03:59:34PM +0100, Hans de Goede wrote:
> >
> >>>>>>> What is the plan for merging this series ?
> >>>>>>>
> >>>>>>> It touches files under:
> >>>>>>> arch/x86
> >>>>>>> drivers/watchdog
> >>>>>>> drivers/platform/x86
> >>>>>>>
> >>>>>>> It is probably best if the entire series is merged through a single
> >>>>>>> tree. I don't expect this to cause any conflicts with current / upcoming
> >>>>>>> changes under drivers/platform/x86, so I'm fine with this being merged
> >>>>>>> through another tree.
> >>>>>>>
> >>>>>>> Or if I can get an ack for that from the x86 and watchdog maintainers
> >>>>>>> I can merge the entire series through the pdx86 tree.
> >>>>>>
> >>>>>> For time being I'm collecting tags and comments.
> >>>>>> As of today this series is independent per se, but other (later) clean up
> >>>>>> patches may rely on this.
> >>>>>>
> >>>>>> One strategy can be providing an immutable branch for anybody who wants it and
> >>>>>> merge thru PDx86 or watchdog tree (I guess PDx86 is preferable because there
> >>>>>> are more patches touching other stuff, though independently, in this matters).
> >>>>>
> >>>>> Yes, I think one more time and guess the best is:
> >>>>> - get tags from x86 / watchdog
> >>>>> - get tag from RTC
> >>>>> - collect everything in PDx86 immutable branch
> >>>>
> >>>> Ok, that works for me. I assume that it is best for the other subsystems
> >>>> if I create an immutable branch for this based on 5.11-rc1
> >>>> (and then merge that into pdx86/for-next) ?
> >>>>
> >>>> Guenter, I believe you have already reviewed all the watchdog changes,
> >>>> are you ok with me creating an immutable branch for the entire series
> >>>> and then sending you a pull-req for that ?
> >>>
> >>> Wim handles watchdog pull requests,
> >
> > Wim, can you Ack the watchdog changes?
Ah, I missed the fact that Guenter *is* one of the WDT maintainers, so
we are fine here.
> >> Ah, sorry I was under the impression you would do that because you were
> >> doing the reviews (which I'm sure Wim appreciates).
> >>
> >>> and he usually creates a branch
> >>> for Linus to pull from either during or shortly before a commit window.
> >>> Also, I don't immediately see why the watchdog tree should carry the other
> >>> patches of this series, immutable or not; I would not expect a dependency
> >>> against those.
> >>>
> >>> Anyway, I am fine if the the wdt driver removal is submitted through some
> >>> other branch.
> >>
> >> Hmm, looking at the code again it indeed seems that patch 1/5 + 2/5 could
> >> be simply merged through the watchdog tree, as they don't appear to
> >> have any interdependencies with the arch/x86 and drivers/platform/x86
> >> changes. Andy ?
> >
> > The first patch is a dependency to the later (that are not published yet).
> > I was considering gathering all dependent patches in immutable
> > branches and doing in three parts / phases.
> >
> > I can prepare a branch with a first batch (with Acks from respective
> > maintainers) and send a PR to you.
> > Would it work?
>
> I guess you would then publish this as an immutable branch yourself and
> send a PR to all involved subsys-maintainers ?
Right, I may do this.
> You have commit rights to the pdx86/platform-drivers-x86 git repo if
> I'm not mistaken, so you could publish the immutable branch there if
> you want.
>
> That actually sounds like a good plan to me, assuming the other maintainers
> are ok with it too.
Oki-doki, lemme do this way then. Thanks!
Taking into account above (Guenter had given his tags) I will prepare
an immutable branch and create a tag with current stuff and will send
it to appropriate subsystems, including PDx86.
Tell me if I need to do something differently.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread