[PATCH v4 0/3] Fix Arndale Octa/Peach Pi boot on Audio subsystem clocks

[PATCH v4 0/3] Fix Arndale Octa/Peach Pi boot on Audio subsystem clocks

[Krzysztof Kozlowski]

Hi,


Changes since v3
================
1. Patch 1/3: Fix issues pointed by Sylwester.
2. Add Javier's tested-by [1]

Changes since v2
================
1. Patch 1 applied ("clk: samsung: Fix double add of syscore ops after
   driver rebind"), remove it.
2. Squash patch 5 with "clk: samsung: Fix clock disable
   failure because domain being gated". Suggested by Sylwester.
3. Patch 1/3: Fix issues pointed by Sylwester.
4. Patch 2/3: Fix redundant clk_disable when removing driver (clk is
   already disabled). Add missing check !=null when removing driver.
5. Patch 3/3: Extend commit message.

Tomasz Figa had some questions about power domains on Exynos5420 in
relation to this patchset, but I haven't addressed all of them yet.

Changes since v1
================
1. clocks-audss: Reimplement own clock register functions instead
   changing clk API. Minor fixes. (after idea from Tomasz Figa)
2. Add new patches: fix for pinctrl and minor fixes in clk-audss.

Description
===========
This patchset tries to solve dependency between AudioSS components
(clocks and GPIO) and main clock controller on Exynos 5420 platform.

This solves boot failure of Peach Pi/Pit and Arndale Octa [2].

Any access to memory of audss block (like checking if clock is enabled
or configuring GPIO) will hang if main audss clock is gated.

Tested on Arndale Octa board.

[1] https://lkml.org/lkml/2014/11/26/420
[2] http://www.spinics.net/lists/linux-samsung-soc/msg39331.html

Best regards,
Krzysztof Kozlowski


Krzysztof Kozlowski (3):
  clk: samsung: Fix clock disable failure because domain being gated
  pinctrl: exynos: Fix GPIO setup failure because domain clock being
    gated
  ARM: dts: exynos5420: Add clock for audss pinctrl (fixing GPIO setup
    failure)

 .../bindings/pinctrl/samsung-pinctrl.txt           |   6 +
 arch/arm/boot/dts/exynos5420-pinctrl.dtsi          |   3 +
 drivers/clk/samsung/clk-exynos-audss.c             | 339 ++++++++++++++++++---
 drivers/pinctrl/samsung/pinctrl-samsung.c          | 111 ++++++-
 drivers/pinctrl/samsung/pinctrl-samsung.h          |   2 +
 5 files changed, 418 insertions(+), 43 deletions(-)

-- 
1.9.1

[PATCH v4 1/3] clk: samsung: Fix clock disable failure because domain being gated

[Krzysztof Kozlowski]

Audio subsystem clocks are located in separate block. If clock for this
block (from main clock domain) 'mau_epll' is gated then any read or
write to audss registers will block.

This was observed on Exynos 5420 platforms (Arndale Octa and Peach
Pi/Pit) after introducing runtime PM to pl330 DMA driver. After that
commit the 'mau_epll' was gated, because the "amba" clock was disabled
and there were no more users of mau_epll. The system hang on disabling
unused clocks from audss block.

Unfortunately the 'mau_epll' clock is not parent of some of audss clocks.

Whenever system wants to operate on audss clocks it has to enable epll
clock. The solution reuses common clk-gate/divider/mux code and duplicates
clk_register_*() functions.

Additionally this patch fixes memory leak of clock gate/divider/mux
structures. The leak exists in generic clk_register_*() functions. Patch
replaces them with custom code with managed allocation.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/clk/samsung/clk-exynos-audss.c | 339 +++++++++++++++++++++++++++++----
 1 file changed, 303 insertions(+), 36 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index 7c4368e75ede..0460618807e0 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -29,6 +29,13 @@ static DEFINE_SPINLOCK(lock);
 static struct clk **clk_table;
 static void __iomem *reg_base;
 static struct clk_onecell_data clk_data;
+/*
+ * On Exynos5420 this will be a clock which has to be enabled before any
+ * access to audss registers. Typically a child of EPLL.
+ *
+ * On other platforms this will be -ENODEV.
+ */
+static struct clk *epll;
 
 #define ASS_CLK_SRC 0x0
 #define ASS_CLK_DIV 0x4
@@ -75,6 +82,247 @@ static const struct of_device_id exynos_audss_clk_of_match[] = {
 	{},
 };
 
+static int pll_clk_enable(void)
+{
+	if (!IS_ERR(epll))
+		return clk_enable(epll);
+
+	return 0;
+}
+
+static void pll_clk_disable(void)
+{
+	if (!IS_ERR(epll))
+		clk_disable(epll);
+}
+
+static int audss_clk_gate_enable(struct clk_hw *hw)
+{
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = clk_gate_ops.enable(hw);
+
+	pll_clk_disable();
+
+	return ret;
+}
+
+static void audss_clk_gate_disable(struct clk_hw *hw)
+{
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret)
+		return;
+
+	clk_gate_ops.disable(hw);
+
+	pll_clk_disable();
+}
+
+static int audss_clk_gate_is_enabled(struct clk_hw *hw)
+{
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = clk_gate_ops.is_enabled(hw);
+
+	pll_clk_disable();
+
+	return ret;
+}
+
+static const struct clk_ops audss_clk_gate_ops = {
+	.enable = audss_clk_gate_enable,
+	.disable = audss_clk_gate_disable,
+	.is_enabled = audss_clk_gate_is_enabled,
+};
+
+/*
+ * A simplified copy of clk-gate.c:clk_register_gate() to mimic
+ * clk-gate behavior while using customized ops.
+ */
+static struct clk *audss_clk_register_gate(struct device *dev, const char *name,
+		const char *parent_name, unsigned long flags,
+		void __iomem *reg, u8 bit_idx)
+{
+	struct clk_gate *gate;
+	struct clk_init_data init;
+
+	gate = devm_kzalloc(dev, sizeof(struct clk_gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &audss_clk_gate_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = (parent_name ? &parent_name : NULL);
+	init.num_parents = (parent_name ? 1 : 0);
+
+	/* struct clk_gate assignments */
+	gate->reg = reg;
+	gate->bit_idx = bit_idx;
+	gate->lock = &lock;
+	gate->hw.init = &init;
+
+	return clk_register(dev, &gate->hw);
+}
+
+static unsigned long audss_clk_divider_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	unsigned long ret;
+
+	ret = pll_clk_enable();
+	if (ret) {
+		WARN(ret, "Could not enable epll clock\n");
+		return parent_rate;
+	}
+
+	ret = clk_divider_ops.recalc_rate(hw, parent_rate);
+
+	pll_clk_disable();
+
+	return ret;
+}
+
+static long audss_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *prate)
+{
+	return clk_divider_ops.round_rate(hw, rate, prate);
+}
+
+static int audss_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = clk_divider_ops.set_rate(hw, rate, parent_rate);
+
+	pll_clk_disable();
+
+	return ret;
+}
+
+static const struct clk_ops audss_clk_divider_ops = {
+	.recalc_rate = audss_clk_divider_recalc_rate,
+	.round_rate = audss_clk_divider_round_rate,
+	.set_rate = audss_clk_divider_set_rate,
+};
+
+/*
+ * A simplified copy of clk-divider.c:clk_register_divider() to mimic
+ * clk-divider behavior while using customized ops.
+ */
+static struct clk *audss_clk_register_divider(struct device *dev,
+		const char *name, const char *parent_name, unsigned long flags,
+		void __iomem *reg, u8 shift, u8 width)
+{
+	struct clk_divider *div;
+	struct clk_init_data init;
+
+	div = devm_kzalloc(dev, sizeof(struct clk_divider), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &audss_clk_divider_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = (parent_name ? &parent_name : NULL);
+	init.num_parents = (parent_name ? 1 : 0);
+
+	/* struct clk_divider assignments */
+	div->reg = reg;
+	div->shift = shift;
+	div->width = width;
+	div->lock = &lock;
+	div->hw.init = &init;
+
+	return clk_register(dev, &div->hw);
+}
+
+static u8 audss_clk_mux_get_parent(struct clk_hw *hw)
+{
+	u8 parent;
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret) {
+		WARN(ret, "Could not enable epll clock\n");
+		return ret; /* Just like clk_mux_get_parent() */
+	}
+
+	parent = clk_mux_ops.get_parent(hw);
+
+	pll_clk_disable();
+
+	return parent;
+}
+
+static int audss_clk_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+	int ret;
+
+	ret = pll_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = clk_mux_ops.set_parent(hw, index);
+
+	pll_clk_disable();
+
+	return ret;
+}
+
+static const struct clk_ops audss_clk_mux_ops = {
+	.get_parent = audss_clk_mux_get_parent,
+	.set_parent = audss_clk_mux_set_parent,
+	.determine_rate = __clk_mux_determine_rate,
+};
+
+/*
+ * A simplified copy of clk-mux.c:clk_register_mux_table() to mimic
+ * clk-mux behavior while using customized ops.
+ */
+static struct clk *audss_clk_register_mux(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		void __iomem *reg, u8 shift, u8 width)
+{
+	struct clk_mux *mux;
+	struct clk_init_data init;
+	u32 mask = BIT(width) - 1;
+
+	mux = devm_kzalloc(dev, sizeof(struct clk_mux), GFP_KERNEL);
+	if (!mux)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &audss_clk_mux_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = parent_names;
+	init.num_parents = num_parents;
+
+	/* struct clk_mux assignments */
+	mux->reg = reg;
+	mux->shift = shift;
+	mux->mask = mask;
+	mux->lock = &lock;
+	mux->hw.init = &init;
+
+	return clk_register(dev, &mux->hw);
+}
+
 /* register exynos_audss clocks */
 static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
