| Message ID | 1417011857-10419-4-git-send-email-k.kozlowski@samsung.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Nov 26, 2014 at 3:24 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> Waiting for Tomasz to review this. Can this patch be applied in separation from the others? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: > On Wed, Nov 26, 2014 at 3:24 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> > > Waiting for Tomasz to review this. > > Can this patch be applied in separation from the others? Yes, it can be picked independently. The commit message is somehow misleading because issue is actually fixed by enabling this in DTS. So the next patch (4/5: ARM: dts: exynos5420: Add clock for audss pinctrl) actually fixes the issue on Arndale Octa board from pinctrl perspective. Unfortunately I spot that mistake (in commit msg) later... Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof, 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: >> On Wed, Nov 26, 2014 at 3:24 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. Could you make sure that possibility of gating this clock is worth the effort of adding gating code to all affected drivers? If there is no significant change in power consumption maybe it could be simply keep running all the time? Also isn't a similar problem happening due to power domains? I believe the whole maudio block is located in a separate power domain but somehow it doesn't get turned off? Could you investigate the above, please? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: > Hi Krzysztof, > > 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: > >> On Wed, Nov 26, 2014 at 3:24 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. > > Could you make sure that possibility of gating this clock is worth the > effort of adding gating code to all affected drivers? If there is no > significant change in power consumption maybe it could be simply keep > running all the time? I had an impression that last time you disliked such idea: http://www.spinics.net/lists/arm-kernel/msg338127.html That's why I developed these patches. Because keeping a clock always on, even when it is unused, is undesirable. Anyway, I did some simple measurements (after booting Arndale Octa to /bin/sh, idle): - with mau_epll gated: ~523 mA - with mau_epll always on: ~531 mA Keeping it on increases energy usage by 1.5% in idle (with measurement uncertainty ~0.4%). > Also isn't a similar problem happening due to power domains? I believe > the whole maudio block is located in a separate power domain but > somehow it doesn't get turned off? There is Maudio power domain... but I think it is not related here. Pinctrl driver does not have runtime PM and is not attached to a domain. I thought about other solution to this problem (with utilization of power domains): - add runtime PM to pinctrl and audss clocks, - attach pinctrl and audss clocks to maudio power domain, - enable the clock when power domain is turned on. However almost the same changes had to be added to pinctrl and audss clocks drivers (replace clock_enable() with pm_runtime_get_sync()). Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2014-12-01 17:37 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > > On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: >> Hi Krzysztof, >> >> 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: >> > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: >> >> On Wed, Nov 26, 2014 at 3:24 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. >> >> Could you make sure that possibility of gating this clock is worth the >> effort of adding gating code to all affected drivers? If there is no >> significant change in power consumption maybe it could be simply keep >> running all the time? > > I had an impression that last time you disliked such idea: > http://www.spinics.net/lists/arm-kernel/msg338127.html > That's why I developed these patches. Because keeping a clock always on, > even when it is unused, is undesirable. I was mostly concerned about the particular solution implemented by that patch that kept the clocks enabled on all platforms, not only the affected one. However... > > Anyway, I did some simple measurements (after booting Arndale Octa > to /bin/sh, idle): > - with mau_epll gated: ~523 mA > - with mau_epll always on: ~531 mA > > Keeping it on increases energy usage by 1.5% in idle (with measurement > uncertainty ~0.4%). > ...this definitely shows that the effort isn't worthless, which means that your patch goes in the right direction. > >> Also isn't a similar problem happening due to power domains? I believe >> the whole maudio block is located in a separate power domain but >> somehow it doesn't get turned off? > > There is Maudio power domain... but I think it is not related here. Could you somehow check what happens when the maudio power domain is turned off and maudio pin controller is accessed? Could you also check the difference in power consumption with this domain powered on and off? > Pinctrl driver does not have runtime PM and is not attached to a domain. > I thought about other solution to this problem (with utilization of > power domains): > - add runtime PM to pinctrl and audss clocks, > - attach pinctrl and audss clocks to maudio power domain, > - enable the clock when power domain is turned on. > However almost the same changes had to be added to pinctrl and audss > clocks drivers (replace clock_enable() with pm_runtime_get_sync()). Well, if the dependency is there due to hardware design, I think it needs to be reflected in the drivers as well. Few other issues that came to my mind: - Your previous patch added clock control only to pinctrl operations. Shouldn't GPIO operations be included as well? - If power domain dependency is there too, what happens with GPIO pins if the domain is powered off? If they lose their states, wouldn't it necessary to keep the domain powered on whenever there is some GPIO pin requested (which usually = in use)? (I'd assume that special functions shouldn't take a reference on runtime PM, because they are related to IP blocks in the same PM domain, which will implicitly keep the domain powered on.) - Doesn't the clock controller also lose its state whenever the power domain is powered off? I guess that would be similar to ISP clock controller, issues of which are still not resolved in mainline. Sylwester could tell you more about this, I guess. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On pon, 2014-12-01 at 23:34 +0900, Tomasz Figa wrote: > 2014-12-01 17:37 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > > > > On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: > >> Hi Krzysztof, > >> > >> 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > >> > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: > >> >> On Wed, Nov 26, 2014 at 3:24 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. > >> > >> Could you make sure that possibility of gating this clock is worth the > >> effort of adding gating code to all affected drivers? If there is no > >> significant change in power consumption maybe it could be simply keep > >> running all the time? > > > > I had an impression that last time you disliked such idea: > > http://www.spinics.net/lists/arm-kernel/msg338127.html > > That's why I developed these patches. Because keeping a clock always on, > > even when it is unused, is undesirable. > > I was mostly concerned about the particular solution implemented by > that patch that kept the clocks enabled on all platforms, not only the > affected one. However... > > > > > Anyway, I did some simple measurements (after booting Arndale Octa > > to /bin/sh, idle): > > - with mau_epll gated: ~523 mA > > - with mau_epll always on: ~531 mA > > > > Keeping it on increases energy usage by 1.5% in idle (with measurement > > uncertainty ~0.4%). > > > > ...this definitely shows that the effort isn't worthless, which means > that your patch goes in the right direction. Great! > > > > >> Also isn't a similar problem happening due to power domains? I believe > >> the whole maudio block is located in a separate power domain but > >> somehow it doesn't get turned off? > > > > There is Maudio power domain... but I think it is not related here. > > Could you somehow check what happens when the maudio power domain is > turned off and maudio pin controller is accessed? Could you also check > the difference in power consumption with this domain powered on and > off? I can try. I'll let you know the results. > > > Pinctrl driver does not have runtime PM and is not attached to a domain. > > I thought about other solution to this problem (with utilization of > > power domains): > > - add runtime PM to pinctrl and audss clocks, > > - attach pinctrl and audss clocks to maudio power domain, > > - enable the clock when power domain is turned on. > > However almost the same changes had to be added to pinctrl and audss > > clocks drivers (replace clock_enable() with pm_runtime_get_sync()). > > Well, if the dependency is there due to hardware design, I think it > needs to be reflected in the drivers as well. > > Few other issues that came to my mind: > > - Your previous patch added clock control only to pinctrl operations. > Shouldn't GPIO operations be included as well? Yeah... but does it make any sense? For example imagine GPIO is configured as input. Enabling the clock won't change anything because already that clock should be enabled by peripheral which writes to such GPIO. > - If power domain dependency is there too, what happens with GPIO > pins if the domain is powered off? If they lose their states, wouldn't > it necessary to keep the domain powered on whenever there is some GPIO > pin requested (which usually = in use)? (I'd assume that special > functions shouldn't take a reference on runtime PM, because they are > related to IP blocks in the same PM domain, which will implicitly keep > the domain powered on.) What you're saying makes sense but that look like job for sound soc driver? Anyway the domain is not present in DTS so by default it is turned on. > - Doesn't the clock controller also lose its state whenever the power > domain is powered off? I guess that would be similar to ISP clock > controller, issues of which are still not resolved in mainline. > Sylwester could tell you more about this, I guess. Once again - the domain. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On pon, 2014-12-01 at 23:34 +0900, Tomasz Figa wrote: > 2014-12-01 17:37 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > > > > On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: > >> Hi Krzysztof, > >> (...) > > > >> Also isn't a similar problem happening due to power domains? I believe > >> the whole maudio block is located in a separate power domain but > >> somehow it doesn't get turned off? > > > > There is Maudio power domain... but I think it is not related here. > > Could you somehow check what happens when the maudio power domain is > turned off and maudio pin controller is accessed? Could you also check > the difference in power consumption with this domain powered on and > off? Hi Tomasz, Some time ago you asked for some details of Exynos5420 Maudio domain behavior. Here it goes: 1. mau domain ON, EPL: OFF: boot hang (this was behavior after adding runtime PM to pl330 driver) 2. maud domain OFF, EPLL OFF: boot hang 3. maud domain ON, EPLL ON: works (current linux-next and mainline) 4. maud domain OFF, EPLL ON: works, almost the same energy consumption as in (3) (diff: 0.1% so it is smaller than measuring accuracy). However an unspecified imprecise abort shows up in (4): [ 11.911628] Freeing unused kernel memory: 328K (c0674000 - c06c6000) [ 12.122695] usb 5-1.4: new high-speed USB device number 3 using exynos-ehci [ 12.190011] Unhandled fault: imprecise external abort (0x1406) at 0x00000000 [ 12.219062] usb 5-1.4: New USB device found, idVendor=0b95, idProduct=772a [ 12.224548] usb 5-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 12.231794] usb 5-1.4: Product: AX88772 [ 12.235681] usb 5-1.4: Manufacturer: ASIX Elec. Corp. [ 12.240707] usb 5-1.4: SerialNumber: 000001 [ 12.248835] asix 5-1.4:1.0 (unnamed net_device) (uninitialized): invalid hw address, using random root@(none):/# Best regards, Krzysztof > > > Pinctrl driver does not have runtime PM and is not attached to a domain. > > I thought about other solution to this problem (with utilization of > > power domains): > > - add runtime PM to pinctrl and audss clocks, > > - attach pinctrl and audss clocks to maudio power domain, > > - enable the clock when power domain is turned on. > > However almost the same changes had to be added to pinctrl and audss > > clocks drivers (replace clock_enable() with pm_runtime_get_sync()). > > Well, if the dependency is there due to hardware design, I think it > needs to be reflected in the drivers as well. > > Few other issues that came to my mind: > > - Your previous patch added clock control only to pinctrl operations. > Shouldn't GPIO operations be included as well? > > - If power domain dependency is there too, what happens with GPIO > pins if the domain is powered off? If they lose their states, wouldn't > it necessary to keep the domain powered on whenever there is some GPIO > pin requested (which usually = in use)? (I'd assume that special > functions shouldn't take a reference on runtime PM, because they are > related to IP blocks in the same PM domain, which will implicitly keep > the domain powered on.) > > - Doesn't the clock controller also lose its state whenever the power > domain is powered off? I guess that would be similar to ISP clock > controller, issues of which are still not resolved in mainline. > Sylwester could tell you more about this, I guess. > > Best regards, > Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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..96419aba7650 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,22 @@ 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); + + clk_disable_unprepare(drvdata->clk); return 0; } @@ -1102,6 +1179,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 +1217,8 @@ static void samsung_pinctrl_suspend_dev( if (drvdata->suspend) drvdata->suspend(drvdata); + + clk_disable(drvdata->clk); } /** @@ -1148,6 +1234,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 +1274,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 +1359,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;
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> --- .../bindings/pinctrl/samsung-pinctrl.txt | 6 ++ drivers/pinctrl/samsung/pinctrl-samsung.c | 110 +++++++++++++++++++-- drivers/pinctrl/samsung/pinctrl-samsung.h | 2 + 3 files changed, 111 insertions(+), 7 deletions(-)