Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback
@ 2019-09-27 10:31 Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 1/4] dt-bindings: mediatek: mt8183: Add #reset-cells Jiaxin Yu
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Jiaxin Yu @ 2019-09-27 10:31 UTC (permalink / raw)
  To: broonie, mark.rutland, robh+dt, linux, wim
  Cc: alsa-devel, yong.liang, lgirdwood, jiaxin.yu, tzungbi,
	linux-mediatek, eason.yen, linux-arm-kernel

This series patches add reset controller for MT8183, and audio will use it in 
machine driver during bootup, they depend on the following patch:

1. this series add support reset-controller in infra
	[v5,2/2] clk: reset: Modify reset-controller driver
	https://patchwork.kernel.org/patch/11060419/

v2 changes:
	1. remove "WIP" that in the title of patches
	2. add hyper link for the patch that depends on

Jiaxin Yu (2):
  dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"
  ASoC: mt8183: fix audio playback slowly after playback during bootup

yong.liang (2):
  dt-bindings: mediatek: mt8183: Add #reset-cells
  watchdog: mtk_wdt: mt8183: Add reset controller

 .../bindings/sound/mt8183-afe-pcm.txt         |   6 +
 .../devicetree/bindings/watchdog/mtk-wdt.txt  |   9 +-
 drivers/watchdog/Kconfig                      |   1 +
 drivers/watchdog/mtk_wdt.c                    | 110 +++++++++++++++++-
 .../reset-controller/mt8183-resets.h          |  13 +++
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c    |  15 +++
 6 files changed, 150 insertions(+), 4 deletions(-)

-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] [PATCH v2 1/4] dt-bindings: mediatek: mt8183: Add #reset-cells
  2019-09-27 10:31 [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback Jiaxin Yu
@ 2019-09-27 10:31 ` Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Jiaxin Yu @ 2019-09-27 10:31 UTC (permalink / raw)
  To: broonie, mark.rutland, robh+dt, linux, wim
  Cc: alsa-devel, yong.liang, yong.liang, lgirdwood, jiaxin.yu,
	tzungbi, linux-mediatek, eason.yen, linux-arm-kernel

From: "yong.liang" <yong.liang@mediatek.corp-partner.google.com>

Add #reset-cells property and update example

Signed-off-by: yong.liang <yong.liang@mediatek.corp-partner.google.com>
---
 .../devicetree/bindings/watchdog/mtk-wdt.txt        |  9 ++++++---
 .../dt-bindings/reset-controller/mt8183-resets.h    | 13 +++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
index 3ee625d0812f..ecb9ff784832 100644
--- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
@@ -16,11 +16,14 @@ Required properties:
 
 Optional properties:
 - timeout-sec: contains the watchdog timeout in seconds.
+- #reset-cells: Should be 1.
 
 Example:
 
-wdt: watchdog@10000000 {
-	compatible = "mediatek,mt6589-wdt";
-	reg = <0x10000000 0x18>;
+watchdog: watchdog@10007000 {
+	compatible = "mediatek,mt8183-wdt",
+		     "mediatek,mt6589-wdt";
+	reg = <0 0x10007000 0 0x100>;
 	timeout-sec = <10>;
+	#reset-cells = <1>;
 };
diff --git a/include/dt-bindings/reset-controller/mt8183-resets.h b/include/dt-bindings/reset-controller/mt8183-resets.h
index 8804e34ebdd4..47dadcf3fd24 100644
--- a/include/dt-bindings/reset-controller/mt8183-resets.h
+++ b/include/dt-bindings/reset-controller/mt8183-resets.h
@@ -78,4 +78,17 @@
 #define MT8183_INFRACFG_AO_I2C7_SW_RST				126
 #define MT8183_INFRACFG_AO_I2C8_SW_RST				127
 
+#define MT8183_TOPRGU_MM_SW_RST                                 1
+#define MT8183_TOPRGU_MFG_SW_RST                                2
+#define MT8183_TOPRGU_VENC_SW_RST                               3
+#define MT8183_TOPRGU_VDEC_SW_RST                               4
+#define MT8183_TOPRGU_IMG_SW_RST                                5
+#define MT8183_TOPRGU_MD_SW_RST                                 7
+#define MT8183_TOPRGU_CONN_SW_RST                               9
+#define MT8183_TOPRGU_CONN_MCU_SW_RST                           12
+#define MT8183_TOPRGU_IPU0_SW_RST                               14
+#define MT8183_TOPRGU_IPU1_SW_RST                               15
+#define MT8183_TOPRGU_AUDIO_SW_RST                              17
+#define MT8183_TOPRGU_CAMSYS_SW_RST                             18
+
 #endif  /* _DT_BINDINGS_RESET_CONTROLLER_MT8183 */
-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-09-27 10:31 [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 1/4] dt-bindings: mediatek: mt8183: Add #reset-cells Jiaxin Yu
@ 2019-09-27 10:31 ` Jiaxin Yu
  2019-09-28 17:49   ` Guenter Roeck
                     ` (2 more replies)
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 3/4] dt-bindings: medaitek: mt8183: add property "resets" && "reset-names" Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
  3 siblings, 3 replies; 16+ messages in thread
From: Jiaxin Yu @ 2019-09-27 10:31 UTC (permalink / raw)
  To: broonie, mark.rutland, robh+dt, linux, wim
  Cc: alsa-devel, yong.liang, lgirdwood, jiaxin.yu, tzungbi,
	linux-mediatek, eason.yen, linux-arm-kernel

From: "yong.liang" <yong.liang@mediatek.com>

Provide assert/deassert/reset API in watchdog driver.
Register reset controller for toprgu device in watchdog probe.

Signed-off-by: yong.liang <yong.liang@mediatek.com>
---
 drivers/watchdog/Kconfig   |   1 +
 drivers/watchdog/mtk_wdt.c | 110 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 2e07caab9db2..629249fe5305 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -717,6 +717,7 @@ config MEDIATEK_WATCHDOG
 	tristate "Mediatek SoCs watchdog support"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
 	select WATCHDOG_CORE
+	select RESET_CONTROLLER
 	help
 	  Say Y here to include support for the watchdog timer
 	  in Mediatek SoCs.
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 9c3d0033260d..660fb0e48d8e 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -20,6 +20,10 @@
 #include <linux/types.h>
 #include <linux/watchdog.h>
 #include <linux/delay.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/reset.h>
+#include <linux/of_device.h>
 
 #define WDT_MAX_TIMEOUT		31
 #define WDT_MIN_TIMEOUT		1
@@ -44,17 +48,113 @@
 #define WDT_SWRST		0x14
 #define WDT_SWRST_KEY		0x1209
 
+#define WDT_SWSYSRST		0x18U
+#define WDT_SWSYS_RST_KEY	0x88000000
+
 #define DRV_NAME		"mtk-wdt"
 #define DRV_VERSION		"1.0"
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static unsigned int timeout;
 
