[PATCH 1/3] dt-bindings: watchdog: add Broadcom's BCM63xx watchdog

[PATCH 1/3] dt-bindings: watchdog: add Broadcom's BCM63xx watchdog

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

It's a block that can be found on old MIPS devices as well as on
relatively new BCM4908.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../bindings/watchdog/brcm,bcm63xx-wdt.yaml   | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml
new file mode 100644
index 000000000000..f89bdefdeb19
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/brcm,bcm63xx-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BCM63xx watchdog
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm3368-wdt
+      - brcm,bcm4908-wdt
+      - brcm,bcm6328-wdt
+      - brcm,bcm6338-wdt
+      - brcm,bcm6345-wdt
+      - brcm,bcm6348-wdt
+      - brcm,bcm6358-wdt
+      - brcm,bcm6362-wdt
+      - brcm,bcm6368-wdt
+      - brcm,bcm6838-wdt
+      - brcm,bcm6848-wdt
+      - brcm,bcm6858-wdt
+      - brcm,bcm60333-wdt
+      - brcm,bcm63138-wdt
+      - brcm,bcm63148-wdt
+      - brcm,bcm63268-wdt
+      - brcm,bcm63381-wdt
+      - brcm,bcm68360-wdt
+
+  reg:
+    maxItems: 1
+
+unevaluatedProperties: false
+
+required:
+  - reg
+
+examples:
+  - |
+    watchdog@ff800400 {
+      compatible = "brcm,bcm4908-wdt";
+      reg = <0xff800400 0x3c>;
+    };
-- 
2.31.1

[PATCH 2/3] watchdog: bcm63xx_wdt: support compiling with COMPILE_TEST

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Most of this driver uses generic code and can be compile tested on any
platform / arch. Move watchdog specific defines to a globally available
header and check for BCM63XX for platform specific code.

This helps maintaining kernel / driver and will allow adding support for
other platforms later.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/mips/bcm63xx/prom.c                      |  1 +
 arch/mips/bcm63xx/setup.c                     |  1 +
 .../include/asm/mach-bcm63xx/bcm63xx_regs.h   | 22 ------------
 drivers/watchdog/Kconfig                      |  2 +-
 drivers/watchdog/bcm63xx_wdt.c                | 35 +++++++++++++++----
 include/linux/bcm63xx_wdt.h                   | 24 +++++++++++++
 6 files changed, 55 insertions(+), 30 deletions(-)
 create mode 100644 include/linux/bcm63xx_wdt.h

diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
index c3a2ea62c5c3..5e602a58a986 100644
--- a/arch/mips/bcm63xx/prom.c
+++ b/arch/mips/bcm63xx/prom.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/memblock.h>
 #include <linux/smp.h>
diff --git a/arch/mips/bcm63xx/setup.c b/arch/mips/bcm63xx/setup.c
index d811e3e03f81..86d4ad7dc8ba 100644
--- a/arch/mips/bcm63xx/setup.c
+++ b/arch/mips/bcm63xx/setup.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2008 Maxime Bizon <mbizon@freebox.fr>
  */
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
index 9ceb5e72889f..fc0272e2dec2 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
@@ -441,28 +441,6 @@
 #define TIMER_CTL_ENABLE_MASK		(1 << 31)
 
 
-/*************************************************************************
- * _REG relative to RSET_WDT
- *************************************************************************/
-
-/* Watchdog default count register */
-#define WDT_DEFVAL_REG			0x0
-
-/* Watchdog control register */
-#define WDT_CTL_REG			0x4
-
-/* Watchdog control register constants */
-#define WDT_START_1			(0xff00)
-#define WDT_START_2			(0x00ff)
-#define WDT_STOP_1			(0xee00)
-#define WDT_STOP_2			(0x00ee)
-
-/* Watchdog reset length register */
-#define WDT_RSTLEN_REG			0x8
-
-/* Watchdog soft reset register (BCM6328 only) */
-#define WDT_SOFTRESET_REG		0xc
-
 /*************************************************************************
  * _REG relative to RSET_GPIO
  *************************************************************************/
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index bf59faeb3de1..913cde027e92 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1711,7 +1711,7 @@ config OCTEON_WDT
 
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
-	depends on BCM63XX
+	depends on BCM63XX || COMPILE_TEST
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 56cc262571a5..1334e7fe77d8 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -9,6 +9,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/bcm63xx_wdt.h>
 #include <linux/bitops.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
@@ -27,10 +28,10 @@
 #include <linux/resource.h>
 #include <linux/platform_device.h>
 
