From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
Mike Turquette <mturquette@linaro.org>,
Kukjin Kim <kgene@kernel.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Thomas Abraham <thomas.abraham@linaro.org>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
Vivek Gautam <gautam.vivek@samsung.com>,
Kevin Hilman <khilman@kernel.org>,
Russell King <linux@arm.linux.org.uk>,
Kyungmin Park <kyungmin.park@samsung.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
Date: Mon, 01 Dec 2014 09:37:23 +0100 [thread overview]
Message-ID: <1417423043.4055.14.camel@AMDC1943> (raw)
In-Reply-To: <CA+Ln22Ejo51VPWf6DmuRvV_fZr_H=-BXAshSPSOokNE1bz=-QA@mail.gmail.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.
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
WARNING: multiple messages have this Message-ID (diff)
From: k.kozlowski@samsung.com (Krzysztof Kozlowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
Date: Mon, 01 Dec 2014 09:37:23 +0100 [thread overview]
Message-ID: <1417423043.4055.14.camel@AMDC1943> (raw)
In-Reply-To: <CA+Ln22Ejo51VPWf6DmuRvV_fZr_H=-BXAshSPSOokNE1bz=-QA@mail.gmail.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.
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
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
Mike Turquette <mturquette@linaro.org>,
Kukjin Kim <kgene@kernel.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Thomas Abraham <thomas.abraham@linaro.org>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
Vivek Gautam <gautam.vivek@samsung.com>,
Kevin Hilman <khilman@kernel.org>,
Russell King <linux@arm.linux.org.uk>,
Kyungmin Park <kyungmin.park@samsung.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
Date: Mon, 01 Dec 2014 09:37:23 +0100 [thread overview]
Message-ID: <1417423043.4055.14.camel@AMDC1943> (raw)
In-Reply-To: <CA+Ln22Ejo51VPWf6DmuRvV_fZr_H=-BXAshSPSOokNE1bz=-QA@mail.gmail.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.
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
next prev parent reply other threads:[~2014-12-01 8:37 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-26 14:24 [PATCH v2 0/5] Fix Arndale Octa/Peach Pi boot on Audio subsystem clocks Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 14:24 ` [PATCH v2 1/5] clk: samsung: Fix double add of syscore ops after driver rebind Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-12-03 12:10 ` Sylwester Nawrocki
2014-12-03 12:10 ` Sylwester Nawrocki
2014-11-26 14:24 ` [PATCH v2 2/5] clk: samsung: Fix clock disable failure because domain being gated Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-12-03 14:12 ` Sylwester Nawrocki
2014-12-03 14:12 ` Sylwester Nawrocki
2014-12-04 9:46 ` Krzysztof Kozlowski
2014-12-04 9:46 ` Krzysztof Kozlowski
2014-11-26 14:24 ` [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock " Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-28 14:04 ` Linus Walleij
2014-11-28 14:04 ` Linus Walleij
2014-11-28 14:08 ` Krzysztof Kozlowski
2014-11-28 14:08 ` Krzysztof Kozlowski
2014-11-30 12:19 ` Tomasz Figa
2014-11-30 12:19 ` Tomasz Figa
2014-11-30 12:19 ` Tomasz Figa
2014-12-01 8:37 ` Krzysztof Kozlowski [this message]
2014-12-01 8:37 ` Krzysztof Kozlowski
2014-12-01 8:37 ` Krzysztof Kozlowski
2014-12-01 14:34 ` Tomasz Figa
2014-12-01 14:34 ` Tomasz Figa
[not found] ` <CA+Ln22H_a9CWxrYO_i9Esm6keXA8XLLYmE1Vg-8ynamJ1sAwpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-12-01 14:52 ` Krzysztof Kozlowski
2014-12-01 14:52 ` Krzysztof Kozlowski
2014-12-01 14:52 ` Krzysztof Kozlowski
2015-01-12 14:11 ` Krzysztof Kozlowski
2015-01-12 14:11 ` Krzysztof Kozlowski
2014-11-26 14:24 ` [PATCH v2 4/5] ARM: dts: exynos5420: Add clock for audss pinctrl Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 14:24 ` [PATCH v2 5/5] clk: samsung: Fix memory leak of clock gate/divider/mux structures Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 14:24 ` Krzysztof Kozlowski
2014-11-26 16:31 ` [PATCH v2 0/5] Fix Arndale Octa/Peach Pi boot on Audio subsystem clocks Javier Martinez Canillas
2014-11-26 16:31 ` Javier Martinez Canillas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1417423043.4055.14.camel@AMDC1943 \
--to=k.kozlowski@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=gautam.vivek@samsung.com \
--cc=javier.martinez@collabora.co.uk \
--cc=kgene@kernel.org \
--cc=khilman@kernel.org \
--cc=kyungmin.park@samsung.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=m.szyprowski@samsung.com \
--cc=mturquette@linaro.org \
--cc=s.nawrocki@samsung.com \
--cc=thomas.abraham@linaro.org \
--cc=tomasz.figa@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.