+struct toprgu_reset {
+	spinlock_t lock; /* Protects reset_controller access */
+	void __iomem *toprgu_swrst_base;
+	int regofs;
+	struct reset_controller_dev rcdev;
+};
+
 struct mtk_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
+	struct toprgu_reset reset_controller;
+	const struct mtk_wdt_compatible *dev_comp;
+};
+
+struct mtk_wdt_compatible {
+	int sw_rst_num;
+};
+
+static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	unsigned int tmp;
+	unsigned long flags;
+	struct toprgu_reset *data = container_of(rcdev,
+				struct toprgu_reset, rcdev);
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
+	tmp |= BIT(id);
+	tmp |= WDT_SWSYS_RST_KEY;
+	writel(tmp, data->toprgu_swrst_base + data->regofs);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	unsigned int tmp;
+	unsigned long flags;
+	struct toprgu_reset *data = container_of(rcdev,
+					struct toprgu_reset, rcdev);
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
+	tmp &= ~BIT(id);
+	tmp |= WDT_SWSYS_RST_KEY;
+	writel(tmp, data->toprgu_swrst_base + data->regofs);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static int toprgu_reset(struct reset_controller_dev *rcdev,
+			unsigned long id)
+{
+	int ret;
+
+	ret = toprgu_reset_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	return toprgu_reset_deassert(rcdev, id);
+}
+
+static struct reset_control_ops toprgu_reset_ops = {
+	.assert = toprgu_reset_assert,
+	.deassert = toprgu_reset_deassert,
+	.reset = toprgu_reset,
 };
 
+static void toprgu_register_reset_controller(struct platform_device *pdev,
+					     int regofs)
+{
+	int ret;
+	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
+
+	spin_lock_init(&mtk_wdt->reset_controller.lock);
+
+	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
+	mtk_wdt->reset_controller.toprgu_swrst_base = mtk_wdt->wdt_base;
+	mtk_wdt->reset_controller.regofs = regofs;
+	mtk_wdt->reset_controller.rcdev.owner = THIS_MODULE;
+	mtk_wdt->reset_controller.rcdev.nr_resets =
+				mtk_wdt->dev_comp->sw_rst_num;
+	mtk_wdt->reset_controller.rcdev.ops = &toprgu_reset_ops;
+	mtk_wdt->reset_controller.rcdev.of_node = pdev->dev.of_node;
+	ret = reset_controller_register(&mtk_wdt->reset_controller.rcdev);
+	if (ret != 0)
+		dev_err(&pdev->dev,
+			"couldn't register wdt reset controller: %d\n", ret);
+}
+
 static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
 			   unsigned long action, void *data)
 {
@@ -187,9 +287,12 @@ static int mtk_wdt_probe(struct platform_device *pdev)
 	if (unlikely(err))
 		return err;
 
-	dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
+	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
 		 mtk_wdt->wdt_dev.timeout, nowayout);
 
+	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
+	if (mtk_wdt->dev_comp)
+		toprgu_register_reset_controller(pdev, WDT_SWSYSRST);
 	return 0;
 }
 
@@ -217,7 +320,12 @@ static int mtk_wdt_resume(struct device *dev)
 }
 #endif
 
+static const struct mtk_wdt_compatible mt8183_compat = {
+	.sw_rst_num = 18,
+};
+
 static const struct of_device_id mtk_wdt_dt_ids[] = {
+	{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_compat },
 	{ .compatible = "mediatek,mt6589-wdt" },
 	{ /* sentinel */ }
 };
-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] [PATCH v2 3/4] dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"
  2019-09-27 10:31 [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 1/4] dt-bindings: mediatek: mt8183: Add #reset-cells Jiaxin Yu
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
@ 2019-09-27 10:31 ` Jiaxin Yu
  2019-10-08 12:53   ` [alsa-devel] Applied "dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"" to the asoc tree Mark Brown
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
  3 siblings, 1 reply; 16+ messages in thread
From: Jiaxin Yu @ 2019-09-27 10:31 UTC (permalink / raw)
  To: broonie, mark.rutland, robh+dt, linux, wim
  Cc: alsa-devel, yong.liang, lgirdwood, jiaxin.yu, tzungbi,
	linux-mediatek, eason.yen, linux-arm-kernel

This patch add property "resets" && "reset-names" in examples so that we can
use reset controller to reset audio domain regs.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
---
 Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt b/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
index 396ba38619f6..1f1cba4152ce 100644
--- a/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
+++ b/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
@@ -4,6 +4,10 @@ Required properties:
 - compatible = "mediatek,mt68183-audio";
 - reg: register location and size
 - interrupts: should contain AFE interrupt
+- resets: Must contain an entry for each entry in reset-names
+  See ../reset/reset.txt for details.
+- reset-names: should have these reset names:
+		"audiosys";
 - power-domains: should define the power domain
 - clocks: Must contain an entry for each entry in clock-names
 - clock-names: should have these clock names:
@@ -20,6 +24,8 @@ Example:
 		compatible = "mediatek,mt8183-audio";
 		reg = <0 0x11220000 0 0x1000>;
 		interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
+		resets = <&watchdog MT8183_TOPRGU_AUDIO_SW_RST>;
+		reset-names = "audiosys";
 		power-domains = <&scpsys MT8183_POWER_DOMAIN_AUDIO>;
 		clocks = <&infrasys CLK_INFRA_AUDIO>,
 			 <&infrasys CLK_INFRA_AUDIO_26M_BCLK>,