-#include <bcm63xx_cpu.h>
+#ifdef BCM63XX
 #include <bcm63xx_io.h>
-#include <bcm63xx_regs.h>
 #include <bcm63xx_timer.h>
+#endif
 
 #define PFX KBUILD_MODNAME
 
@@ -53,26 +54,40 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+static void bcm63xx_wdt_write_reg(u32 reg, u32 val)
+{
+	void __iomem *addr = bcm63xx_wdt_device.regs;
+
+	addr += reg;
+#ifdef BCM63XX
+	bcm_writel(val, addr);
+#else
+	writel(val, addr);
+#endif
+}
+
 /* HW functions */
 static void bcm63xx_wdt_hw_start(void)
 {
-	bcm_writel(0xfffffffe, bcm63xx_wdt_device.regs + WDT_DEFVAL_REG);
-	bcm_writel(WDT_START_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_START_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_write_reg(WDT_DEFVAL_REG, 0xfffffffe);
+	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_START_1);
+	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_START_2);
 }
 
 static void bcm63xx_wdt_hw_stop(void)
 {
-	bcm_writel(WDT_STOP_1, bcm63xx_wdt_device.regs + WDT_CTL_REG);
-	bcm_writel(WDT_STOP_2, bcm63xx_wdt_device.regs + WDT_CTL_REG);
+	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_STOP_1);
+	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_STOP_2);
 }
 
+#ifdef BCM63XX
 static void bcm63xx_wdt_isr(void *data)
 {
 	struct pt_regs *regs = get_irq_regs();
 
 	die(PFX " fire", regs);
 }
+#endif
 
 static void bcm63xx_timer_tick(struct timer_list *unused)
 {
@@ -255,11 +270,13 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+#ifdef BCM63XX
 	ret = bcm63xx_timer_register(TIMER_WDT_ID, bcm63xx_wdt_isr, NULL);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to register wdt timer isr\n");
 		return ret;
 	}
+#endif
 
 	if (bcm63xx_wdt_settimeout(wdt_time)) {
 		bcm63xx_wdt_settimeout(WDT_DEFAULT_TIME);
@@ -280,7 +297,9 @@ static int bcm63xx_wdt_probe(struct platform_device *pdev)
 	return 0;
 
 unregister_timer:
+#ifdef BCM63XX
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
+#endif
 	return ret;
 }
 
@@ -290,7 +309,9 @@ static int bcm63xx_wdt_remove(struct platform_device *pdev)
 		bcm63xx_wdt_pause();
 
 	misc_deregister(&bcm63xx_wdt_miscdev);
+#ifdef BCM63XX
 	bcm63xx_timer_unregister(TIMER_WDT_ID);
+#endif
 	return 0;
 }
 
diff --git a/include/linux/bcm63xx_wdt.h b/include/linux/bcm63xx_wdt.h
new file mode 100644
index 000000000000..a33d23237b32
--- /dev/null
+++ b/include/linux/bcm63xx_wdt.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __BCM63XX_WDT_H
+#define __BCM63XX_WDT_H
+
+/* Watchdog default count register */
+#define WDT_DEFVAL_REG			0x0
+
+/* Watchdog control register */
+#define WDT_CTL_REG			0x4
+
+/* Watchdog control register constants */
+#define WDT_START_1			(0xff00)
+#define WDT_START_2			(0x00ff)
+#define WDT_STOP_1			(0xee00)
+#define WDT_STOP_2			(0x00ee)
+
+/* Watchdog reset length register */
+#define WDT_RSTLEN_REG			0x8
+
+/* Watchdog soft reset register (BCM6328 only) */
+#define WDT_SOFTRESET_REG		0xc
+
+#endif /* __BCM63XX_WDT_H */
-- 
2.31.1

[PATCH 3/3] watchdog: bcm63xx_wdt: support BCM4908

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

So far bcm63xx_wdt included support for Broadcom's watchdog block of old
MIPS devices only. It was also a fully platform (non-DT) driver.

The same block was recently found on BCM4908 with just a slightly
different registers layout. Extend this driver to support OF and add
support for per-chipset registers offsets.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/watchdog/Kconfig       |  2 +-
 drivers/watchdog/bcm63xx_wdt.c | 63 ++++++++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 913cde027e92..58a2474cd929 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1711,7 +1711,7 @@ config OCTEON_WDT
 
 config BCM63XX_WDT
 	tristate "Broadcom BCM63xx hardware watchdog"
-	depends on BCM63XX || COMPILE_TEST
+	depends on ARCH_BCM4908 || BCM63XX || COMPILE_TEST
 	help
 	  Watchdog driver for the built in watchdog hardware in Broadcom
 	  BCM63xx SoC.
