* [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:29 ` Guenter Roeck
2018-11-20 17:19 ` [PATCH 2/8] soc: bcm: bcm2835-pm: Rename the driver to its new "PM" name Eric Anholt
` (7 subsequent siblings)
8 siblings, 1 reply; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
The binding for the bcm2835 "wdt" actually covers a large range of the
PM block's register space. The WDT is not a separate HW block from
the rest of power domain management, so move the driver to the soc/
directory in preparation for expanding its role to cover power
domains.
This move does result in the driver being made mandatory for the
BCM2835 platform, which is probably actually reasonable given that
it's necessary for reboot/halt support.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
drivers/soc/bcm/Makefile | 1 +
.../{watchdog/bcm2835_wdt.c => soc/bcm/bcm2835-pm.c} | 0
drivers/watchdog/Kconfig | 11 -----------
drivers/watchdog/Makefile | 1 -
4 files changed, 1 insertion(+), 12 deletions(-)
rename drivers/{watchdog/bcm2835_wdt.c => soc/bcm/bcm2835-pm.c} (100%)
diff --git a/drivers/soc/bcm/Makefile b/drivers/soc/bcm/Makefile
index dc4fced72d21..16504eb694b1 100644
--- a/drivers/soc/bcm/Makefile
+++ b/drivers/soc/bcm/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
obj-$(CONFIG_RASPBERRYPI_POWER) += raspberrypi-power.o
obj-$(CONFIG_SOC_BRCMSTB) += brcmstb/
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/soc/bcm/bcm2835-pm.c
similarity index 100%
rename from drivers/watchdog/bcm2835_wdt.c
rename to drivers/soc/bcm/bcm2835-pm.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 2d64333f4782..796e2a593056 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1573,17 +1573,6 @@ config BCM63XX_WDT
To compile this driver as a loadable module, choose M here.
The module will be called bcm63xx_wdt.
-config BCM2835_WDT
- tristate "Broadcom BCM2835 hardware watchdog"
- depends on ARCH_BCM2835 || (OF && COMPILE_TEST)
- select WATCHDOG_CORE
- help
- Watchdog driver for the built in watchdog hardware in Broadcom
- BCM2835 SoC.
-
- To compile this driver as a loadable module, choose M here.
- The module will be called bcm2835_wdt.
-
config BCM_KONA_WDT
tristate "BCM Kona Watchdog"
depends on ARCH_BCM_MOBILE || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f69cdff5ad7f..1788537e85af 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -70,7 +70,6 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
-obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o
obj-$(CONFIG_ST_LPC_WATCHDOG) += st_lpc_wdt.o
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory.
2018-11-20 17:19 ` [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory Eric Anholt
@ 2018-11-20 17:29 ` Guenter Roeck
0 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2018-11-20 17:29 UTC (permalink / raw)
To: Eric Anholt
Cc: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
linux-watchdog, linux-rpi-kernel, linux-arm-kernel, linux-kernel,
Stefan Wahren, bcm-kernel-feedback-list
On Tue, Nov 20, 2018 at 09:19:53AM -0800, Eric Anholt wrote:
> The binding for the bcm2835 "wdt" actually covers a large range of the
> PM block's register space. The WDT is not a separate HW block from
> the rest of power domain management, so move the driver to the soc/
> directory in preparation for expanding its role to cover power
> domains.
>
> This move does result in the driver being made mandatory for the
> BCM2835 platform, which is probably actually reasonable given that
> it's necessary for reboot/halt support.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
Keeping drivers out of their domain tends to have the effect of maintainers
not being aware of changes, which in turn tends to result in bad code.
I have seen that happen a lot with hwmon drivers, and I am not in favor
of it. It would be better to keep the watchdog code where it is and have
it instantiated from a soc parent, which could pass, for example, regmap
information to the driver for register accesses.
If the new SoC approach is to move everything into SoC, you'll be on
your own. I won't NACK this, but I won't ACK it either.
Guenter
> ---
> drivers/soc/bcm/Makefile | 1 +
> .../{watchdog/bcm2835_wdt.c => soc/bcm/bcm2835-pm.c} | 0
> drivers/watchdog/Kconfig | 11 -----------
> drivers/watchdog/Makefile | 1 -
> 4 files changed, 1 insertion(+), 12 deletions(-)
> rename drivers/{watchdog/bcm2835_wdt.c => soc/bcm/bcm2835-pm.c} (100%)
>
> diff --git a/drivers/soc/bcm/Makefile b/drivers/soc/bcm/Makefile
> index dc4fced72d21..16504eb694b1 100644
> --- a/drivers/soc/bcm/Makefile
> +++ b/drivers/soc/bcm/Makefile
> @@ -1,2 +1,3 @@
> +obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
> obj-$(CONFIG_RASPBERRYPI_POWER) += raspberrypi-power.o
> obj-$(CONFIG_SOC_BRCMSTB) += brcmstb/
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/soc/bcm/bcm2835-pm.c
> similarity index 100%
> rename from drivers/watchdog/bcm2835_wdt.c
> rename to drivers/soc/bcm/bcm2835-pm.c
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 2d64333f4782..796e2a593056 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1573,17 +1573,6 @@ config BCM63XX_WDT
> To compile this driver as a loadable module, choose M here.
> The module will be called bcm63xx_wdt.
>
> -config BCM2835_WDT
> - tristate "Broadcom BCM2835 hardware watchdog"
> - depends on ARCH_BCM2835 || (OF && COMPILE_TEST)
> - select WATCHDOG_CORE
> - help
> - Watchdog driver for the built in watchdog hardware in Broadcom
> - BCM2835 SoC.
> -
> - To compile this driver as a loadable module, choose M here.
> - The module will be called bcm2835_wdt.
> -
> config BCM_KONA_WDT
> tristate "BCM Kona Watchdog"
> depends on ARCH_BCM_MOBILE || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index f69cdff5ad7f..1788537e85af 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -70,7 +70,6 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
> obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> -obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
> obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o
> obj-$(CONFIG_ST_LPC_WATCHDOG) += st_lpc_wdt.o
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/8] soc: bcm: bcm2835-pm: Rename the driver to its new "PM" name.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
2018-11-20 17:19 ` [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 3/8] soc: bcm: bcm2835-pm: Stop using _relaxed mmio accessors Eric Anholt
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
This is the name of the block we're controlling.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
drivers/soc/bcm/bcm2835-pm.c | 92 ++++++++++++++++++------------------
1 file changed, 47 insertions(+), 45 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-pm.c b/drivers/soc/bcm/bcm2835-pm.c
index ed05514cc2dc..ca37145e4e40 100644
--- a/drivers/soc/bcm/bcm2835-pm.c
+++ b/drivers/soc/bcm/bcm2835-pm.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Watchdog driver for Broadcom BCM2835
+ * PM driver for Broadcom BCM2835
*
* "bcm2708_wdog" driver written by Luke Diamand that was obtained from
* branch "rpi-3.6.y" of git://github.com/raspberrypi/linux.git was used
@@ -42,7 +42,7 @@
#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
-struct bcm2835_wdt {
+struct bcm2835_pm {
void __iomem *base;
spinlock_t lock;
};
@@ -50,60 +50,60 @@ struct bcm2835_wdt {
static unsigned int heartbeat;
static bool nowayout = WATCHDOG_NOWAYOUT;
-static bool bcm2835_wdt_is_running(struct bcm2835_wdt *wdt)
+static bool bcm2835_wdt_is_running(struct bcm2835_pm *pm)
{
uint32_t cur;
- cur = readl(wdt->base + PM_RSTC);
+ cur = readl(pm->base + PM_RSTC);
return !!(cur & PM_RSTC_WRCFG_FULL_RESET);
}
static int bcm2835_wdt_start(struct watchdog_device *wdog)
{
- struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+ struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
uint32_t cur;
unsigned long flags;
- spin_lock_irqsave(&wdt->lock, flags);
+ spin_lock_irqsave(&pm->lock, flags);
writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
- PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
- cur = readl_relaxed(wdt->base + PM_RSTC);
+ PM_WDOG_TIME_SET), pm->base + PM_WDOG);
+ cur = readl_relaxed(pm->base + PM_RSTC);
writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
- PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
+ PM_RSTC_WRCFG_FULL_RESET, pm->base + PM_RSTC);
- spin_unlock_irqrestore(&wdt->lock, flags);
+ spin_unlock_irqrestore(&pm->lock, flags);
return 0;
}
static int bcm2835_wdt_stop(struct watchdog_device *wdog)
{
- struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+ struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
+ writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, pm->base + PM_RSTC);
return 0;
}
static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
{
- struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+ struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
+ uint32_t ret = readl_relaxed(pm->base + PM_WDOG);
return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
}
-static void __bcm2835_restart(struct bcm2835_wdt *wdt)
+static void __bcm2835_restart(struct bcm2835_pm *pm)
{
u32 val;
/* use a timeout of 10 ticks (~150us) */
- writel_relaxed(10 | PM_PASSWORD, wdt->base + PM_WDOG);
- val = readl_relaxed(wdt->base + PM_RSTC);
+ writel_relaxed(10 | PM_PASSWORD, pm->base + PM_WDOG);
+ val = readl_relaxed(pm->base + PM_RSTC);
val &= PM_RSTC_WRCFG_CLR;
val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
- writel_relaxed(val, wdt->base + PM_RSTC);
+ writel_relaxed(val, pm->base + PM_RSTC);
/* No sleeping, possibly atomic. */
mdelay(1);
@@ -112,9 +112,9 @@ static void __bcm2835_restart(struct bcm2835_wdt *wdt)
static int bcm2835_restart(struct watchdog_device *wdog,
unsigned long action, void *data)
{
- struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+ struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- __bcm2835_restart(wdt);
+ __bcm2835_restart(pm);
return 0;
}
@@ -151,7 +151,7 @@ static void bcm2835_power_off(void)
struct device_node *np =
of_find_compatible_node(NULL, NULL, "brcm,bcm2835-pm-wdt");
struct platform_device *pdev = of_find_device_by_node(np);
- struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+ struct bcm2835_pm *pm = platform_get_drvdata(pdev);
u32 val;
/*
@@ -159,38 +159,39 @@ static void bcm2835_power_off(void)
* from the normal (full) reset. bootcode.bin will not reboot after a
* hard reset.
*/
- val = readl_relaxed(wdt->base + PM_RSTS);
+ val = readl_relaxed(pm->base + PM_RSTS);
val |= PM_PASSWORD | PM_RSTS_RASPBERRYPI_HALT;
- writel_relaxed(val, wdt->base + PM_RSTS);
+ writel_relaxed(val, pm->base + PM_RSTS);
/* Continue with normal reset mechanism */
- __bcm2835_restart(wdt);
+ __bcm2835_restart(pm);
}
-static int bcm2835_wdt_probe(struct platform_device *pdev)
+static int bcm2835_pm_probe(struct platform_device *pdev)
{
struct resource *res;
struct device *dev = &pdev->dev;
- struct bcm2835_wdt *wdt;
+ struct bcm2835_pm *pm;
int err;
- wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
- if (!wdt)
+ pm = devm_kzalloc(dev, sizeof(struct bcm2835_pm), GFP_KERNEL);
+ if (!pm)
return -ENOMEM;
- platform_set_drvdata(pdev, wdt);
+ platform_set_drvdata(pdev, pm);
- spin_lock_init(&wdt->lock);
+ spin_lock_init(&pm->lock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- wdt->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(wdt->base))
- return PTR_ERR(wdt->base);
+ pm->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(pm->base))
+ return PTR_ERR(pm->base);
- watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
+#if defined(CONFIG_WATCHDOG_CORE)
+ watchdog_set_drvdata(&bcm2835_wdt_wdd, pm);
watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
bcm2835_wdt_wdd.parent = dev;
- if (bcm2835_wdt_is_running(wdt)) {
+ if (bcm2835_wdt_is_running(pm)) {
/*
* The currently active timeout value (set by the
* bootloader) may be different from the module
@@ -210,6 +211,7 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
dev_err(dev, "Failed to register watchdog device");
return err;
}
+#endif
if (pm_power_off == NULL)
pm_power_off = bcm2835_power_off;
@@ -218,7 +220,7 @@ static int bcm2835_wdt_probe(struct platform_device *pdev)
return 0;
}
-static int bcm2835_wdt_remove(struct platform_device *pdev)
+static int bcm2835_pm_remove(struct platform_device *pdev)
{
if (pm_power_off == bcm2835_power_off)
pm_power_off = NULL;
@@ -226,21 +228,21 @@ static int bcm2835_wdt_remove(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id bcm2835_wdt_of_match[] = {
+static const struct of_device_id bcm2835_pm_of_match[] = {
{ .compatible = "brcm,bcm2835-pm-wdt", },
{},
};
-MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);
+MODULE_DEVICE_TABLE(of, bcm2835_pm_of_match);
-static struct platform_driver bcm2835_wdt_driver = {
- .probe = bcm2835_wdt_probe,
- .remove = bcm2835_wdt_remove,
+static struct platform_driver bcm2835_pm_driver = {
+ .probe = bcm2835_pm_probe,
+ .remove = bcm2835_pm_remove,
.driver = {
- .name = "bcm2835-wdt",
- .of_match_table = bcm2835_wdt_of_match,
+ .name = "bcm2835-pm",
+ .of_match_table = bcm2835_pm_of_match,
},
};
-module_platform_driver(bcm2835_wdt_driver);
+module_platform_driver(bcm2835_pm_driver);
module_param(heartbeat, uint, 0);
MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
@@ -250,5 +252,5 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
-MODULE_DESCRIPTION("Driver for Broadcom BCM2835 watchdog timer");
+MODULE_DESCRIPTION("Driver for Broadcom BCM2835 PM/WDT");
MODULE_LICENSE("GPL");
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/8] soc: bcm: bcm2835-pm: Stop using _relaxed mmio accessors.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
2018-11-20 17:19 ` [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory Eric Anholt
2018-11-20 17:19 ` [PATCH 2/8] soc: bcm: bcm2835-pm: Rename the driver to its new "PM" name Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 4/8] soc: bcm: bcm2835-pm: Make some little accessor macros for the mmio area Eric Anholt
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
We definitely don't want I/O to be reordered here (like the setting of
the WDT timeout before enabling the WDT). None of this is performance
critical, anyway.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
drivers/soc/bcm/bcm2835-pm.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-pm.c b/drivers/soc/bcm/bcm2835-pm.c
index ca37145e4e40..decc316fbe40 100644
--- a/drivers/soc/bcm/bcm2835-pm.c
+++ b/drivers/soc/bcm/bcm2835-pm.c
@@ -67,11 +67,11 @@ static int bcm2835_wdt_start(struct watchdog_device *wdog)
spin_lock_irqsave(&pm->lock, flags);
- writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
- PM_WDOG_TIME_SET), pm->base + PM_WDOG);
- cur = readl_relaxed(pm->base + PM_RSTC);
- writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
- PM_RSTC_WRCFG_FULL_RESET, pm->base + PM_RSTC);
+ writel(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
+ PM_WDOG_TIME_SET), pm->base + PM_WDOG);
+ cur = readl(pm->base + PM_RSTC);
+ writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
+ PM_RSTC_WRCFG_FULL_RESET, pm->base + PM_RSTC);
spin_unlock_irqrestore(&pm->lock, flags);
@@ -82,7 +82,7 @@ static int bcm2835_wdt_stop(struct watchdog_device *wdog)
{
struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, pm->base + PM_RSTC);
+ writel(PM_PASSWORD | PM_RSTC_RESET, pm->base + PM_RSTC);
return 0;
}
@@ -90,7 +90,7 @@ static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
{
struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- uint32_t ret = readl_relaxed(pm->base + PM_WDOG);
+ uint32_t ret = readl(pm->base + PM_WDOG);
return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
}
@@ -99,11 +99,11 @@ static void __bcm2835_restart(struct bcm2835_pm *pm)
u32 val;
/* use a timeout of 10 ticks (~150us) */
- writel_relaxed(10 | PM_PASSWORD, pm->base + PM_WDOG);
- val = readl_relaxed(pm->base + PM_RSTC);
+ writel(10 | PM_PASSWORD, pm->base + PM_WDOG);
+ val = readl(pm->base + PM_RSTC);
val &= PM_RSTC_WRCFG_CLR;
val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
- writel_relaxed(val, pm->base + PM_RSTC);
+ writel(val, pm->base + PM_RSTC);
/* No sleeping, possibly atomic. */
mdelay(1);
@@ -159,9 +159,9 @@ static void bcm2835_power_off(void)
* from the normal (full) reset. bootcode.bin will not reboot after a
* hard reset.
*/
- val = readl_relaxed(pm->base + PM_RSTS);
+ val = readl(pm->base + PM_RSTS);
val |= PM_PASSWORD | PM_RSTS_RASPBERRYPI_HALT;
- writel_relaxed(val, pm->base + PM_RSTS);
+ writel(val, pm->base + PM_RSTS);
/* Continue with normal reset mechanism */
__bcm2835_restart(pm);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/8] soc: bcm: bcm2835-pm: Make some little accessor macros for the mmio area.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (2 preceding siblings ...)
2018-11-20 17:19 ` [PATCH 3/8] soc: bcm: bcm2835-pm: Stop using _relaxed mmio accessors Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 5/8] dt-bindings: soc: Add a new binding for the BCM2835 PM node Eric Anholt
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
This is more legible than trying to remember to set PM_PASSWORD on
everything.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
drivers/soc/bcm/bcm2835-pm.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-pm.c b/drivers/soc/bcm/bcm2835-pm.c
index decc316fbe40..e504381556cf 100644
--- a/drivers/soc/bcm/bcm2835-pm.c
+++ b/drivers/soc/bcm/bcm2835-pm.c
@@ -32,6 +32,9 @@
#define PM_RSTC_WRCFG_FULL_RESET 0x00000020
#define PM_RSTC_RESET 0x00000102
+#define PM_READ(reg) readl(pm->base + (reg))
+#define PM_WRITE(reg, val) writel(PM_PASSWORD | (val), pm->base + (reg))
+
/*
* The Raspberry Pi firmware uses the RSTS register to know which partition
* to boot from. The partition value is spread into bits 0, 2, 4, 6, 8, 10.
@@ -54,7 +57,7 @@ static bool bcm2835_wdt_is_running(struct bcm2835_pm *pm)
{
uint32_t cur;
- cur = readl(pm->base + PM_RSTC);
+ cur = PM_READ(PM_RSTC);
return !!(cur & PM_RSTC_WRCFG_FULL_RESET);
}
@@ -67,11 +70,9 @@ static int bcm2835_wdt_start(struct watchdog_device *wdog)
spin_lock_irqsave(&pm->lock, flags);
- writel(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
- PM_WDOG_TIME_SET), pm->base + PM_WDOG);
- cur = readl(pm->base + PM_RSTC);
- writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
- PM_RSTC_WRCFG_FULL_RESET, pm->base + PM_RSTC);
+ PM_WRITE(PM_WDOG, SECS_TO_WDOG_TICKS(wdog->timeout) & PM_WDOG_TIME_SET);
+ cur = PM_READ(PM_RSTC);
+ PM_WRITE(PM_RSTC, (cur & PM_RSTC_WRCFG_CLR) | PM_RSTC_WRCFG_FULL_RESET);
spin_unlock_irqrestore(&pm->lock, flags);
@@ -82,7 +83,7 @@ static int bcm2835_wdt_stop(struct watchdog_device *wdog)
{
struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- writel(PM_PASSWORD | PM_RSTC_RESET, pm->base + PM_RSTC);
+ PM_WRITE(PM_RSTC, PM_RSTC_RESET);
return 0;
}
@@ -90,7 +91,7 @@ static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
{
struct bcm2835_pm *pm = watchdog_get_drvdata(wdog);
- uint32_t ret = readl(pm->base + PM_WDOG);
+ uint32_t ret = PM_READ(PM_WDOG);
return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
}
@@ -100,7 +101,7 @@ static void __bcm2835_restart(struct bcm2835_pm *pm)
/* use a timeout of 10 ticks (~150us) */
writel(10 | PM_PASSWORD, pm->base + PM_WDOG);
- val = readl(pm->base + PM_RSTC);
+ val = PM_READ(PM_RSTC);
val &= PM_RSTC_WRCFG_CLR;
val |= PM_PASSWORD | PM_RSTC_WRCFG_FULL_RESET;
writel(val, pm->base + PM_RSTC);
@@ -159,7 +160,7 @@ static void bcm2835_power_off(void)
* from the normal (full) reset. bootcode.bin will not reboot after a
* hard reset.
*/
- val = readl(pm->base + PM_RSTS);
+ val = PM_READ(PM_RSTS);
val |= PM_PASSWORD | PM_RSTS_RASPBERRYPI_HALT;
writel(val, pm->base + PM_RSTS);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/8] dt-bindings: soc: Add a new binding for the BCM2835 PM node.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (3 preceding siblings ...)
2018-11-20 17:19 ` [PATCH 4/8] soc: bcm: bcm2835-pm: Make some little accessor macros for the mmio area Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 6/8] soc: bcm: bcm2835-pm: Add support for power domains under a new binding Eric Anholt
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
This binding supersedes the bcm2835-pm-wdt binding which only covered
enough to provide a watchdog, but the HW block is actually mostly
about power domains.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
.../bindings/soc/bcm/brcm,bcm2835-pm.txt | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
new file mode 100644
index 000000000000..7818d33a158f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
@@ -0,0 +1,42 @@
+BCM2835 PM (Power domains, watchdog)
+
+The PM block controls power domains and some reset lines, and includes
+a watchdog timer. This binding supersedes the brcm,bcm2835-pm-wdt
+binding which covered some of PM's register range and functionality.
+
+Required properties:
+
+- compatible: Should be "brcm,bcm2835-pm"
+- reg: Specifies base physical address and size of the two
+ register ranges ("PM" and "ASYNC_BRIDGE" in that
+ order)
+- clocks: a) v3d: The V3D clock from CPRMAN
+ b) peri_image: The PERI_IMAGE clock from CPRMAN
+ c) h264: The H264 clock from CPRMAN
+ d) isp: The ISP clock from CPRMAN
+- #reset-cells: Should be 1. This property follows the reset controller
+ bindings[1].
+- #power-domain-cells: Should be 1. This property follows the power domain
+ bindings[2].
+
+Optional properties:
+
+- timeout-sec: Contains the watchdog timeout in seconds
+
+[1] Documentation/devicetree/bindings/reset/reset.txt
+[2] Documentation/devicetree/bindings/power/power_domain.txt
+
+Example:
+
+pm {
+ compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+ #power-domain-cells = <1>;
+ #reset-cells = <1>;
+ reg = <0x7e100000 0x114>,
+ <0x7e00a000 0x24>;
+ clocks = <&clocks BCM2835_CLOCK_V3D>,
+ <&clocks BCM2835_CLOCK_PERI_IMAGE>,
+ <&clocks BCM2835_CLOCK_H264>,
+ <&clocks BCM2835_CLOCK_ISP>;
+ clock-names = "v3d", "peri_image", "h264", "isp";
+};
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/8] soc: bcm: bcm2835-pm: Add support for power domains under a new binding.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (4 preceding siblings ...)
2018-11-20 17:19 ` [PATCH 5/8] dt-bindings: soc: Add a new binding for the BCM2835 PM node Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 7/8] ARM: bcm283x: Extend the WDT DT node out to cover the whole PM block Eric Anholt
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
This provides a free software alternative to raspberrypi-power.c's
firmware calls to manage power domains. It also exposes a reset line,
where previously the vc4 driver had to try to force power off the
domain in order to trigger a reset.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
arch/arm/mach-bcm/Kconfig | 1 +
drivers/soc/bcm/bcm2835-pm.c | 609 +++++++++++++++++++++++++++
include/dt-bindings/soc/bcm2835-pm.h | 28 ++
3 files changed, 638 insertions(+)
create mode 100644 include/dt-bindings/soc/bcm2835-pm.h
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 25aac6ee2ab1..d1ff5a9e43ad 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -167,6 +167,7 @@ config ARCH_BCM2835
select BCM2835_TIMER
select PINCTRL
select PINCTRL_BCM2835
+ select RESET_CONTROLLER
help
This enables support for the Broadcom BCM2835 and BCM2836 SoCs.
This SoC is used in the Raspberry Pi and Roku 2 devices.
diff --git a/drivers/soc/bcm/bcm2835-pm.c b/drivers/soc/bcm/bcm2835-pm.c
index e504381556cf..6e52f0f698ae 100644
--- a/drivers/soc/bcm/bcm2835-pm.c
+++ b/drivers/soc/bcm/bcm2835-pm.c
@@ -6,22 +6,104 @@
* branch "rpi-3.6.y" of git://github.com/raspberrypi/linux.git was used
* as a hardware reference for the Broadcom BCM2835 watchdog timer.
*
+ * Copyright (C) 2018 Broadcom
* Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
*
*/
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/io.h>
#include <linux/watchdog.h>
#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
#include <linux/of_address.h>
#include <linux/of_platform.h>
+#include <linux/pm_domain.h>
+#include <dt-bindings/soc/bcm2835-pm.h>
+#define PM_GNRIC 0x00
+#define PM_AUDIO 0x04
+#define PM_STATUS 0x18
#define PM_RSTC 0x1c
#define PM_RSTS 0x20
#define PM_WDOG 0x24
+#define PM_PADS0 0x28
+#define PM_PADS2 0x2c
+#define PM_PADS3 0x30
+#define PM_PADS4 0x34
+#define PM_PADS5 0x38
+#define PM_PADS6 0x3c
+#define PM_CAM0 0x44
+#define PM_CAM0_LDOHPEN BIT(2)
+#define PM_CAM0_LDOLPEN BIT(1)
+#define PM_CAM0_CTRLEN BIT(0)
+
+#define PM_CAM1 0x48
+#define PM_CAM1_LDOHPEN BIT(2)
+#define PM_CAM1_LDOLPEN BIT(1)
+#define PM_CAM1_CTRLEN BIT(0)
+
+#define PM_CCP2TX 0x4c
+#define PM_CCP2TX_LDOEN BIT(1)
+#define PM_CCP2TX_CTRLEN BIT(0)
+
+#define PM_DSI0 0x50
+#define PM_DSI0_LDOHPEN BIT(2)
+#define PM_DSI0_LDOLPEN BIT(1)
+#define PM_DSI0_CTRLEN BIT(0)
+
+#define PM_DSI1 0x54
+#define PM_DSI1_LDOHPEN BIT(2)
+#define PM_DSI1_LDOLPEN BIT(1)
+#define PM_DSI1_CTRLEN BIT(0)
+
+#define PM_HDMI 0x58
+#define PM_HDMI_RSTDR BIT(19)
+#define PM_HDMI_LDOPD BIT(1)
+#define PM_HDMI_CTRLEN BIT(0)
+
+#define PM_USB 0x5c
+/* The power gates must be enabled with this bit before enabling the LDO in the
+ * USB block.
+ */
+#define PM_USB_CTRLEN BIT(0)
+
+#define PM_PXLDO 0x60
+#define PM_PXBG 0x64
+#define PM_DFT 0x68
+#define PM_SMPS 0x6c
+#define PM_XOSC 0x70
+#define PM_SPAREW 0x74
+#define PM_SPARER 0x78
+#define PM_AVS_RSTDR 0x7c
+#define PM_AVS_STAT 0x80
+#define PM_AVS_EVENT 0x84
+#define PM_AVS_INTEN 0x88
+#define PM_DUMMY 0xfc
+
+#define PM_IMAGE 0x108
+#define PM_GRAFX 0x10c
+#define PM_PROC 0x110
+#define PM_ENAB BIT(12)
+#define PM_ISPRSTN BIT(8)
+#define PM_H264RSTN BIT(7)
+#define PM_PERIRSTN BIT(6)
+#define PM_V3DRSTN BIT(6)
+#define PM_ISFUNC BIT(5)
+#define PM_MRDONE BIT(4)
+#define PM_MEMREP BIT(3)
+#define PM_ISPOW BIT(2)
+#define PM_POWOK BIT(1)
+#define PM_POWUP BIT(0)
+#define PM_INRUSH_SHIFT 13
+#define PM_INRUSH_3_5_MA 0
+#define PM_INRUSH_5_MA 1
+#define PM_INRUSH_10_MA 2
+#define PM_INRUSH_20_MA 3
+#define PM_INRUSH_MASK (3 << PM_INRUSH_SHIFT)
#define PM_PASSWORD 0x5a000000
@@ -35,6 +117,26 @@
#define PM_READ(reg) readl(pm->base + (reg))
#define PM_WRITE(reg, val) writel(PM_PASSWORD | (val), pm->base + (reg))
+#define ASB_BRDG_VERSION 0x00
+#define ASB_CPR_CTRL 0x04
+
+#define ASB_V3D_S_CTRL 0x08
+#define ASB_V3D_M_CTRL 0x0c
+#define ASB_ISP_S_CTRL 0x10
+#define ASB_ISP_M_CTRL 0x14
+#define ASB_H264_S_CTRL 0x18
+#define ASB_H264_M_CTRL 0x1c
+
+#define ASB_REQ_STOP BIT(0)
+#define ASB_ACK BIT(1)
+#define ASB_EMPTY BIT(2)
+#define ASB_FULL BIT(3)
+
+#define ASB_AXI_BRDG_ID 0x20
+
+#define ASB_READ(reg) readl(pm->asb + (reg))
+#define ASB_WRITE(reg, val) writel(PM_PASSWORD | (val), pm->asb + (reg))
+
/*
* The Raspberry Pi firmware uses the RSTS register to know which partition
* to boot from. The partition value is spread into bits 0, 2, 4, 6, 8, 10.
@@ -45,9 +147,24 @@
#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
+struct bcm2835_power_domain {
+ struct generic_pm_domain base;
+ struct bcm2835_pm *pm;
+ u32 domain;
+ struct clk *clk;
+};
+
struct bcm2835_pm {
+ struct device *dev;
+ /* PM registers. */
void __iomem *base;
+ /* AXI Async bridge registers. */
+ void __iomem *asb;
spinlock_t lock;
+
+ struct genpd_onecell_data pd_xlate;
+ struct bcm2835_power_domain domains[BCM2835_POWER_DOMAIN_COUNT];
+ struct reset_controller_dev reset;
};
static unsigned int heartbeat;
@@ -142,6 +259,474 @@ static struct watchdog_device bcm2835_wdt_wdd = {
.timeout = WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
};
+static int bcm2835_asb_enable(struct bcm2835_pm *pm, u32 reg)
+{
+ u64 start = ktime_get_ns();
+
+ /* Enable the module's async AXI bridges. */
+ ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP);
+ while (ASB_READ(reg) & ASB_ACK) {
+ cpu_relax();
+ if (ktime_get_ns() - start >= 1000)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int bcm2835_asb_disable(struct bcm2835_pm *pm, u32 reg)
+{
+ u64 start = ktime_get_ns();
+
+ /* Enable the module's async AXI bridges. */
+ ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP);
+ while (!(ASB_READ(reg) & ASB_ACK)) {
+ cpu_relax();
+ if (ktime_get_ns() - start >= 1000)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int bcm2835_pm_power_off(struct bcm2835_power_domain *pd, u32 pm_reg)
+{
+ struct bcm2835_pm *pm = pd->pm;
+
+ /* Enable functional isolation */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~PM_ISFUNC);
+
+ /* Enable electrical isolation */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~PM_ISPOW);
+
+ /* Open the power switches. */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~PM_POWUP);
+
+ return 0;
+}
+
+static int bcm2835_pm_power_on(struct bcm2835_power_domain *pd, u32 pm_reg)
+{
+ struct bcm2835_pm *pm = pd->pm;
+ struct device *dev = pm->dev;
+ u64 start;
+ int ret;
+ int inrush;
+ bool powok;
+
+ /* If it was already powered on by the fw, leave it that way. */
+ if (PM_READ(pm_reg) & PM_POWUP)
+ return 0;
+
+ /* Enable power. Allowing too much current at once may result
+ * in POWOK never getting set, so start low and ramp it up as
+ * necessary to succeed.
+ */
+ powok = false;
+ for (inrush = PM_INRUSH_3_5_MA; inrush <= PM_INRUSH_20_MA; inrush++) {
+ PM_WRITE(pm_reg,
+ (PM_READ(pm_reg) & ~PM_INRUSH_MASK) |
+ (inrush << PM_INRUSH_SHIFT) |
+ PM_POWUP);
+
+ start = ktime_get_ns();
+ while (!(powok = !!(PM_READ(pm_reg) & PM_POWOK))) {
+ cpu_relax();
+ if (ktime_get_ns() - start >= 3000)
+ break;
+ }
+ }
+ if (!powok) {
+ dev_err(dev, "Timeout waiting for %s power OK\n",
+ pd->base.name);
+ ret = -ETIMEDOUT;
+ goto err_disable_powup;
+ }
+
+ /* Disable electrical isolation */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) | PM_ISPOW);
+
+ /* Repair memory */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) | PM_MEMREP);
+ start = ktime_get_ns();
+ while (!(PM_READ(pm_reg) & PM_MRDONE)) {
+ cpu_relax();
+ if (ktime_get_ns() - start >= 1000) {
+ dev_err(dev, "Timeout waiting for %s memory repair\n",
+ pd->base.name);
+ ret = -ETIMEDOUT;
+ goto err_disable_ispow;
+ }
+ }
+
+ /* Disable functional isolation */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) | PM_ISFUNC);
+
+ return 0;
+
+err_disable_ispow:
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~PM_ISPOW);
+err_disable_powup:
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~(PM_POWUP | PM_INRUSH_MASK));
+ return ret;
+}
+
+static int bcm2835_asb_power_on(struct bcm2835_power_domain *pd,
+ u32 pm_reg,
+ u32 asb_m_reg,
+ u32 asb_s_reg,
+ u32 reset_flags)
+{
+ struct bcm2835_pm *pm = pd->pm;
+ int ret;
+
+ ret = clk_prepare_enable(pd->clk);
+ if (ret) {
+ dev_err(pm->dev, "Failed to enable clock for %s\n",
+ pd->base.name);
+ return ret;
+ }
+
+ /* Wait 32 clocks for reset to propagate, 1 us will be enough */
+ udelay(1);
+
+ clk_disable_unprepare(pd->clk);
+
+ /* Deassert the resets. */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) | reset_flags);
+
+ ret = clk_prepare_enable(pd->clk);
+ if (ret) {
+ dev_err(pm->dev, "Failed to enable clock for %s\n",
+ pd->base.name);
+ goto err_enable_resets;
+ }
+
+ ret = bcm2835_asb_enable(pm, asb_m_reg);
+ if (ret) {
+ dev_err(pm->dev, "Failed to enable ASB master for %s\n",
+ pd->base.name);
+ goto err_disable_clk;
+ }
+ ret = bcm2835_asb_enable(pm, asb_s_reg);
+ if (ret) {
+ dev_err(pm->dev, "Failed to enable ASB slave for %s\n",
+ pd->base.name);
+ goto err_disable_asb_master;
+ }
+
+ return 0;
+
+err_disable_asb_master:
+ bcm2835_asb_disable(pm, asb_m_reg);
+err_disable_clk:
+ clk_disable_unprepare(pd->clk);
+err_enable_resets:
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~reset_flags);
+ return ret;
+}
+
+static int bcm2835_asb_power_off(struct bcm2835_power_domain *pd,
+ u32 pm_reg,
+ u32 asb_m_reg,
+ u32 asb_s_reg,
+ u32 reset_flags)
+{
+ struct bcm2835_pm *pm = pd->pm;
+ int ret;
+
+ ret = bcm2835_asb_disable(pm, asb_s_reg);
+ if (ret) {
+ dev_warn(pm->dev, "Failed to disable ASB slave for %s\n",
+ pd->base.name);
+ return ret;
+ }
+ ret = bcm2835_asb_disable(pm, asb_m_reg);
+ if (ret) {
+ dev_warn(pm->dev, "Failed to disable ASB master for %s\n",
+ pd->base.name);
+ bcm2835_asb_enable(pm, asb_s_reg);
+ return ret;
+ }
+
+ clk_disable_unprepare(pd->clk);
+
+ /* Assert the resets. */
+ PM_WRITE(pm_reg, PM_READ(pm_reg) & ~reset_flags);
+
+ return 0;
+}
+
+static int bcm2835_pm_pd_power_on(struct generic_pm_domain *domain)
+{
+ struct bcm2835_power_domain *pd =
+ container_of(domain, struct bcm2835_power_domain, base);
+ struct bcm2835_pm *pm = pd->pm;
+
+ switch (pd->domain) {
+ case BCM2835_POWER_DOMAIN_GRAFX:
+ return bcm2835_pm_power_on(pd, PM_GRAFX);
+
+ case BCM2835_POWER_DOMAIN_GRAFX_V3D:
+ return bcm2835_asb_power_on(pd, PM_GRAFX,
+ ASB_V3D_M_CTRL, ASB_V3D_S_CTRL,
+ PM_V3DRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE:
+ return bcm2835_pm_power_on(pd, PM_IMAGE);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_PERI:
+ return bcm2835_asb_power_on(pd, PM_IMAGE,
+ 0, 0,
+ PM_PERIRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_ISP:
+ return bcm2835_asb_power_on(pd, PM_IMAGE,
+ ASB_ISP_M_CTRL, ASB_ISP_S_CTRL,
+ PM_ISPRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_H264:
+ return bcm2835_asb_power_on(pd, PM_IMAGE,
+ ASB_H264_M_CTRL, ASB_H264_S_CTRL,
+ PM_H264RSTN);
+
+ case BCM2835_POWER_DOMAIN_USB:
+ PM_WRITE(PM_USB, PM_USB_CTRLEN);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_DSI0:
+ PM_WRITE(PM_DSI0, PM_DSI0_CTRLEN);
+ PM_WRITE(PM_DSI0, PM_DSI0_CTRLEN | PM_DSI0_LDOHPEN);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_DSI1:
+ PM_WRITE(PM_DSI1, PM_DSI1_CTRLEN);
+ PM_WRITE(PM_DSI1, PM_DSI1_CTRLEN | PM_DSI1_LDOHPEN);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_CCP2TX:
+ PM_WRITE(PM_CCP2TX, PM_CCP2TX_CTRLEN);
+ PM_WRITE(PM_CCP2TX, PM_CCP2TX_CTRLEN | PM_CCP2TX_LDOEN);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_HDMI:
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) | PM_HDMI_RSTDR);
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) | PM_HDMI_CTRLEN);
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) & ~PM_HDMI_LDOPD);
+ usleep_range(100, 200);
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) & ~PM_HDMI_RSTDR);
+ return 0;
+
+ default:
+ dev_err(pm->dev, "Invalid domain %d\n", pd->domain);
+ return -EINVAL;
+ }
+}
+
+static int bcm2835_pm_pd_power_off(struct generic_pm_domain *domain)
+{
+ struct bcm2835_power_domain *pd =
+ container_of(domain, struct bcm2835_power_domain, base);
+ struct bcm2835_pm *pm = pd->pm;
+
+ switch (pd->domain) {
+ case BCM2835_POWER_DOMAIN_GRAFX:
+ return bcm2835_pm_power_off(pd, PM_GRAFX);
+
+ case BCM2835_POWER_DOMAIN_GRAFX_V3D:
+ return bcm2835_asb_power_off(pd, PM_GRAFX,
+ ASB_V3D_M_CTRL, ASB_V3D_S_CTRL,
+ PM_V3DRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE:
+ return bcm2835_pm_power_off(pd, PM_IMAGE);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_PERI:
+ return bcm2835_asb_power_off(pd, PM_IMAGE,
+ 0, 0,
+ PM_PERIRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_ISP:
+ return bcm2835_asb_power_off(pd, PM_IMAGE,
+ ASB_ISP_M_CTRL, ASB_ISP_S_CTRL,
+ PM_ISPRSTN);
+
+ case BCM2835_POWER_DOMAIN_IMAGE_H264:
+ return bcm2835_asb_power_off(pd, PM_IMAGE,
+ ASB_H264_M_CTRL, ASB_H264_S_CTRL,
+ PM_H264RSTN);
+
+ case BCM2835_POWER_DOMAIN_USB:
+ PM_WRITE(PM_USB, 0);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_DSI0:
+ PM_WRITE(PM_DSI0, PM_DSI0_CTRLEN);
+ PM_WRITE(PM_DSI0, 0);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_DSI1:
+ PM_WRITE(PM_DSI1, PM_DSI1_CTRLEN);
+ PM_WRITE(PM_DSI1, 0);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_CCP2TX:
+ PM_WRITE(PM_CCP2TX, PM_CCP2TX_CTRLEN);
+ PM_WRITE(PM_CCP2TX, 0);
+ return 0;
+
+ case BCM2835_POWER_DOMAIN_HDMI:
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) | PM_HDMI_LDOPD);
+ PM_WRITE(PM_HDMI, PM_READ(PM_HDMI) & ~PM_HDMI_CTRLEN);
+ return 0;
+
+ default:
+ dev_err(pm->dev, "Invalid domain %d\n", pd->domain);
+ return -EINVAL;
+ }
+}
+
+static void
+bcm2835_init_power_domain(struct bcm2835_pm *pm,
+ int pd_xlate_index, const char *name)
+{
+ struct device *dev = pm->dev;
+ struct bcm2835_power_domain *dom = &pm->domains[pd_xlate_index];
+
+ dom->clk = devm_clk_get(dev, name);
+
+ dom->base.name = name;
+ dom->base.power_on = bcm2835_pm_pd_power_on;
+ dom->base.power_off = bcm2835_pm_pd_power_off;
+
+ dom->domain = pd_xlate_index;
+ dom->pm = pm;
+
+ /* XXX: on/off at boot? */
+ pm_genpd_init(&dom->base, NULL, true);
+
+ pm->pd_xlate.domains[pd_xlate_index] = &dom->base;
+}
+
+/** bcm2835_reset_reset - Resets a block that has a reset line in the
+ * PM block.
+ *
+ * The consumer of the reset controller must have the power domain up
+ * -- there's no reset ability with the power domain down. To reset
+ * the sub-block, we just disable its access to memory through the
+ * ASB, reset, and re-enable.
+ */
+static int bcm2835_reset_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct bcm2835_pm *pm = container_of(rcdev, struct bcm2835_pm, reset);
+ struct bcm2835_power_domain *pd;
+ int ret;
+
+ switch (id) {
+ case BCM2835_RESET_V3D:
+ pd = &pm->domains[BCM2835_POWER_DOMAIN_GRAFX_V3D];
+ break;
+ case BCM2835_RESET_H264:
+ pd = &pm->domains[BCM2835_POWER_DOMAIN_IMAGE_H264];
+ break;
+ case BCM2835_RESET_ISP:
+ pd = &pm->domains[BCM2835_POWER_DOMAIN_IMAGE_ISP];
+ break;
+ default:
+ dev_err(pm->dev, "Bad reset id %ld\n", id);
+ return -EINVAL;
+ }
+
+ ret = bcm2835_pm_pd_power_off(&pd->base);
+ if (ret)
+ return ret;
+
+ return bcm2835_pm_pd_power_on(&pd->base);
+}
+
+static int bcm2835_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct bcm2835_pm *pm = container_of(rcdev, struct bcm2835_pm, reset);
+
+ switch (id) {
+ case BCM2835_RESET_V3D:
+ return !PM_READ(PM_GRAFX & PM_V3DRSTN);
+ case BCM2835_RESET_H264:
+ return !PM_READ(PM_IMAGE & PM_H264RSTN);
+ case BCM2835_RESET_ISP:
+ return !PM_READ(PM_IMAGE & PM_ISPRSTN);
+ default:
+ return -EINVAL;
+ }
+}
+
+const struct reset_control_ops bcm2835_reset_ops = {
+ .reset = bcm2835_reset_reset,
+ .status = bcm2835_reset_status,
+};
+
+static int bcm2835_init_power_domains(struct bcm2835_pm *pm)
+{
+ struct device *dev = pm->dev;
+ static const struct {
+ int parent, child;
+ } domain_deps[] = {
+ { BCM2835_POWER_DOMAIN_GRAFX, BCM2835_POWER_DOMAIN_GRAFX_V3D },
+ { BCM2835_POWER_DOMAIN_IMAGE, BCM2835_POWER_DOMAIN_IMAGE_PERI },
+ { BCM2835_POWER_DOMAIN_IMAGE, BCM2835_POWER_DOMAIN_IMAGE_H264 },
+ { BCM2835_POWER_DOMAIN_IMAGE, BCM2835_POWER_DOMAIN_IMAGE_ISP },
+ { BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_USB },
+ { BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM0 },
+ { BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM1 },
+ };
+ int ret, i;
+
+ pm->pd_xlate.domains = devm_kcalloc(dev,
+ BCM2835_POWER_DOMAIN_COUNT,
+ sizeof(*pm->pd_xlate.domains),
+ GFP_KERNEL);
+ if (!pm->pd_xlate.domains)
+ return -ENOMEM;
+
+ pm->pd_xlate.num_domains = BCM2835_POWER_DOMAIN_COUNT;
+
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_GRAFX, "grafx");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_GRAFX_V3D, "v3d");
+
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_IMAGE, "image");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_IMAGE_PERI, "peri_image");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_IMAGE_H264, "h264");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_IMAGE_ISP, "isp");
+
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_USB, "usb");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_DSI0, "dsi0");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_DSI1, "dsi1");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_CAM0, "cam0");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_CAM1, "cam1");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_CCP2TX, "ccp2tx");
+ bcm2835_init_power_domain(pm, BCM2835_POWER_DOMAIN_HDMI, "hdmi");
+
+ for (i = 0; i < ARRAY_SIZE(domain_deps); i++) {
+ pm_genpd_add_subdomain(&pm->domains[domain_deps[i].parent].base,
+ &pm->domains[domain_deps[i].child].base);
+ }
+
+ pm->reset.owner = THIS_MODULE;
+ pm->reset.nr_resets = BCM2835_RESET_COUNT;
+ pm->reset.ops = &bcm2835_reset_ops;
+ pm->reset.of_node = dev->of_node;
+
+ ret = devm_reset_controller_register(dev, &pm->reset);
+ if (ret)
+ return ret;
+
+ of_genpd_add_provider_onecell(dev->of_node, &pm->pd_xlate);
+
+ return 0;
+}
+
/*
* We can't really power off, but if we do the normal reset scheme, and
* indicate to bootcode.bin not to reboot, then most of the chip will be
@@ -178,6 +763,7 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
pm = devm_kzalloc(dev, sizeof(struct bcm2835_pm), GFP_KERNEL);
if (!pm)
return -ENOMEM;
+ pm->dev = dev;
platform_set_drvdata(pdev, pm);
spin_lock_init(&pm->lock);
@@ -187,6 +773,15 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
if (IS_ERR(pm->base))
return PTR_ERR(pm->base);
+ /* We'll use the presence of the AXI ASB regs in the
+ * bcm2835-pm binding as the key for whether we can reference
+ * the full PM register range and support power domains.
+ */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ pm->asb = devm_ioremap_resource(dev, res);
+ if (IS_ERR(pm->asb))
+ pm->asb = NULL;
+
#if defined(CONFIG_WATCHDOG_CORE)
watchdog_set_drvdata(&bcm2835_wdt_wdd, pm);
watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
@@ -214,6 +809,19 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
}
#endif
+ if (pm->asb) {
+ u32 id = ASB_READ(ASB_AXI_BRDG_ID);
+
+ if (id != 0x62726467 /* "BRDG" */) {
+ dev_err(dev, "ASB register ID returned 0x%08x\n", id);
+ return -ENODEV;
+ }
+
+ err = bcm2835_init_power_domains(pm);
+ if (err)
+ return err;
+ }
+
if (pm_power_off == NULL)
pm_power_off = bcm2835_power_off;
@@ -231,6 +839,7 @@ static int bcm2835_pm_remove(struct platform_device *pdev)
static const struct of_device_id bcm2835_pm_of_match[] = {
{ .compatible = "brcm,bcm2835-pm-wdt", },
+ { .compatible = "brcm,bcm2835-pm", },
{},
};
MODULE_DEVICE_TABLE(of, bcm2835_pm_of_match);
diff --git a/include/dt-bindings/soc/bcm2835-pm.h b/include/dt-bindings/soc/bcm2835-pm.h
new file mode 100644
index 000000000000..153d75b8d99f
--- /dev/null
+++ b/include/dt-bindings/soc/bcm2835-pm.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+
+#ifndef _DT_BINDINGS_ARM_BCM2835_PM_H
+#define _DT_BINDINGS_ARM_BCM2835_PM_H
+
+#define BCM2835_POWER_DOMAIN_GRAFX 0
+#define BCM2835_POWER_DOMAIN_GRAFX_V3D 1
+#define BCM2835_POWER_DOMAIN_IMAGE 2
+#define BCM2835_POWER_DOMAIN_IMAGE_PERI 3
+#define BCM2835_POWER_DOMAIN_IMAGE_ISP 4
+#define BCM2835_POWER_DOMAIN_IMAGE_H264 5
+#define BCM2835_POWER_DOMAIN_USB 6
+#define BCM2835_POWER_DOMAIN_DSI0 7
+#define BCM2835_POWER_DOMAIN_DSI1 8
+#define BCM2835_POWER_DOMAIN_CAM0 9
+#define BCM2835_POWER_DOMAIN_CAM1 10
+#define BCM2835_POWER_DOMAIN_CCP2TX 11
+#define BCM2835_POWER_DOMAIN_HDMI 12
+
+#define BCM2835_POWER_DOMAIN_COUNT 13
+
+#define BCM2835_RESET_V3D 0
+#define BCM2835_RESET_ISP 1
+#define BCM2835_RESET_H264 2
+
+#define BCM2835_RESET_COUNT 3
+
+#endif /* _DT_BINDINGS_ARM_BCM2835_PM_H */
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/8] ARM: bcm283x: Extend the WDT DT node out to cover the whole PM block.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (5 preceding siblings ...)
2018-11-20 17:19 ` [PATCH 6/8] soc: bcm: bcm2835-pm: Add support for power domains under a new binding Eric Anholt
@ 2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:20 ` [PATCH 8/8] ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware Eric Anholt
2018-11-20 17:49 ` [PATCH 0/8] BCM2835 PM driver Stefan Wahren
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:19 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
It was covering part of the PM block's range, up to the WDT regs. To
support the rest of the PM block's functionality, we need the full
register range plus the AXI Async Bridge regs for PM sequencing.
This doesn't convert any of the consumers over to the new binding yet,
since we will need to be careful in coordinating our usage of firmware
services that might power domains on and off versus the bcm2835-pm
driver's access of those same domains.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
arch/arm/boot/dts/bcm283x.dtsi | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index 31b29646b14c..bd5be68b4561 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -121,8 +121,16 @@
};
watchdog@7e100000 {
- compatible = "brcm,bcm2835-pm-wdt";
- reg = <0x7e100000 0x28>;
+ compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+ #power-domain-cells = <1>;
+ #reset-cells = <1>;
+ reg = <0x7e100000 0x114>,
+ <0x7e00a000 0x24>;
+ clocks = <&clocks BCM2835_CLOCK_V3D>,
+ <&clocks BCM2835_CLOCK_PERI_IMAGE>,
+ <&clocks BCM2835_CLOCK_H264>,
+ <&clocks BCM2835_CLOCK_ISP>;
+ clock-names = "v3d", "peri_image", "h264", "isp";
};
clocks: cprman@7e101000 {
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/8] ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware.
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (6 preceding siblings ...)
2018-11-20 17:19 ` [PATCH 7/8] ARM: bcm283x: Extend the WDT DT node out to cover the whole PM block Eric Anholt
@ 2018-11-20 17:20 ` Eric Anholt
2018-11-20 17:49 ` [PATCH 0/8] BCM2835 PM driver Stefan Wahren
8 siblings, 0 replies; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 17:20 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel, Stefan Wahren,
bcm-kernel-feedback-list, Eric Anholt
The GRAFX domain only contains V3D, and this driver should be the only
accessor of V3D (firmware usage gets disabled when V3D is in the DT),
so we can safely make Linux control the GRAFX and GRAFX_V3D power
domains.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ----
arch/arm/boot/dts/bcm283x.dtsi | 4 +++-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index cb2d6d78a7fb..21a930148709 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -87,10 +87,6 @@
power-domains = <&power RPI_POWER_DOMAIN_USB>;
};
-&v3d {
- power-domains = <&power RPI_POWER_DOMAIN_V3D>;
-};
-
&hdmi {
power-domains = <&power RPI_POWER_DOMAIN_HDMI>;
status = "okay";
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index bd5be68b4561..ce7bc9cc43ae 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -3,6 +3,7 @@
#include <dt-bindings/clock/bcm2835-aux.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/soc/bcm2835-pm.h>
/* firmware-provided startup stubs live here, where the secondary CPUs are
* spinning.
@@ -120,7 +121,7 @@
#interrupt-cells = <2>;
};
- watchdog@7e100000 {
+ pm: watchdog@7e100000 {
compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
#power-domain-cells = <1>;
#reset-cells = <1>;
@@ -637,6 +638,7 @@
compatible = "brcm,bcm2835-v3d";
reg = <0x7ec00000 0x1000>;
interrupts = <1 10>;
+ power-domains = <&pm BCM2835_POWER_DOMAIN_GRAFX_V3D>;
};
vc4: gpu {
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 0/8] BCM2835 PM driver
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
` (7 preceding siblings ...)
2018-11-20 17:20 ` [PATCH 8/8] ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware Eric Anholt
@ 2018-11-20 17:49 ` Stefan Wahren
2018-11-20 21:34 ` Eric Anholt
8 siblings, 1 reply; 13+ messages in thread
From: Stefan Wahren @ 2018-11-20 17:49 UTC (permalink / raw)
To: Eric Anholt, Florian Fainelli, Rob Herring, Mark Rutland,
Wim Van Sebroeck, Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
Hi Eric,
> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben:
>
>
> This series moves the BCM2835 WDT driver that controls a fraction of
> the PM block out to soc/ and adds most of the rest of its
> functionality. My motivation has been to have V3D be functional
> without firmware calls, probably improve its interactivity (since
> we'll be able to power on/off without RPC to the firmware that may be
> busy with other tasks), and (in a patch not submitted in this series)
> extend its binding to use the reset controller instead of trying to
> reset by toggling its power domain.
>
> I've tested V3D with a few hours of running a V3D test, sleep(1) (to
> trigger PM domain off); running a GPU hang job (to trigger reset);
> sleep(1). The non-hanging success-case job always passed, and dmesg
> had no complaints from bcm2835-pm. The other power domains are not
> tested, but I've done my best.
>
> This series will probably also be of interest to the
> https://github.com/christinaa/rpi-open-firmware project for enabling USB.
>
apologize to give you my feedback after you send out the series.
I know you won't be happy about it, but i think we need a little more complex but future proof solution for this power driver. According to the register definition of the PM block, we have multiple functions here (power domains, watchdog, pads/pinctrl, ...). Since this is common for ARM SoCs there is a subsystem called mfd (multi function device) [1] to abstract all resources of the IP block.
This has the advantage that we don't need a monolithic driver which takes care of all functions.
According to this approach we would have the following drivers:
mfd/bcm2835.c
soc/bcm/bcm2835-power.c
watchdog/bcm2835_wdt.c
Best regards
Stefan
[1] - http://events17.linuxfoundation.org/sites/events/files/slides/belloni-mfd-regmap-syscon_0.pdf
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/8] BCM2835 PM driver
2018-11-20 17:49 ` [PATCH 0/8] BCM2835 PM driver Stefan Wahren
@ 2018-11-20 21:34 ` Eric Anholt
2018-11-20 21:39 ` Arnd Bergmann
0 siblings, 1 reply; 13+ messages in thread
From: Eric Anholt @ 2018-11-20 21:34 UTC (permalink / raw)
To: Stefan Wahren, Florian Fainelli, Rob Herring, Mark Rutland,
Wim Van Sebroeck, Guenter Roeck, linux-watchdog
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]
Stefan Wahren <stefan.wahren@i2se.com> writes:
> Hi Eric,
>
>> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben:
>>
>>
>> This series moves the BCM2835 WDT driver that controls a fraction of
>> the PM block out to soc/ and adds most of the rest of its
>> functionality. My motivation has been to have V3D be functional
>> without firmware calls, probably improve its interactivity (since
>> we'll be able to power on/off without RPC to the firmware that may be
>> busy with other tasks), and (in a patch not submitted in this series)
>> extend its binding to use the reset controller instead of trying to
>> reset by toggling its power domain.
>>
>> I've tested V3D with a few hours of running a V3D test, sleep(1) (to
>> trigger PM domain off); running a GPU hang job (to trigger reset);
>> sleep(1). The non-hanging success-case job always passed, and dmesg
>> had no complaints from bcm2835-pm. The other power domains are not
>> tested, but I've done my best.
>>
>> This series will probably also be of interest to the
>> https://github.com/christinaa/rpi-open-firmware project for enabling USB.
>>
>
> apologize to give you my feedback after you send out the series.
>
> I know you won't be happy about it, but i think we need a little more
> complex but future proof solution for this power driver. According to
> the register definition of the PM block, we have multiple functions
> here (power domains, watchdog, pads/pinctrl, ...). Since this is
> common for ARM SoCs there is a subsystem called mfd (multi function
> device) [1] to abstract all resources of the IP block.
>
> This has the advantage that we don't need a monolithic driver which
> takes care of all functions.
I consider your "advantage" to be a disadvantage. By forcing the split,
you end up having more driver files to manage, more platform devices,
and more error-prone code to get the resources from the parent down into
the client. It feels like writing software for the sake of writing
software, rather than solving a concrete problem.
My original series:
10 files changed, 951 insertions(+), 273 deletions(-)
The MFD series:
12 files changed, 882 insertions(+), 25 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/8] BCM2835 PM driver
2018-11-20 21:34 ` Eric Anholt
@ 2018-11-20 21:39 ` Arnd Bergmann
0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2018-11-20 21:39 UTC (permalink / raw)
To: Eric Anholt
Cc: Stefan Wahren, Florian Fainelli, Rob Herring, Mark Rutland,
Wim Van Sebroeck, Guenter Roeck, linux-watchdog,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, Linux ARM,
Linux Kernel Mailing List
On Tue, Nov 20, 2018 at 10:34 PM Eric Anholt <eric@anholt.net> wrote:
> >> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben:
> >>
> >>
> >> This series moves the BCM2835 WDT driver that controls a fraction of
> >> the PM block out to soc/ and adds most of the rest of its
> >> functionality. My motivation has been to have V3D be functional
> >> without firmware calls, probably improve its interactivity (since
> >> we'll be able to power on/off without RPC to the firmware that may be
> >> busy with other tasks), and (in a patch not submitted in this series)
> >> extend its binding to use the reset controller instead of trying to
> >> reset by toggling its power domain.
> >>
> >> I've tested V3D with a few hours of running a V3D test, sleep(1) (to
> >> trigger PM domain off); running a GPU hang job (to trigger reset);
> >> sleep(1). The non-hanging success-case job always passed, and dmesg
> >> had no complaints from bcm2835-pm. The other power domains are not
> >> tested, but I've done my best.
> >>
> >> This series will probably also be of interest to the
> >> https://github.com/christinaa/rpi-open-firmware project for enabling USB.
> >>
> >
> > apologize to give you my feedback after you send out the series.
> >
> > I know you won't be happy about it, but i think we need a little more
> > complex but future proof solution for this power driver. According to
> > the register definition of the PM block, we have multiple functions
> > here (power domains, watchdog, pads/pinctrl, ...). Since this is
> > common for ARM SoCs there is a subsystem called mfd (multi function
> > device) [1] to abstract all resources of the IP block.
> >
> > This has the advantage that we don't need a monolithic driver which
> > takes care of all functions.
>
> I consider your "advantage" to be a disadvantage. By forcing the split,
> you end up having more driver files to manage, more platform devices,
> and more error-prone code to get the resources from the parent down into
> the client. It feels like writing software for the sake of writing
> software, rather than solving a concrete problem.
>
> My original series:
>
> 10 files changed, 951 insertions(+), 273 deletions(-)
>
> The MFD series:
>
> 12 files changed, 882 insertions(+), 25 deletions(-)
My preference in general is having less code in drivers/soc
and more in subsystems of people that understand their stuff.
Not every driver leans itself to being an MFD, but the more differnent
functions it has, the better it is to have it split up. This was the
original motivation of moving irqchip, clk, pinctrl, etc drivers out
of arch/arm/mach-* into their own subsystems.
Arnd
^ permalink raw reply [flat|nested] 13+ messages in thread