-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup
  2019-09-27 10:31 [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback Jiaxin Yu
                   ` (2 preceding siblings ...)
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 3/4] dt-bindings: medaitek: mt8183: add property "resets" && "reset-names" Jiaxin Yu
@ 2019-09-27 10:31 ` Jiaxin Yu
  2019-10-05  6:07   ` Yingjoe Chen
                     ` (2 more replies)
  3 siblings, 3 replies; 16+ messages in thread
From: Jiaxin Yu @ 2019-09-27 10:31 UTC (permalink / raw)
  To: broonie, mark.rutland, robh+dt, linux, wim
  Cc: alsa-devel, yong.liang, lgirdwood, jiaxin.yu, tzungbi,
	linux-mediatek, eason.yen, linux-arm-kernel

Before regmap_reinit_cache we must reset audio regs as default values.
So we use reset controller unit(toprgu) to reset audio hw.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
---
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
index 4a31106d3471..721632386a50 100644
--- a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
+++ b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include "mt8183-afe-common.h"
 #include "mt8183-afe-clk.h"
@@ -1089,6 +1090,7 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 	struct mtk_base_afe *afe;
 	struct mt8183_afe_private *afe_priv;
 	struct device *dev;
+	struct reset_control *rstc;
 	int i, irq_id, ret;
 
 	afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
@@ -1126,6 +1128,19 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	rstc = devm_reset_control_get(dev, "audiosys");
+	if (IS_ERR(rstc)) {
+		ret = PTR_ERR(rstc);
+		dev_err(dev, "could not get audiosys reset:%d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_reset(rstc);
+	if (ret) {
+		dev_err(dev, "failed to trigger audio reset:%d\n", ret);
+		return ret;
+	}
+
 	/* enable clock for regcache get default value from hw */
 	afe_priv->pm_runtime_bypass_reg_ctl = true;
 	pm_runtime_get_sync(&pdev->dev);
-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
@ 2019-09-28 17:49   ` Guenter Roeck
  2019-09-30  8:17     ` Yingjoe Chen
  2019-10-03 13:49   ` Guenter Roeck
  2019-10-05  5:50   ` Yingjoe Chen
  2 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2019-09-28 17:49 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, yong.liang, lgirdwood, robh+dt,
	tzungbi, broonie, linux-mediatek, eason.yen, wim,
	linux-arm-kernel

On Fri, Sep 27, 2019 at 06:31:55PM +0800, Jiaxin Yu wrote:
> From: "yong.liang" <yong.liang@mediatek.com>
> 
> Provide assert/deassert/reset API in watchdog driver.
> Register reset controller for toprgu device in watchdog probe.
> 
> Signed-off-by: yong.liang <yong.liang@mediatek.com>
> ---
>  drivers/watchdog/Kconfig   |   1 +
>  drivers/watchdog/mtk_wdt.c | 110 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 2e07caab9db2..629249fe5305 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -717,6 +717,7 @@ config MEDIATEK_WATCHDOG
>  	tristate "Mediatek SoCs watchdog support"
>  	depends on ARCH_MEDIATEK || COMPILE_TEST
>  	select WATCHDOG_CORE
> +	select RESET_CONTROLLER
>  	help
>  	  Say Y here to include support for the watchdog timer
>  	  in Mediatek SoCs.
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 9c3d0033260d..660fb0e48d8e 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -20,6 +20,10 @@
>  #include <linux/types.h>
>  #include <linux/watchdog.h>
>  #include <linux/delay.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/reset.h>
> +#include <linux/of_device.h>
>  
>  #define WDT_MAX_TIMEOUT		31
>  #define WDT_MIN_TIMEOUT		1
> @@ -44,17 +48,113 @@
>  #define WDT_SWRST		0x14
>  #define WDT_SWRST_KEY		0x1209
>  
> +#define WDT_SWSYSRST		0x18U
> +#define WDT_SWSYS_RST_KEY	0x88000000
> +
>  #define DRV_NAME		"mtk-wdt"
>  #define DRV_VERSION		"1.0"
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  static unsigned int timeout;
>  
> +struct toprgu_reset {
> +	spinlock_t lock; /* Protects reset_controller access */
> +	void __iomem *toprgu_swrst_base;
> +	int regofs;
> +	struct reset_controller_dev rcdev;
> +};
> +
>  struct mtk_wdt_dev {
>  	struct watchdog_device wdt_dev;
>  	void __iomem *wdt_base;
> +	struct toprgu_reset reset_controller;
> +	const struct mtk_wdt_compatible *dev_comp;
> +};
> +
> +struct mtk_wdt_compatible {
> +	int sw_rst_num;
> +};
> +
> +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +				struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp |= BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +					struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp &= ~BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset(struct reset_controller_dev *rcdev,
> +			unsigned long id)
> +{
> +	int ret;
> +
> +	ret = toprgu_reset_assert(rcdev, id);
> +	if (ret)
> +		return ret;
> +
> +	return toprgu_reset_deassert(rcdev, id);
> +}
> +
> +static struct reset_control_ops toprgu_reset_ops = {
> +	.assert = toprgu_reset_assert,
> +	.deassert = toprgu_reset_deassert,
> +	.reset = toprgu_reset,
>  };
>  
> +static void toprgu_register_reset_controller(struct platform_device *pdev,
> +					     int regofs)
> +{
> +	int ret;
> +	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> +
> +	spin_lock_init(&mtk_wdt->reset_controller.lock);
> +
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> +	mtk_wdt->reset_controller.toprgu_swrst_base = mtk_wdt->wdt_base;
> +	mtk_wdt->reset_controller.regofs = regofs;
> +	mtk_wdt->reset_controller.rcdev.owner = THIS_MODULE;
> +	mtk_wdt->reset_controller.rcdev.nr_resets =
> +				mtk_wdt->dev_comp->sw_rst_num;
> +	mtk_wdt->reset_controller.rcdev.ops = &toprgu_reset_ops;
> +	mtk_wdt->reset_controller.rcdev.of_node = pdev->dev.of_node;
> +	ret = reset_controller_register(&mtk_wdt->reset_controller.rcdev);
> +	if (ret != 0)
> +		dev_err(&pdev->dev,
> +			"couldn't register wdt reset controller: %d\n", ret);
> +}
> +
>  static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
>  			   unsigned long action, void *data)
>  {
> @@ -187,9 +287,12 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>  	if (unlikely(err))
>  		return err;
>  
> -	dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
> +	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
>  		 mtk_wdt->wdt_dev.timeout, nowayout);
>  
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> +	if (mtk_wdt->dev_comp)
> +		toprgu_register_reset_controller(pdev, WDT_SWSYSRST);

Please explain why you can not use the watchdog device built-in support
for handling system resets.

Guenter

>  	return 0;
>  }
>  
> @@ -217,7 +320,12 @@ static int mtk_wdt_resume(struct device *dev)
>  }
>  #endif
>  
> +static const struct mtk_wdt_compatible mt8183_compat = {
> +	.sw_rst_num = 18,
> +};
> +
>  static const struct of_device_id mtk_wdt_dt_ids[] = {
> +	{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_compat },
>  	{ .compatible = "mediatek,mt6589-wdt" },
>  	{ /* sentinel */ }
>  };
> -- 
> 2.18.0
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-09-28 17:49   ` Guenter Roeck
@ 2019-09-30  8:17     ` Yingjoe Chen
  0 siblings, 0 replies; 16+ messages in thread
From: Yingjoe Chen @ 2019-09-30  8:17 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: mark.rutland, alsa-devel, wim, broonie, yong.liang, Jiaxin Yu,
	lgirdwood, tzungbi, robh+dt, linux-mediatek, Philipp Zabel,
	eason.yen, linux-arm-kernel

On Sat, 2019-09-28 at 10:49 -0700, Guenter Roeck wrote:
> On Fri, Sep 27, 2019 at 06:31:55PM +0800, Jiaxin Yu wrote:
> > From: "yong.liang" <yong.liang@mediatek.com>
> > 
> > Provide assert/deassert/reset API in watchdog driver.
> > Register reset controller for toprgu device in watchdog probe.
> > 
> > Signed-off-by: yong.liang <yong.liang@mediatek.com>
> > ---
> >  drivers/watchdog/Kconfig   |   1 +
> >  drivers/watchdog/mtk_wdt.c | 110 ++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 110 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 2e07caab9db2..629249fe5305 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -717,6 +717,7 @@ config MEDIATEK_WATCHDOG
> >  	tristate "Mediatek SoCs watchdog support"
> >  	depends on ARCH_MEDIATEK || COMPILE_TEST
> >  	select WATCHDOG_CORE
> > +	select RESET_CONTROLLER
> >  	help
> >  	  Say Y here to include support for the watchdog timer
> >  	  in Mediatek SoCs.