@@ -98,6 +346,8 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to map audss registers\n");
 		return PTR_ERR(reg_base);
 	}
+	/* EPLL don't have to be enabled for boards other than Exynos5420 */
+	epll = ERR_PTR(-ENODEV);
 
 	clk_table = devm_kzalloc(&pdev->dev,
 				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
@@ -115,12 +365,24 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	pll_in = devm_clk_get(&pdev->dev, "pll_in");
 	if (!IS_ERR(pll_ref))
 		mout_audss_p[0] = __clk_get_name(pll_ref);
-	if (!IS_ERR(pll_in))
+	if (!IS_ERR(pll_in)) {
 		mout_audss_p[1] = __clk_get_name(pll_in);
-	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
-				mout_audss_p, ARRAY_SIZE(mout_audss_p),
-				CLK_SET_RATE_NO_REPARENT,
-				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
+
+		if (variant == TYPE_EXYNOS5420) {
+			epll = pll_in;
+
+			ret = clk_prepare(epll);
+			if (ret) {
+				dev_err(&pdev->dev,
+					"failed to prepare the epll clock\n");
+				return ret;
+			}
+		}
+	}
+
+	clk_table[EXYNOS_MOUT_AUDSS] = audss_clk_register_mux(&pdev->dev,
+			"mout_audss", mout_audss_p, ARRAY_SIZE(mout_audss_p),
+			CLK_SET_RATE_NO_REPARENT, reg_base + ASS_CLK_SRC, 0, 1);
 
 	cdclk = devm_clk_get(&pdev->dev, "cdclk");
 	sclk_audio = devm_clk_get(&pdev->dev, "sclk_audio");
@@ -128,50 +390,49 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		mout_i2s_p[1] = __clk_get_name(cdclk);
 	if (!IS_ERR(sclk_audio))
 		mout_i2s_p[2] = __clk_get_name(sclk_audio);
-	clk_table[EXYNOS_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
-				mout_i2s_p, ARRAY_SIZE(mout_i2s_p),
-				CLK_SET_RATE_NO_REPARENT,
-				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
+	clk_table[EXYNOS_MOUT_I2S] = audss_clk_register_mux(&pdev->dev,
+			"mout_i2s", mout_i2s_p, ARRAY_SIZE(mout_i2s_p),
+			CLK_SET_RATE_NO_REPARENT, reg_base + ASS_CLK_SRC, 2, 2);
 
-	clk_table[EXYNOS_DOUT_SRP] = clk_register_divider(NULL, "dout_srp",
-				"mout_audss", 0, reg_base + ASS_CLK_DIV, 0, 4,
-				0, &lock);
+	clk_table[EXYNOS_DOUT_SRP] = audss_clk_register_divider(&pdev->dev,
+			"dout_srp", "mout_audss", 0,
+			reg_base + ASS_CLK_DIV, 0, 4);
 
-	clk_table[EXYNOS_DOUT_AUD_BUS] = clk_register_divider(NULL,
-				"dout_aud_bus", "dout_srp", 0,
-				reg_base + ASS_CLK_DIV, 4, 4, 0, &lock);
+	clk_table[EXYNOS_DOUT_AUD_BUS] = audss_clk_register_divider(&pdev->dev,
+			"dout_aud_bus", "dout_srp", 0,
+			reg_base + ASS_CLK_DIV, 4, 4);
 
-	clk_table[EXYNOS_DOUT_I2S] = clk_register_divider(NULL, "dout_i2s",
-				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 4, 0,
-				&lock);
+	clk_table[EXYNOS_DOUT_I2S] = audss_clk_register_divider(&pdev->dev,
+			"dout_i2s", "mout_i2s", 0,
+			reg_base + ASS_CLK_DIV, 8, 4);
 
-	clk_table[EXYNOS_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
-				"dout_srp", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 0, 0, &lock);
+	clk_table[EXYNOS_SRP_CLK] = audss_clk_register_gate(&pdev->dev,
+			"srp_clk", "dout_srp", CLK_SET_RATE_PARENT,
+			reg_base + ASS_CLK_GATE, 0);
 
-	clk_table[EXYNOS_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
-				"dout_aud_bus", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 2, 0, &lock);
+	clk_table[EXYNOS_I2S_BUS] = audss_clk_register_gate(&pdev->dev,
+			"i2s_bus", "dout_aud_bus", CLK_SET_RATE_PARENT,
+			reg_base + ASS_CLK_GATE, 2);
 
-	clk_table[EXYNOS_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
-				"dout_i2s", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 3, 0, &lock);
+	clk_table[EXYNOS_SCLK_I2S] = audss_clk_register_gate(&pdev->dev,
+			"sclk_i2s", "dout_i2s", CLK_SET_RATE_PARENT,
+			reg_base + ASS_CLK_GATE, 3);
 
-	clk_table[EXYNOS_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
-				 "sclk_pcm", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 4, 0, &lock);
+	clk_table[EXYNOS_PCM_BUS] = audss_clk_register_gate(&pdev->dev,
+			"pcm_bus", "sclk_pcm", CLK_SET_RATE_PARENT,
+			reg_base + ASS_CLK_GATE, 4);
 
 	sclk_pcm_in = devm_clk_get(&pdev->dev, "sclk_pcm_in");
 	if (!IS_ERR(sclk_pcm_in))
 		sclk_pcm_p = __clk_get_name(sclk_pcm_in);
-	clk_table[EXYNOS_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
-				sclk_pcm_p, CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 5, 0, &lock);
+	clk_table[EXYNOS_SCLK_PCM] = audss_clk_register_gate(&pdev->dev,
+			"sclk_pcm", sclk_pcm_p, CLK_SET_RATE_PARENT,
+			reg_base + ASS_CLK_GATE, 5);
 
 	if (variant == TYPE_EXYNOS5420) {
-		clk_table[EXYNOS_ADMA] = clk_register_gate(NULL, "adma",
-				"dout_srp", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 9, 0, &lock);
+		clk_table[EXYNOS_ADMA] = audss_clk_register_gate(&pdev->dev,
+				"adma", "dout_srp", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 9);
 	}
 
 	for (i = 0; i < clk_data.clk_num; i++) {
@@ -198,6 +459,9 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	return 0;
 
 unregister:
+	if (!IS_ERR(epll))
+		clk_unprepare(epll);
+
 	for (i = 0; i < clk_data.clk_num; i++) {
 		if (!IS_ERR(clk_table[i]))
 			clk_unregister(clk_table[i]);
@@ -214,6 +478,9 @@ static int exynos_audss_clk_remove(struct platform_device *pdev)
 	unregister_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
+	if (!IS_ERR(epll))
+		clk_unprepare(epll);
+
 	of_clk_del_provider(pdev->dev.of_node);
 
 	for (i = 0; i < clk_data.clk_num; i++) {
-- 
1.9.1

[PATCH v4 2/3] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated

[Krzysztof Kozlowski]

The audio subsystem on Exynos 5420 has separate clocks and GPIO. To
operate properly on GPIOs the main block clock 'mau_epll' must be
enabled.

This was observed on Peach Pi/Pit and Arndale Octa (after enabling i2s0)
after introducing runtime PM to pl330 DMA driver. After that commit the
'mau_epll' was gated, because the "amba" clock was disabled and there
were no more users of mau_epll.

The system hang just before probing i2s0 because
samsung_pinmux_setup() tried to access memory from audss block which was
gated.

Add a clock property to the pinctrl driver and enable the clock during
GPIO setup. During normal GPIO operations (set, get, set_direction) the
clock is not enabled.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 .../bindings/pinctrl/samsung-pinctrl.txt           |   6 ++
 drivers/pinctrl/samsung/pinctrl-samsung.c          | 111 +++++++++++++++++++--
 drivers/pinctrl/samsung/pinctrl-samsung.h          |   2 +
 3 files changed, 112 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 8425838a6dff..eb121daabe9d 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -93,6 +93,12 @@ Required Properties:
   pin configuration should use the bindings listed in the "pinctrl-bindings.txt"
   file.
 
+Optional Properties:
+- clocks: Optional clock needed to access the block. Will be enabled/disabled
+  during GPIO configuration, suspend and resume but not during GPIO operations
+  (like set, get, set direction).
+- clock-names: Must be "block".
+
 External GPIO and Wakeup Interrupts:
 
 The controller supports two types of external interrupts over gpio. The first
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index ec580af35856..85e487fe43ec 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/irqdomain.h>
@@ -55,6 +56,32 @@ static LIST_HEAD(drvdata_list);
 
 static unsigned int pin_base;
 
+static int pctl_clk_enable(struct pinctrl_dev *pctldev)
+{
+	struct samsung_pinctrl_drv_data *drvdata;
+	int ret;
+
+	drvdata = pinctrl_dev_get_drvdata(pctldev);
+	if (!drvdata->clk)
+		return 0;
+
+	ret = clk_enable(drvdata->clk);
+	if (ret)
+		dev_err(pctldev->dev, "failed to enable clock: %d\n", ret);
+
+	return ret;
+}
+
+static void pctl_clk_disable(struct pinctrl_dev *pctldev)
+{
+	struct samsung_pinctrl_drv_data *drvdata;
+
+	drvdata = pinctrl_dev_get_drvdata(pctldev);
+
+	/* clk/core.c does the check if clk != NULL */
+	clk_disable(drvdata->clk);
+}
+
 static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
 {
 	return container_of(gc, struct samsung_pin_bank, gpio_chip);
@@ -374,7 +401,9 @@ static void samsung_pinmux_setup(struct pinctrl_dev *pctldev, unsigned selector,
 	const struct samsung_pmx_func *func;
 	const struct samsung_pin_group *grp;
 
+	pctl_clk_enable(pctldev);
 	drvdata = pinctrl_dev_get_drvdata(pctldev);
+
 	func = &drvdata->pmx_functions[selector];
 	grp = &drvdata->pin_groups[group];
 
@@ -398,6 +427,8 @@ static void samsung_pinmux_setup(struct pinctrl_dev *pctldev, unsigned selector,
 	writel(data, reg + type->reg_offset[PINCFG_TYPE_FUNC]);
 
 	spin_unlock_irqrestore(&bank->slock, flags);
+
+	pctl_clk_disable(pctldev);
 }
 
 /* enable a specified pinmux by writing to registers */
@@ -469,20 +500,37 @@ static int samsung_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 {
 	int i, ret;
 
+	ret = pctl_clk_enable(pctldev);
+	if (ret)
+		goto out;
+
 	for (i = 0; i < num_configs; i++) {
 		ret = samsung_pinconf_rw(pctldev, pin, &configs[i], true);
 		if (ret < 0)
-			return ret;
+			goto out;
 	} /* for each config */
 
-	return 0;
+out:
+	pctl_clk_disable(pctldev);
+
+	return ret;
 }
 
 /* get the pin config settings for a specified pin */
 static int samsung_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
 					unsigned long *config)
 {
-	return samsung_pinconf_rw(pctldev, pin, config, false);
+	int ret;
+
+	ret = pctl_clk_enable(pctldev);
+	if (ret)
+		return ret;
+
+	ret = samsung_pinconf_rw(pctldev, pin, config, false);
+
+	pctl_clk_disable(pctldev);
+
+	return ret;
 }
 
 /* set the pin config settings for a specified pin group */
@@ -1057,10 +1105,23 @@ static int samsung_pinctrl_probe(struct platform_device *pdev)
 	}
 	drvdata->dev = dev;
 
+	drvdata->clk = clk_get(&pdev->dev, "block");
+	if (!IS_ERR(drvdata->clk)) {
+		ret = clk_prepare_enable(drvdata->clk);
+		if (ret) {
+			dev_err(dev, "failed to enable clk: %d\n", ret);
+			return ret;
+		}
+	} else {
+		drvdata->clk = NULL;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	drvdata->virt_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(drvdata->virt_base))
-		return PTR_ERR(drvdata->virt_base);
+	if (IS_ERR(drvdata->virt_base)) {
+		ret = PTR_ERR(drvdata->virt_base);
+		goto err;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res)
@@ -1068,12 +1129,12 @@ static int samsung_pinctrl_probe(struct platform_device *pdev)
 
 	ret = samsung_gpiolib_register(pdev, drvdata);
 	if (ret)
-		return ret;
+		goto err;
 
 	ret = samsung_pinctrl_register(pdev, drvdata);
 	if (ret) {
 		samsung_gpiolib_unregister(pdev, drvdata);
-		return ret;
+		goto err;
 	}
 
 	if (ctrl->eint_gpio_init)
@@ -1085,6 +1146,23 @@ static int samsung_pinctrl_probe(struct platform_device *pdev)
 
 	/* Add to the global list */
 	list_add_tail(&drvdata->node, &drvdata_list);
+	clk_disable(drvdata->clk); /* Leave prepared */
+
+	return 0;
+
+err:
+	if (drvdata->clk)
+		clk_disable_unprepare(drvdata->clk);
+
+	return ret;
+}
+
+static int samsung_pinctrl_remove(struct platform_device *pdev)
+{
+	struct samsung_pinctrl_drv_data *drvdata = platform_get_drvdata(pdev);
+
+	if (drvdata->clk)
+		clk_unprepare(drvdata->clk);
 
 	return 0;
 }
@@ -1102,6 +1180,13 @@ static void samsung_pinctrl_suspend_dev(
 	void __iomem *virt_base = drvdata->virt_base;
 	int i;
 
+	if (drvdata->clk) {
+		if (clk_enable(drvdata->clk)) {
+			dev_err(drvdata->dev, "failed to enable clock\n");
+			return;
+		}
+	}
+
 	for (i = 0; i < drvdata->nr_banks; i++) {
 		struct samsung_pin_bank *bank = &drvdata->pin_banks[i];
 		void __iomem *reg = virt_base + bank->pctl_offset;
@@ -1133,6 +1218,8 @@ static void samsung_pinctrl_suspend_dev(
 
 	if (drvdata->suspend)
 		drvdata->suspend(drvdata);
+
+	clk_disable(drvdata->clk);
 }
 
 /**
@@ -1148,6 +1235,13 @@ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata)
 	void __iomem *virt_base = drvdata->virt_base;
 	int i;
 
+	if (drvdata->clk) {
+		if (clk_enable(drvdata->clk)) {
+			dev_err(drvdata->dev, "failed to enable clock\n");
+			return;
+		}
+	}
+
 	if (drvdata->resume)
 		drvdata->resume(drvdata);
 
@@ -1181,6 +1275,8 @@ static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata)
 			if (widths[type])
 				writel(bank->pm_save[type], reg + offs[type]);
 	}
+
+	clk_disable(drvdata->clk);
 }
 
 /**
@@ -1264,6 +1360,7 @@ MODULE_DEVICE_TABLE(of, samsung_pinctrl_dt_match);
 
 static struct platform_driver samsung_pinctrl_driver = {
 	.probe		= samsung_pinctrl_probe,
+	.remove		= samsung_pinctrl_remove,
 	.driver = {
 		.name	= "samsung-pinctrl",
 		.of_match_table = samsung_pinctrl_dt_match,
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
index 1b8c0139d604..666cb23eb9f2 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.h
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
@@ -201,6 +201,7 @@ struct samsung_pin_ctrl {
  * struct samsung_pinctrl_drv_data: wrapper for holding driver data together.
  * @node: global list node
  * @virt_base: register base address of the controller.
+ * @clk: Optional clock to enable/disable during setup. May be NULL.
  * @dev: device instance representing the controller.
  * @irq: interrpt number used by the controller to notify gpio interrupts.
  * @ctrl: pin controller instance managed by the driver.
@@ -218,6 +219,7 @@ struct samsung_pinctrl_drv_data {
 	void __iomem			*virt_base;
 	struct device			*dev;
 	int				irq;
+	struct clk			*clk;
 
 	struct pinctrl_desc		pctl;
 	struct pinctrl_dev		*pctl_dev;
-- 
1.9.1

[PATCH v4 3/3] ARM: dts: exynos5420: Add clock for audss pinctrl (fixing GPIO setup failure)

[Krzysztof Kozlowski]

The pinctrl for audio subsystem needs 'mau_epll' clock to be enabled in
order to properly access memory during GPIO setup.

After introducing runtime PM to pl330 DMA driver the 'mau_epll' was
gated, because the "amba" clock was disabled and there were no more
users of mau_epll.

This lead to system hang just before probing i2s0 because
samsung_pinmux_setup() tried to access memory from audss block which was
gated.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/exynos5420-pinctrl.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-pinctrl.dtsi b/arch/arm/boot/dts/exynos5420-pinctrl.dtsi
index ba686e40eac7..c0ca0da36ade 100644
--- a/arch/arm/boot/dts/exynos5420-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos5420-pinctrl.dtsi
@@ -696,6 +696,9 @@
 	};
 
 	pinctrl@03860000 {
+		clocks = <&clock CLK_MAU_EPLL>;
+		clock-names = "block";
+
 		gpz: gpz {
 			gpio-controller;
 			#gpio-cells = <2>;
-- 
1.9.1

Re: [PATCH v4 1/3] clk: samsung: Fix clock disable failure because domain being gated

[Sylwester Nawrocki]

Javier,

On 05/12/14 12:00, Krzysztof Kozlowski wrote:
> Audio subsystem clocks are located in separate block. If clock for this
> block (from main clock domain) 'mau_epll' is gated then any read or
> write to audss registers will block.
> 
> This was observed on Exynos 5420 platforms (Arndale Octa and Peach
> Pi/Pit) after introducing runtime PM to pl330 DMA driver. After that
> commit the 'mau_epll' was gated, because the "amba" clock was disabled
> and there were no more users of mau_epll. The system hang on disabling
> unused clocks from audss block.
> 
> Unfortunately the 'mau_epll' clock is not parent of some of audss clocks.
> 
> Whenever system wants to operate on audss clocks it has to enable epll
> clock. The solution reuses common clk-gate/divider/mux code and duplicates
> clk_register_*() functions.
> 
> Additionally this patch fixes memory leak of clock gate/divider/mux
> structures. The leak exists in generic clk_register_*() functions. Patch
> replaces them with custom code with managed allocation.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reported-by: Kevin Hilman <khilman@kernel.org>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Can you confirm sound works with this patch on exynos5420 ? Or does
your Tested-by refer only to successful booting ?

--
Thanks,
Sylwester

Re: [PATCH v4 1/3] clk: samsung: Fix clock disable failure because domain being gated

[Javier Martinez Canillas]

Hello Sylwester,

On 12/05/2014 12:22 PM, Sylwester Nawrocki wrote:
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
> Can you confirm sound works with this patch on exynos5420 ? Or does
> your Tested-by refer only to successful booting ?
> 

I tested latest linux-next + "clk: samsung: Fix double add of syscore
ops after driver rebind" [0] + this series and I confirm that sound is
also working on my Exynos5420 Peach Pit Chromebook.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/11/26/290

Re: [PATCH v4 1/3] clk: samsung: Fix clock disable failure because domain being gated

[Sylwester Nawrocki]

Hi Krzysztof,

On 05/12/14 12:00, Krzysztof Kozlowski wrote:
> Audio subsystem clocks are located in separate block. If clock for this
> block (from main clock domain) 'mau_epll' is gated then any read or
> write to audss registers will block.
> 
> This was observed on Exynos 5420 platforms (Arndale Octa and Peach
> Pi/Pit) after introducing runtime PM to pl330 DMA driver. After that
> commit the 'mau_epll' was gated, because the "amba" clock was disabled
> and there were no more users of mau_epll. The system hang on disabling
> unused clocks from audss block.
> 
> Unfortunately the 'mau_epll' clock is not parent of some of audss clocks.
> 
> Whenever system wants to operate on audss clocks it has to enable epll
> clock. The solution reuses common clk-gate/divider/mux code and duplicates
> clk_register_*() functions.
> 
> Additionally this patch fixes memory leak of clock gate/divider/mux
> structures. The leak exists in generic clk_register_*() functions. Patch
> replaces them with custom code with managed allocation.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reported-by: Kevin Hilman <khilman@kernel.org>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Instead of this big patch perhaps for v3.19 we could just add
a clk_enable() call in exynos5420 clock controller driver, to make
sure the required clock is always enabled, as it was before you have
added runtime PM support to the PL330 DMA controller driver ?

It sounds like a workaround but wouldn't be as intrusive as this
patch.

--
Regards,
Sylwester

Re: [PATCH v4 1/3] clk: samsung: Fix clock disable failure because domain being gated

[Sylwester Nawrocki]

On 05/12/14 13:34, Javier Martinez Canillas wrote:
> Hello Sylwester,
> 
> On 12/05/2014 12:22 PM, Sylwester Nawrocki wrote:
>>> >> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> > 
>> > Can you confirm sound works with this patch on exynos5420 ? Or does
>> > your Tested-by refer only to successful booting ?
>> > 
> I tested latest linux-next + "clk: samsung: Fix double add of syscore
> ops after driver rebind" [0] + this series and I confirm that sound is
> also working on my Exynos5420 Peach Pit Chromebook.

OK, thanks a lot for checking it again.

--
Regards,
Sylwester

Re: [PATCH v4 0/3] Fix Arndale Octa/Peach Pi boot on Audio subsystem clocks

[Kevin Hilman]

Hi Krzysztof,

Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:

> Changes since v3
> ================
> 1. Patch 1/3: Fix issues pointed by Sylwester.
> 2. Add Javier's tested-by [1]

I tested this series on top of next-20141210 (exynos_defconfig and
multi_v7_defconfig) on exynos5800-peach-pi and exynos5420-arndale-octa
it fixes the boot hangs I was seeing with linux-next on those platforms.

Feel free to add:

Tested-by: Kevin Hilman <khilman@linaro.org>

Kevin

Re: [PATCH v4 2/3] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated

[Linus Walleij]

On Fri, Dec 5, 2014 at 12:00 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:

> The audio subsystem on Exynos 5420 has separate clocks and GPIO. To
> operate properly on GPIOs the main block clock 'mau_epll' must be
> enabled.
>
> This was observed on Peach Pi/Pit and Arndale Octa (after enabling i2s0)
> after introducing runtime PM to pl330 DMA driver. After that commit the
> 'mau_epll' was gated, because the "amba" clock was disabled and there
> were no more users of mau_epll.
>
> The system hang just before probing i2s0 because
> samsung_pinmux_setup() tried to access memory from audss block which was
> gated.
>
> Add a clock property to the pinctrl driver and enable the clock during
> GPIO setup. During normal GPIO operations (set, get, set_direction) the
> clock is not enabled.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Tomasz, is this OK and should I apply it for fixes or next?

Yours,
Linus Walleij

Re: [PATCH v4 2/3] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated

[Krzysztof Kozlowski]

On pon, 2015-01-05 at 15:05 +0100, Linus Walleij wrote:
> On Fri, Dec 5, 2014 at 12:00 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> 
> > The audio subsystem on Exynos 5420 has separate clocks and GPIO. To
> > operate properly on GPIOs the main block clock 'mau_epll' must be
> > enabled.
> >
> > This was observed on Peach Pi/Pit and Arndale Octa (after enabling i2s0)
> > after introducing runtime PM to pl330 DMA driver. After that commit the
> > 'mau_epll' was gated, because the "amba" clock was disabled and there
> > were no more users of mau_epll.
> >
> > The system hang just before probing i2s0 because
> > samsung_pinmux_setup() tried to access memory from audss block which was
> > gated.
> >
> > Add a clock property to the pinctrl driver and enable the clock during
> > GPIO setup. During normal GPIO operations (set, get, set_direction) the
> > clock is not enabled.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
> Tomasz, is this OK and should I apply it for fixes or next?

Ha! That is a good question. The issue is fixed for now by the
workaround (merged) [1].

The workaround just enables the clock for entire runtime of Exynos
5420-like device. This has some energy impact - around 1.5% in idle [2].

So actually this is question which way we want to solve it:
1. Stick with the workaround (small piece of code, energy impact when
not used).
2. Full clock enable on use (big chunk of code, no energy impact when
not used).

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f1e9203e2366164b832d8a6ce10134de8c575178
[2] http://www.spinics.net/lists/linux-samsung-soc/msg39827.html

Best regards,
Krzysztof