diff --git a/drivers/watchdog/bcm63xx_wdt.c b/drivers/watchdog/bcm63xx_wdt.c
index 1334e7fe77d8..df3c7dfea00f 100644
--- a/drivers/watchdog/bcm63xx_wdt.c
+++ b/drivers/watchdog/bcm63xx_wdt.c
@@ -18,6 +18,7 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of_device.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
@@ -39,7 +40,13 @@
 #define WDT_DEFAULT_TIME	30      /* seconds */
 #define WDT_MAX_TIME		256     /* seconds */
 
+enum bcm63xx_wdt_soc {
+	BCM63XX_WDT_SOC_BCM4908,
+	BCM63XX_WDT_SOC_BCM63XX,
+};
+
 static struct {
+	enum bcm63xx_wdt_soc soc;
 	void __iomem *regs;
 	struct timer_list timer;
 	unsigned long inuse;
@@ -54,11 +61,41 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-static void bcm63xx_wdt_write_reg(u32 reg, u32 val)
+static const struct of_device_id bcm63xx_wdt_id_table[] = {
+	{ .compatible = "brcm,bcm4908-wdt", .data = (const void *)BCM63XX_WDT_SOC_BCM4908, },
+	{},
+};
+
+enum bcm63xx_wdt_regs {
+	BCM63XX_WDT_REG_DEFVAL = 0,
+	BCM63XX_WDT_REG_CTL,
+	BCM63XX_WDT_REG_SOFTRESET,
+};
+
+static const u16 bcm63xx_wdt_regs_bcm4908[] = {
+	[BCM63XX_WDT_REG_DEFVAL]	= 0x28,
+	[BCM63XX_WDT_REG_CTL]		= 0x2c,
+	[BCM63XX_WDT_REG_SOFTRESET]	= 0x34,
+};
+
+static const u16 bcm63xx_wdt_regs_bcm63xx[] = {
+	[BCM63XX_WDT_REG_DEFVAL]	= WDT_DEFVAL_REG,
+	[BCM63XX_WDT_REG_CTL]		= WDT_CTL_REG,
+};
+
+static void bcm63xx_wdt_write_reg(enum bcm63xx_wdt_regs reg, u32 val)
 {
 	void __iomem *addr = bcm63xx_wdt_device.regs;
 
-	addr += reg;
+	switch (bcm63xx_wdt_device.soc) {
+	case BCM63XX_WDT_SOC_BCM4908:
+		addr += bcm63xx_wdt_regs_bcm4908[reg];
+		break;
+	case BCM63XX_WDT_SOC_BCM63XX:
+		addr += bcm63xx_wdt_regs_bcm63xx[reg];
+		break;
+	}
+
 #ifdef BCM63XX
 	bcm_writel(val, addr);
 #else
@@ -69,15 +106,15 @@ static void bcm63xx_wdt_write_reg(u32 reg, u32 val)
 /* HW functions */
 static void bcm63xx_wdt_hw_start(void)
 {
-	bcm63xx_wdt_write_reg(WDT_DEFVAL_REG, 0xfffffffe);
-	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_START_1);
-	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_START_2);
+	bcm63xx_wdt_write_reg(BCM63XX_WDT_REG_DEFVAL, 0xfffffffe);
+	bcm63xx_wdt_write_reg(BCM63XX_WDT_REG_CTL, WDT_START_1);
+	bcm63xx_wdt_write_reg(BCM63XX_WDT_REG_CTL, WDT_START_2);
 }
 
 static void bcm63xx_wdt_hw_stop(void)
 {
-	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_STOP_1);
-	bcm63xx_wdt_write_reg(WDT_CTL_REG, WDT_STOP_2);
+	bcm63xx_wdt_write_reg(BCM63XX_WDT_REG_CTL, WDT_STOP_1);
+	bcm63xx_wdt_write_reg(BCM63XX_WDT_REG_CTL, WDT_STOP_2);
 }
 
 #ifdef BCM63XX