<...snip...> 

> > +static void toprgu_register_reset_controller(struct platform_device *pdev,
> > +					     int regofs)
> > +{
> > +	int ret;
> > +	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> > +
> > +	spin_lock_init(&mtk_wdt->reset_controller.lock);
> > +
> > +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> > +	mtk_wdt->reset_controller.toprgu_swrst_base = mtk_wdt->wdt_base;
> > +	mtk_wdt->reset_controller.regofs = regofs;
> > +	mtk_wdt->reset_controller.rcdev.owner = THIS_MODULE;
> > +	mtk_wdt->reset_controller.rcdev.nr_resets =
> > +				mtk_wdt->dev_comp->sw_rst_num;
> > +	mtk_wdt->reset_controller.rcdev.ops = &toprgu_reset_ops;
> > +	mtk_wdt->reset_controller.rcdev.of_node = pdev->dev.of_node;
> > +	ret = reset_controller_register(&mtk_wdt->reset_controller.rcdev);
> > +	if (ret != 0)
> > +		dev_err(&pdev->dev,
> > +			"couldn't register wdt reset controller: %d\n", ret);
> > +}
> > +
> >  static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
> >  			   unsigned long action, void *data)
> >  {
> > @@ -187,9 +287,12 @@ static int mtk_wdt_probe(struct platform_device *pdev)
> >  	if (unlikely(err))
> >  		return err;
> >  
> > -	dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
> > +	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
> >  		 mtk_wdt->wdt_dev.timeout, nowayout);
> >  
> > +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> > +	if (mtk_wdt->dev_comp)
> > +		toprgu_register_reset_controller(pdev, WDT_SWSYSRST);
> 
> Please explain why you can not use the watchdog device built-in support
> for handling system resets.

Guenter,

This is not about system reset.
Besides watchdog, MTK toprgu module also provide sub-system (eg, audio,
camera, codec and connectivity) software reset functionality. Software
can use this to reset specific sub-system.

This patch add support for this using reset controller framework. We
follow the case of drivers/clk/mediatek/reset.c to add this
functionality in watchdog driver.

Joe.C



_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
  2019-09-28 17:49   ` Guenter Roeck
@ 2019-10-03 13:49   ` Guenter Roeck
  2019-10-05  5:59     ` Yingjoe Chen
  2019-10-05  5:50   ` Yingjoe Chen
  2 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2019-10-03 13:49 UTC (permalink / raw)
  To: Jiaxin Yu, broonie, mark.rutland, robh+dt, wim
  Cc: alsa-devel, yong.liang, lgirdwood, tzungbi, linux-mediatek,
	eason.yen, linux-arm-kernel

On 9/27/19 3:31 AM, Jiaxin Yu wrote:
> From: "yong.liang" <yong.liang@mediatek.com>
> 
> Provide assert/deassert/reset API in watchdog driver.
> Register reset controller for toprgu device in watchdog probe.
> 
> Signed-off-by: yong.liang <yong.liang@mediatek.com>
> ---
>   drivers/watchdog/Kconfig   |   1 +
>   drivers/watchdog/mtk_wdt.c | 110 ++++++++++++++++++++++++++++++++++++-
>   2 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 2e07caab9db2..629249fe5305 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -717,6 +717,7 @@ config MEDIATEK_WATCHDOG
>   	tristate "Mediatek SoCs watchdog support"
>   	depends on ARCH_MEDIATEK || COMPILE_TEST
>   	select WATCHDOG_CORE
> +	select RESET_CONTROLLER
>   	help
>   	  Say Y here to include support for the watchdog timer
>   	  in Mediatek SoCs.
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 9c3d0033260d..660fb0e48d8e 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -20,6 +20,10 @@
>   #include <linux/types.h>
>   #include <linux/watchdog.h>
>   #include <linux/delay.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/reset.h>
> +#include <linux/of_device.h>
>   
>   #define WDT_MAX_TIMEOUT		31
>   #define WDT_MIN_TIMEOUT		1
> @@ -44,17 +48,113 @@
>   #define WDT_SWRST		0x14
>   #define WDT_SWRST_KEY		0x1209
>   
> +#define WDT_SWSYSRST		0x18U
> +#define WDT_SWSYS_RST_KEY	0x88000000
> +
>   #define DRV_NAME		"mtk-wdt"
>   #define DRV_VERSION		"1.0"
>   
>   static bool nowayout = WATCHDOG_NOWAYOUT;
>   static unsigned int timeout;
>   
> +struct toprgu_reset {
> +	spinlock_t lock; /* Protects reset_controller access */
> +	void __iomem *toprgu_swrst_base;
> +	int regofs;
> +	struct reset_controller_dev rcdev;
> +};
> +
>   struct mtk_wdt_dev {
>   	struct watchdog_device wdt_dev;
>   	void __iomem *wdt_base;
> +	struct toprgu_reset reset_controller;
> +	const struct mtk_wdt_compatible *dev_comp;

"dev_comp" suggests that this would be a device name. In practice,
the only value there is sw_rst_num, and the value is only
used in toprgu_register_reset_controller(). Might as well pass
it as argument and drop this pointer.

> +};
> +
> +struct mtk_wdt_compatible {
> +	int sw_rst_num;
> +};
> +
"compatible" is an odd name for this structure. I would suggest
to select a more common name such as "mtk_wdt_data".

> +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +				struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp |= BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +					struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp &= ~BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset(struct reset_controller_dev *rcdev,
> +			unsigned long id)
> +{
> +	int ret;
> +
> +	ret = toprgu_reset_assert(rcdev, id);
> +	if (ret)
> +		return ret;
> +
> +	return toprgu_reset_deassert(rcdev, id);

Unless there is additional synchronization elsewhere, parallel calls
to the -> assert, and ->reset callbacks may result in the reset being
deasserted while at least one caller (the one who called the ->assert
function) believes that it is still asserted.

[ ... and if there _is_ additional synchronization elsewhere, the
   local spinlock would be unnecessary ]

> +}
> +
> +static struct reset_control_ops toprgu_reset_ops = {
> +	.assert = toprgu_reset_assert,
> +	.deassert = toprgu_reset_deassert,
> +	.reset = toprgu_reset,
>   };
>   
> +static void toprgu_register_reset_controller(struct platform_device *pdev,
> +					     int regofs)

Since there is only one well defined offset, it seems unnecessary to pass
regofs as parameter.

> +{
> +	int ret;
> +	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> +
> +	spin_lock_init(&mtk_wdt->reset_controller.lock);
> +
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);

Duplicate, and not really needed. Just pass sw_rst_num as argument.

> +	mtk_wdt->reset_controller.toprgu_swrst_base = mtk_wdt->wdt_base;
> +	mtk_wdt->reset_controller.regofs = regofs;
> +	mtk_wdt->reset_controller.rcdev.owner = THIS_MODULE;
> +	mtk_wdt->reset_controller.rcdev.nr_resets =
> +				mtk_wdt->dev_comp->sw_rst_num;
> +	mtk_wdt->reset_controller.rcdev.ops = &toprgu_reset_ops;
> +	mtk_wdt->reset_controller.rcdev.of_node = pdev->dev.of_node;
> +	ret = reset_controller_register(&mtk_wdt->reset_controller.rcdev);
> +	if (ret != 0)
> +		dev_err(&pdev->dev,
> +			"couldn't register wdt reset controller: %d\n", ret);
> +}
> +
>   static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
>   			   unsigned long action, void *data)
>   {
> @@ -187,9 +287,12 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   	if (unlikely(err))
>   		return err;
>   
> -	dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
> +	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
>   		 mtk_wdt->wdt_dev.timeout, nowayout);
>   
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> +	if (mtk_wdt->dev_comp)
> +		toprgu_register_reset_controller(pdev, WDT_SWSYSRST);
>   	return 0;
>   }
>   
> @@ -217,7 +320,12 @@ static int mtk_wdt_resume(struct device *dev)
>   }
>   #endif
>   
> +static const struct mtk_wdt_compatible mt8183_compat = {
> +	.sw_rst_num = 18,

Please no such magic numbers. This should be a define in an include file.

> +};
> +
>   static const struct of_device_id mtk_wdt_dt_ids[] = {
> +	{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_compat },
>   	{ .compatible = "mediatek,mt6589-wdt" },
>   	{ /* sentinel */ }
>   };
> 

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
  2019-09-28 17:49   ` Guenter Roeck
  2019-10-03 13:49   ` Guenter Roeck
@ 2019-10-05  5:50   ` Yingjoe Chen
  2 siblings, 0 replies; 16+ messages in thread
From: Yingjoe Chen @ 2019-10-05  5:50 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, yong.liang, robh+dt, lgirdwood,
	tzungbi, broonie, linux-mediatek, linux-arm-kernel, eason.yen,
	wim, linux

On Fri, 2019-09-27 at 18:31 +0800, Jiaxin Yu wrote:
> From: "yong.liang" <yong.liang@mediatek.com>
> 
> Provide assert/deassert/reset API in watchdog driver.
> Register reset controller for toprgu device in watchdog probe.

I think we could improve this commit message so it is easier to
understand what is provided by this patch. You could add something like
this:

Besides watchdog, MTK toprgu module also provide sub-system (eg, audio,
camera, codec and connectivity) software reset functionality.

> 
> Signed-off-by: yong.liang <yong.liang@mediatek.com>
> ---
>  drivers/watchdog/Kconfig   |   1 +
>  drivers/watchdog/mtk_wdt.c | 110 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 2e07caab9db2..629249fe5305 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -717,6 +717,7 @@ config MEDIATEK_WATCHDOG
>  	tristate "Mediatek SoCs watchdog support"
>  	depends on ARCH_MEDIATEK || COMPILE_TEST
>  	select WATCHDOG_CORE
> +	select RESET_CONTROLLER
>  	help
>  	  Say Y here to include support for the watchdog timer
>  	  in Mediatek SoCs.
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 9c3d0033260d..660fb0e48d8e 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -20,6 +20,10 @@
>  #include <linux/types.h>
>  #include <linux/watchdog.h>
>  #include <linux/delay.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/reset.h>
> +#include <linux/of_device.h>

sorting please.

>  
>  #define WDT_MAX_TIMEOUT		31
>  #define WDT_MIN_TIMEOUT		1
> @@ -44,17 +48,113 @@
>  #define WDT_SWRST		0x14
>  #define WDT_SWRST_KEY		0x1209
>  
> +#define WDT_SWSYSRST		0x18U
> +#define WDT_SWSYS_RST_KEY	0x88000000
> +
>  #define DRV_NAME		"mtk-wdt"
>  #define DRV_VERSION		"1.0"
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  static unsigned int timeout;
>  
> +struct toprgu_reset {
> +	spinlock_t lock; /* Protects reset_controller access */
> +	void __iomem *toprgu_swrst_base;
> +	int regofs;
> +	struct reset_controller_dev rcdev;
> +};

I'm not sure we need a separate struct, especially when you need to
duplicate wdt_base into this struct.
After removing regofs/swrst_base, this struct only contain 2 members.
Maybe you should just merge this into mtk_wdt_dev.


> +
>  struct mtk_wdt_dev {
>  	struct watchdog_device wdt_dev;
>  	void __iomem *wdt_base;
> +	struct toprgu_reset reset_controller;
> +	const struct mtk_wdt_compatible *dev_comp;
> +};
> +
> +struct mtk_wdt_compatible {
> +	int sw_rst_num;
> +};
> +
> +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +				struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp |= BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	unsigned int tmp;
> +	unsigned long flags;
> +	struct toprgu_reset *data = container_of(rcdev,
> +					struct toprgu_reset, rcdev);
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> +	tmp &= ~BIT(id);
> +	tmp |= WDT_SWSYS_RST_KEY;
> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int toprgu_reset(struct reset_controller_dev *rcdev,
> +			unsigned long id)
> +{
> +	int ret;
> +
> +	ret = toprgu_reset_assert(rcdev, id);
> +	if (ret)
> +		return ret;
> +
> +	return toprgu_reset_deassert(rcdev, id);
> +}
> +
> +static struct reset_control_ops toprgu_reset_ops = {

static const


> +	.assert = toprgu_reset_assert,
> +	.deassert = toprgu_reset_deassert,
> +	.reset = toprgu_reset,
>  };
>  
> +static void toprgu_register_reset_controller(struct platform_device *pdev,
> +					     int regofs)
> +{
> +	int ret;
> +	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> +
> +	spin_lock_init(&mtk_wdt->reset_controller.lock);
> +
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> +	mtk_wdt->reset_controller.toprgu_swrst_base = mtk_wdt->wdt_base;
> +	mtk_wdt->reset_controller.regofs = regofs;
> +	mtk_wdt->reset_controller.rcdev.owner = THIS_MODULE;
> +	mtk_wdt->reset_controller.rcdev.nr_resets =
> +				mtk_wdt->dev_comp->sw_rst_num;
> +	mtk_wdt->reset_controller.rcdev.ops = &toprgu_reset_ops;
> +	mtk_wdt->reset_controller.rcdev.of_node = pdev->dev.of_node;
> +	ret = reset_controller_register(&mtk_wdt->reset_controller.rcdev);
> +	if (ret != 0)
> +		dev_err(&pdev->dev,
> +			"couldn't register wdt reset controller: %d\n", ret);

If this fail, you should return it and make mtk_wdt_probe also return
fail.

> +}
> +
>  static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
>  			   unsigned long action, void *data)
>  {
> @@ -187,9 +287,12 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>  	if (unlikely(err))
>  		return err;
>  
> -	dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
> +	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
>  		 mtk_wdt->wdt_dev.timeout, nowayout);
>  
> +	mtk_wdt->dev_comp = of_device_get_match_data(&pdev->dev);
> +	if (mtk_wdt->dev_comp)
> +		toprgu_register_reset_controller(pdev, WDT_SWSYSRST);

Is this register offset WDT_SWSYSRST identical in all chips?
If it is, you should hardcode it in assert/deassert, just like how we
access other watchdog registers.
If not, you should put it in mtk_wdt_compatible.

>  	return 0;
>  }
>  
> @@ -217,7 +320,12 @@ static int mtk_wdt_resume(struct device *dev)
>  }
>  #endif
>  
> +static const struct mtk_wdt_compatible mt8183_compat = {
> +	.sw_rst_num = 18,
> +};
> +
>  static const struct of_device_id mtk_wdt_dt_ids[] = {
> +	{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_compat },
>  	{ .compatible = "mediatek,mt6589-wdt" },


sorting please

Joe.C


>  	{ /* sentinel */ }
>  };


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-10-03 13:49   ` Guenter Roeck
@ 2019-10-05  5:59     ` Yingjoe Chen
  2019-10-05 14:46       ` Guenter Roeck
  0 siblings, 1 reply; 16+ messages in thread