@@ -252,9 +289,19 @@ static struct miscdevice bcm63xx_wdt_miscdev = {
 
 static int bcm63xx_wdt_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
 	int ret;
 	struct resource *r;
 
+	of_id = of_match_device(bcm63xx_wdt_id_table, dev);
+	if (of_id) {
+		bcm63xx_wdt_device.soc = (enum bcm63xx_wdt_soc)of_id->data;
+	} else {
+		/* Fallback */
+		bcm63xx_wdt_device.soc = BCM63XX_WDT_SOC_BCM63XX;
+	}
+
 	timer_setup(&bcm63xx_wdt_device.timer, bcm63xx_timer_tick, 0);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -326,6 +373,7 @@ static struct platform_driver bcm63xx_wdt_driver = {
 	.shutdown = bcm63xx_wdt_shutdown,
 	.driver = {
 		.name = "bcm63xx-wdt",
+		.of_match_table = bcm63xx_wdt_id_table,
 	}
 };
 
@@ -333,6 +381,7 @@ module_platform_driver(bcm63xx_wdt_driver);
 
 MODULE_AUTHOR("Miguel Gaio <miguel.gaio@efixo.com>");
 MODULE_AUTHOR("Florian Fainelli <florian@openwrt.org>");
+MODULE_DEVICE_TABLE(of, bcm63xx_wdt_id_table);
 MODULE_DESCRIPTION("Driver for the Broadcom BCM63xx SoC watchdog");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:bcm63xx-wdt");
-- 
2.31.1

Re: [PATCH 3/3] watchdog: bcm63xx_wdt: support BCM4908

[Guenter Roeck]

On Wed, Oct 27, 2021 at 11:55:31PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> So far bcm63xx_wdt included support for Broadcom's watchdog block of old
> MIPS devices only. It was also a fully platform (non-DT) driver.
> 
> The same block was recently found on BCM4908 with just a slightly
> different registers layout. Extend this driver to support OF and add
> support for per-chipset registers offsets.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Please convert the driver to use the watchdog subsystem first.
Also, when doing so, please explain why you don't use the bcm7038
driver instead, since it serves the same hardware interface and
already supports the watchdog core.

Guenter

Re: [PATCH 3/3] watchdog: bcm63xx_wdt: support BCM4908

[Florian Fainelli]

On 10/27/21 3:22 PM, Guenter Roeck wrote:
> On Wed, Oct 27, 2021 at 11:55:31PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> So far bcm63xx_wdt included support for Broadcom's watchdog block of old
>> MIPS devices only. It was also a fully platform (non-DT) driver.
>>
>> The same block was recently found on BCM4908 with just a slightly
>> different registers layout. Extend this driver to support OF and add
>> support for per-chipset registers offsets.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> 
> Please convert the driver to use the watchdog subsystem first.
> Also, when doing so, please explain why you don't use the bcm7038
> driver instead, since it serves the same hardware interface and
> already supports the watchdog core.

This is too ugly unfortunately so yes let us use bcm7038_wdt.c and let
me know if you need help with converting arch/mips/bcm63xx to use the
bcm7038_wdt.c
-- 
Florian

Re: [PATCH 1/3] dt-bindings: watchdog: add Broadcom's BCM63xx watchdog

[Florian Fainelli]

On 10/27/21 2:55 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It's a block that can be found on old MIPS devices as well as on
> relatively new BCM4908.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../bindings/watchdog/brcm,bcm63xx-wdt.yaml   | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml
> new file mode 100644
> index 000000000000..f89bdefdeb19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm63xx-wdt.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/brcm,bcm63xx-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BCM63xx watchdog
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"
> +
> +maintainers:
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm3368-wdt
> +      - brcm,bcm4908-wdt
> +      - brcm,bcm6328-wdt
> +      - brcm,bcm6338-wdt
> +      - brcm,bcm6345-wdt
> +      - brcm,bcm6348-wdt
> +      - brcm,bcm6358-wdt
> +      - brcm,bcm6362-wdt
> +      - brcm,bcm6368-wdt
> +      - brcm,bcm6838-wdt
> +      - brcm,bcm6848-wdt
> +      - brcm,bcm6858-wdt
> +      - brcm,bcm60333-wdt
> +      - brcm,bcm63138-wdt
> +      - brcm,bcm63148-wdt
> +      - brcm,bcm63268-wdt
> +      - brcm,bcm63381-wdt
> +      - brcm,bcm68360-wdt

That's a lot of compatible strings for a piece of hardware that has not
really change from the venerable BCM6345 days. I believe if you use only
"brcm,bcm6345-wdt" we ought to have everything covered. Similar to the
bcm63xx-uart binding.
-- 
Florian

Re: [PATCH 1/3] dt-bindings: watchdog: add Broadcom's BCM63xx watchdog

[Rafał Miłecki]

śr., 27 paź 2021 o 23:55 Rafał Miłecki <zajec5@gmail.com> napisał(a):
> From: Rafał Miłecki <rafal@milecki.pl>
>
> It's a block that can be found on old MIPS devices as well as on
> relatively new BCM4908.

After receiving some feedback: please drop this patchset.