From: Yingjoe Chen @ 2019-10-05  5:59 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: mark.rutland, alsa-devel, broonie, yong.liang, Jiaxin Yu,
	lgirdwood, tzungbi, robh+dt, linux-mediatek, Philipp Zabel,
	eason.yen, wim, linux-arm-kernel

On Thu, 2019-10-03 at 06:49 -0700, Guenter Roeck wrote:
> On 9/27/19 3:31 AM, Jiaxin Yu wrote:

<snip..>


> > +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
> > +			       unsigned long id)
> > +{
> > +	unsigned int tmp;
> > +	unsigned long flags;
> > +	struct toprgu_reset *data = container_of(rcdev,
> > +				struct toprgu_reset, rcdev);
> > +
> > +	spin_lock_irqsave(&data->lock, flags);
> > +
> > +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> > +	tmp |= BIT(id);
> > +	tmp |= WDT_SWSYS_RST_KEY;
> > +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> > +
> > +	spin_unlock_irqrestore(&data->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
> > +				 unsigned long id)
> > +{
> > +	unsigned int tmp;
> > +	unsigned long flags;
> > +	struct toprgu_reset *data = container_of(rcdev,
> > +					struct toprgu_reset, rcdev);
> > +
> > +	spin_lock_irqsave(&data->lock, flags);
> > +
> > +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> > +	tmp &= ~BIT(id);
> > +	tmp |= WDT_SWSYS_RST_KEY;
> > +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> > +
> > +	spin_unlock_irqrestore(&data->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int toprgu_reset(struct reset_controller_dev *rcdev,
> > +			unsigned long id)
> > +{
> > +	int ret;
> > +
> > +	ret = toprgu_reset_assert(rcdev, id);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return toprgu_reset_deassert(rcdev, id);
> 
> Unless there is additional synchronization elsewhere, parallel calls
> to the -> assert, and ->reset callbacks may result in the reset being
> deasserted while at least one caller (the one who called the ->assert
> function) believes that it is still asserted.
> 
> [ ... and if there _is_ additional synchronization elsewhere, the
>    local spinlock would be unnecessary ]
> 

I'm not sure if this count as additional synchronization, but you could
get exclusive control to the reset by calling
reset_control_get_exclusive so others won't try to reset the component
while you are using it.

In this case, you still need spinlock because other drivers might trying
to reset their components and they share same register.

Joe.C


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
@ 2019-10-05  6:07   ` Yingjoe Chen
  2019-10-08 12:11   ` Mark Brown
  2019-10-08 12:53   ` [alsa-devel] Applied "ASoC: mt8183: fix audio playback slowly after playback during bootup" to the asoc tree Mark Brown
  2 siblings, 0 replies; 16+ messages in thread
From: Yingjoe Chen @ 2019-10-05  6:07 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, yong.liang, robh+dt, lgirdwood,
	tzungbi, broonie, linux-mediatek, linux-arm-kernel, eason.yen,
	wim, linux

On Fri, 2019-09-27 at 18:31 +0800, Jiaxin Yu wrote:
> Before regmap_reinit_cache we must reset audio regs as default values.
> So we use reset controller unit(toprgu) to reset audio hw.
> 
> Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>


This one looks good to me. You could add this if you want


Reviewed-by: Yingjoe Chen <yingjoe.chen@mediatek.com>


Joe.C

> ---
>  sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
> index 4a31106d3471..721632386a50 100644
> --- a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
> +++ b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
> @@ -11,6 +11,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>  
>  #include "mt8183-afe-common.h"
>  #include "mt8183-afe-clk.h"
> @@ -1089,6 +1090,7 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
>  	struct mtk_base_afe *afe;
>  	struct mt8183_afe_private *afe_priv;
>  	struct device *dev;
> +	struct reset_control *rstc;
>  	int i, irq_id, ret;
>  
>  	afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
> @@ -1126,6 +1128,19 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	rstc = devm_reset_control_get(dev, "audiosys");
> +	if (IS_ERR(rstc)) {
> +		ret = PTR_ERR(rstc);
> +		dev_err(dev, "could not get audiosys reset:%d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = reset_control_reset(rstc);
> +	if (ret) {
> +		dev_err(dev, "failed to trigger audio reset:%d\n", ret);
> +		return ret;
> +	}
> +
>  	/* enable clock for regcache get default value from hw */
>  	afe_priv->pm_runtime_bypass_reg_ctl = true;
>  	pm_runtime_get_sync(&pdev->dev);


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-10-05  5:59     ` Yingjoe Chen
@ 2019-10-05 14:46       ` Guenter Roeck
  2019-10-08 14:08         ` Philipp Zabel
  0 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2019-10-05 14:46 UTC (permalink / raw)
  To: Yingjoe Chen
  Cc: mark.rutland, alsa-devel, broonie, yong.liang, Jiaxin Yu,
	lgirdwood, tzungbi, robh+dt, linux-mediatek, Philipp Zabel,
	eason.yen, wim, linux-arm-kernel

On 10/4/19 10:59 PM, Yingjoe Chen wrote:
> On Thu, 2019-10-03 at 06:49 -0700, Guenter Roeck wrote:
>> On 9/27/19 3:31 AM, Jiaxin Yu wrote:
> 
> <snip..>
> 
> 
>>> +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
>>> +			       unsigned long id)
>>> +{
>>> +	unsigned int tmp;
>>> +	unsigned long flags;
>>> +	struct toprgu_reset *data = container_of(rcdev,
>>> +				struct toprgu_reset, rcdev);
>>> +
>>> +	spin_lock_irqsave(&data->lock, flags);
>>> +
>>> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
>>> +	tmp |= BIT(id);
>>> +	tmp |= WDT_SWSYS_RST_KEY;
>>> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
>>> +
>>> +	spin_unlock_irqrestore(&data->lock, flags);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
>>> +				 unsigned long id)
>>> +{
>>> +	unsigned int tmp;
>>> +	unsigned long flags;
>>> +	struct toprgu_reset *data = container_of(rcdev,
>>> +					struct toprgu_reset, rcdev);
>>> +
>>> +	spin_lock_irqsave(&data->lock, flags);
>>> +
>>> +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
>>> +	tmp &= ~BIT(id);
>>> +	tmp |= WDT_SWSYS_RST_KEY;
>>> +	writel(tmp, data->toprgu_swrst_base + data->regofs);
>>> +
>>> +	spin_unlock_irqrestore(&data->lock, flags);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int toprgu_reset(struct reset_controller_dev *rcdev,
>>> +			unsigned long id)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = toprgu_reset_assert(rcdev, id);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return toprgu_reset_deassert(rcdev, id);
>>
>> Unless there is additional synchronization elsewhere, parallel calls
>> to the -> assert, and ->reset callbacks may result in the reset being
>> deasserted while at least one caller (the one who called the ->assert
>> function) believes that it is still asserted.
>>
>> [ ... and if there _is_ additional synchronization elsewhere, the
>>     local spinlock would be unnecessary ]
>>
> 
> I'm not sure if this count as additional synchronization, but you could
> get exclusive control to the reset by calling
> reset_control_get_exclusive so others won't try to reset the component
> while you are using it.
> 
> In this case, you still need spinlock because other drivers might trying
> to reset their components and they share same register.
> 
That isn't what I meant. I referred to synchronization in the reset
controller core. AFAICS the reset controller core prevents parallel
calls into the same reset controller driver using atomics. Unfortunately,
it is not well defined if additional synchronization on driver level
is needed - some drivers implement it, some drivers don't, and I don't
find a documentation. Maybe Philip can provide guidance.

Thanks,
Guenter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
  2019-10-05  6:07   ` Yingjoe Chen
@ 2019-10-08 12:11   ` Mark Brown
  2019-10-08 12:53   ` [alsa-devel] Applied "ASoC: mt8183: fix audio playback slowly after playback during bootup" to the asoc tree Mark Brown
  2 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2019-10-08 12:11 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, yong.liang, lgirdwood, tzungbi,
	robh+dt, linux-mediatek, linux-arm-kernel, eason.yen, wim, linux

[-- Attachment #1.1: Type: text/plain, Size: 703 bytes --]

On Fri, Sep 27, 2019 at 06:31:57PM +0800, Jiaxin Yu wrote:

> +	rstc = devm_reset_control_get(dev, "audiosys");
> +	if (IS_ERR(rstc)) {
> +		ret = PTR_ERR(rstc);
> +		dev_err(dev, "could not get audiosys reset:%d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = reset_control_reset(rstc);
> +	if (ret) {
> +		dev_err(dev, "failed to trigger audio reset:%d\n", ret);
> +		return ret;
> +	}

This means that we're going to be incompatible with old DT bindings that
don't specify a reset controller.  I don't know how widely used these
bindings are so we may be able to get away with this and I'll apply but
we shouldn't be doing it, the code might need to be fixed to make this
optional if people complain.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] Applied "ASoC: mt8183: fix audio playback slowly after playback during bootup" to the asoc tree
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
  2019-10-05  6:07   ` Yingjoe Chen
  2019-10-08 12:11   ` Mark Brown
@ 2019-10-08 12:53   ` Mark Brown
  2 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2019-10-08 12:53 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, tzungbi, yong.liang, lgirdwood,
	jiaxin.yu, robh+dt, Mark Brown, linux-mediatek, linux, eason.yen,
	wim, linux-arm-kernel

The patch

   ASoC: mt8183: fix audio playback slowly after playback during bootup

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 9e985503ee4b23d576c303a17dfe52cfc8f32727 Mon Sep 17 00:00:00 2001
From: Jiaxin Yu <jiaxin.yu@mediatek.com>
Date: Fri, 27 Sep 2019 18:31:57 +0800
Subject: [PATCH] ASoC: mt8183: fix audio playback slowly after playback during
 bootup

Before regmap_reinit_cache we must reset audio regs as default values.
So we use reset controller unit(toprgu) to reset audio hw.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
Reviewed-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
Link: https://lore.kernel.org/r/1569580317-21181-5-git-send-email-jiaxin.yu@mediatek.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
index 4a31106d3471..721632386a50 100644
--- a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
+++ b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include "mt8183-afe-common.h"
 #include "mt8183-afe-clk.h"
@@ -1089,6 +1090,7 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 	struct mtk_base_afe *afe;
 	struct mt8183_afe_private *afe_priv;
 	struct device *dev;
+	struct reset_control *rstc;
 	int i, irq_id, ret;
 
 	afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
@@ -1126,6 +1128,19 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	rstc = devm_reset_control_get(dev, "audiosys");
+	if (IS_ERR(rstc)) {
+		ret = PTR_ERR(rstc);
+		dev_err(dev, "could not get audiosys reset:%d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_reset(rstc);
+	if (ret) {
+		dev_err(dev, "failed to trigger audio reset:%d\n", ret);
+		return ret;
+	}
+
 	/* enable clock for regcache get default value from hw */
 	afe_priv->pm_runtime_bypass_reg_ctl = true;
 	pm_runtime_get_sync(&pdev->dev);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [alsa-devel] Applied "dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"" to the asoc tree
  2019-09-27 10:31 ` [alsa-devel] [PATCH v2 3/4] dt-bindings: medaitek: mt8183: add property "resets" && "reset-names" Jiaxin Yu
@ 2019-10-08 12:53   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2019-10-08 12:53 UTC (permalink / raw)
  To: Jiaxin Yu
  Cc: mark.rutland, alsa-devel, tzungbi, yong.liang, lgirdwood,
	jiaxin.yu, robh+dt, Mark Brown, linux-mediatek, linux, eason.yen,
	wim, linux-arm-kernel

The patch

   dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 8d6aa1367a7df44bb5939c4bb2727b8d8f7d01b3 Mon Sep 17 00:00:00 2001
From: Jiaxin Yu <jiaxin.yu@mediatek.com>
Date: Fri, 27 Sep 2019 18:31:56 +0800
Subject: [PATCH] dt-bindings: medaitek: mt8183: add property "resets" &&
 "reset-names"

This patch add property "resets" && "reset-names" in examples so that we can
use reset controller to reset audio domain regs.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
Link: https://lore.kernel.org/r/1569580317-21181-4-git-send-email-jiaxin.yu@mediatek.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt b/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
index 396ba38619f6..1f1cba4152ce 100644
--- a/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
+++ b/Documentation/devicetree/bindings/sound/mt8183-afe-pcm.txt
@@ -4,6 +4,10 @@ Required properties:
 - compatible = "mediatek,mt68183-audio";
 - reg: register location and size
 - interrupts: should contain AFE interrupt
+- resets: Must contain an entry for each entry in reset-names
+  See ../reset/reset.txt for details.
+- reset-names: should have these reset names:
+		"audiosys";
 - power-domains: should define the power domain
 - clocks: Must contain an entry for each entry in clock-names
 - clock-names: should have these clock names:
@@ -20,6 +24,8 @@ Example:
 		compatible = "mediatek,mt8183-audio";
 		reg = <0 0x11220000 0 0x1000>;
 		interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
+		resets = <&watchdog MT8183_TOPRGU_AUDIO_SW_RST>;
+		reset-names = "audiosys";
 		power-domains = <&scpsys MT8183_POWER_DOMAIN_AUDIO>;
 		clocks = <&infrasys CLK_INFRA_AUDIO>,
 			 <&infrasys CLK_INFRA_AUDIO_26M_BCLK>,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller
  2019-10-05 14:46       ` Guenter Roeck
@ 2019-10-08 14:08         ` Philipp Zabel
  0 siblings, 0 replies; 16+ messages in thread
From: Philipp Zabel @ 2019-10-08 14:08 UTC (permalink / raw)
  To: Guenter Roeck, Yingjoe Chen
  Cc: mark.rutland, alsa-devel, broonie, yong.liang, Jiaxin Yu,
	lgirdwood, tzungbi, robh+dt, linux-mediatek, eason.yen, wim,
	linux-arm-kernel

Hi Guenter, Yingjoe,

On Sat, 2019-10-05 at 07:46 -0700, Guenter Roeck wrote:
> On 10/4/19 10:59 PM, Yingjoe Chen wrote:
> > On Thu, 2019-10-03 at 06:49 -0700, Guenter Roeck wrote:
> > > On 9/27/19 3:31 AM, Jiaxin Yu wrote:
> > 
> > <snip..>
> > 
> > 
> > > > +static int toprgu_reset_assert(struct reset_controller_dev *rcdev,
> > > > +			       unsigned long id)
> > > > +{
> > > > +	unsigned int tmp;
> > > > +	unsigned long flags;
> > > > +	struct toprgu_reset *data = container_of(rcdev,
> > > > +				struct toprgu_reset, rcdev);
> > > > +
> > > > +	spin_lock_irqsave(&data->lock, flags);
> > > > +
> > > > +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> > > > +	tmp |= BIT(id);
> > > > +	tmp |= WDT_SWSYS_RST_KEY;
> > > > +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> > > > +
> > > > +	spin_unlock_irqrestore(&data->lock, flags);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int toprgu_reset_deassert(struct reset_controller_dev *rcdev,
> > > > +				 unsigned long id)
> > > > +{
> > > > +	unsigned int tmp;
> > > > +	unsigned long flags;
> > > > +	struct toprgu_reset *data = container_of(rcdev,
> > > > +					struct toprgu_reset, rcdev);
> > > > +
> > > > +	spin_lock_irqsave(&data->lock, flags);
> > > > +
> > > > +	tmp = __raw_readl(data->toprgu_swrst_base + data->regofs);
> > > > +	tmp &= ~BIT(id);
> > > > +	tmp |= WDT_SWSYS_RST_KEY;
> > > > +	writel(tmp, data->toprgu_swrst_base + data->regofs);
> > > > +
> > > > +	spin_unlock_irqrestore(&data->lock, flags);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int toprgu_reset(struct reset_controller_dev *rcdev,
> > > > +			unsigned long id)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	ret = toprgu_reset_assert(rcdev, id);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	return toprgu_reset_deassert(rcdev, id);
> > > 
> > > Unless there is additional synchronization elsewhere, parallel calls
> > > to the -> assert, and ->reset callbacks may result in the reset being
> > > deasserted while at least one caller (the one who called the ->assert
> > > function) believes that it is still asserted.
> > > 
> > > [ ... and if there _is_ additional synchronization elsewhere, the
> > >     local spinlock would be unnecessary ]
> > > 
> > 
> > I'm not sure if this count as additional synchronization, but you could
> > get exclusive control to the reset by calling
> > reset_control_get_exclusive so others won't try to reset the component
> > while you are using it.
> > 
> > In this case, you still need spinlock because other drivers might trying
> > to reset their components and they share same register.
> 
> That isn't what I meant. I referred to synchronization in the reset
> controller core. AFAICS the reset controller core prevents parallel
> calls into the same reset controller driver using atomics.

No, it doesn't. The atomics in struct reset_control prevent parallel
calls on the same, reset control only, for shared reset controls.
Two calls on different reset controls can still run simultaneously on
the same rcdev.

> Unfortunately, it is not well defined if additional synchronization on
> driver level is needed - some drivers implement it, some drivers
> don't,

I think all drivers protect read/modify/write cycles to shared registers
with a spinlock. Those that don't either have separate set/clear
registers or use regmap, otherwise it might be a bug.

> and I don't find a documentation.

I am aware this is a problem.

regards
Philipp
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 10:31 [alsa-devel] [PATCH v2 0/4] ASoC: mt8183: fix audio playback slowly after playback Jiaxin Yu
2019-09-27 10:31 ` [alsa-devel] [PATCH v2 1/4] dt-bindings: mediatek: mt8183: Add #reset-cells Jiaxin Yu
2019-09-27 10:31 ` [alsa-devel] [PATCH v2 2/4] watchdog: mtk_wdt: mt8183: Add reset controller Jiaxin Yu
2019-09-28 17:49   ` Guenter Roeck
2019-09-30  8:17     ` Yingjoe Chen
2019-10-03 13:49   ` Guenter Roeck
2019-10-05  5:59     ` Yingjoe Chen
2019-10-05 14:46       ` Guenter Roeck
2019-10-08 14:08         ` Philipp Zabel
2019-10-05  5:50   ` Yingjoe Chen
2019-09-27 10:31 ` [alsa-devel] [PATCH v2 3/4] dt-bindings: medaitek: mt8183: add property "resets" && "reset-names" Jiaxin Yu
2019-10-08 12:53   ` [alsa-devel] Applied "dt-bindings: medaitek: mt8183: add property "resets" && "reset-names"" to the asoc tree Mark Brown
2019-09-27 10:31 ` [alsa-devel] [PATCH v2 4/4] ASoC: mt8183: fix audio playback slowly after playback during bootup Jiaxin Yu
2019-10-05  6:07   ` Yingjoe Chen
2019-10-08 12:11   ` Mark Brown
2019-10-08 12:53   ` [alsa-devel] Applied "ASoC: mt8183: fix audio playback slowly after playback during bootup" to the asoc tree Mark Brown

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org alsa-devel@archiver.kernel.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox