Re: [PATCH] ARM: dts: Add dts file for odroid XU3 board
Hello Simons,
On 12/01/2014 09:59 PM, Sjoerd Simons wrote:
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+/dts-v1/;
+#include "exynos5800.dtsi"
Looking at exynos5800.dtsi, it derives from exynos5420.dtsi and seems
having differences on clock and mfc. Is it proper to include
exynos5800.dtsi here, instead of exynos5420.dtsi? It's just my curiosity
as I am not in a position to know detailed things on the soc. Only
semiconductor guys could answer this.
+
+/ {
+ model = "Hardkernel Odroid XU3";
+ compatible = "hardkernel,odroid-xu3", "samsung,exynos5800",
"samsung,exynos5";
+
+ memory {
+ reg = <0x2000 0x8000>;
+ };
Start address above should be 0x4000, not 0x2000.
One more thing, having bootargs which specifies 'console=xxx' would be
better.
regards,
heesub
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 03/14] drm/bridge: make bridge registration independent of drm flow
On Sat, Nov 15, 2014 at 3:24 PM, Ajay Kumar wrote:
> Currently, third party bridge drivers(ptn3460) are dependent
> on the corresponding encoder driver init, since bridge driver
> needs a drm_device pointer to finish drm initializations.
> The encoder driver passes the drm_device pointer to the
> bridge driver. Because of this dependency, third party drivers
> like ptn3460 doesn't adhere to the driver model.
>
> In this patch, we reframe the bridge registration framework
> so that bridge initialization is split into 2 steps, and
> bridge registration happens independent of drm flow:
> --Step 1: gather all the bridge settings independent of drm and
> add the bridge onto a global list of bridges.
> --Step 2: when the encoder driver is probed, call drm_bridge_attach
> for the corresponding bridge so that the bridge receives
> drm_device pointer and continues with connector and other
> drm initializations.
>
> The old set of bridge helpers are removed, and a set of new helpers
> are added to accomplish the 2 step initialization.
>
> The bridge devices register themselves onto global list of bridges
> when they get probed by calling "drm_bridge_add".
>
> The parent encoder driver waits till the bridge is available
> in the lookup table(by calling "of_drm_find_bridge") and then
> continues with its initialization.
>
> The encoder driver should also call "drm_bridge_attach" to pass
> on the drm_device to the bridge object.
>
> drm_bridge_attach inturn calls "bridge->funcs->attach" so that
> bridge can continue with drm related initializations.
>
> Signed-off-by: Ajay Kumar
> ---
> drivers/gpu/drm/Makefile |2 +-
> drivers/gpu/drm/bridge/ptn3460.c | 27 +-
> drivers/gpu/drm/drm_bridge.c | 91
>
> drivers/gpu/drm/drm_crtc.c | 65 ---
> drivers/gpu/drm/msm/hdmi/hdmi.c|7 +--
> drivers/gpu/drm/msm/hdmi/hdmi.h|1 +
> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |7 ++-
> drivers/gpu/drm/sti/sti_hda.c | 10 +---
> drivers/gpu/drm/sti/sti_hdmi.c | 10 +---
> include/drm/bridge/ptn3460.h |8 +++
> include/drm/drm_crtc.h | 26 -
> 11 files changed, 136 insertions(+), 118 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_bridge.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 9292a76..00f97a5 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -14,7 +14,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \
> drm_info.o drm_debugfs.o drm_encoder_slave.o \
> drm_trace_points.o drm_global.o drm_prime.o \
> drm_rect.o drm_vma_manager.o drm_flip_work.o \
> - drm_modeset_lock.o
> + drm_modeset_lock.o drm_bridge.o
>
> drm-$(CONFIG_COMPAT) += drm_ioc32.o
> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/bridge/ptn3460.c
> b/drivers/gpu/drm/bridge/ptn3460.c
> index a2ddc8d..4a818c1 100644
> --- a/drivers/gpu/drm/bridge/ptn3460.c
> +++ b/drivers/gpu/drm/bridge/ptn3460.c
> @@ -176,24 +176,11 @@ static void ptn3460_post_disable(struct drm_bridge
> *bridge)
> {
> }
>
> -static void ptn3460_bridge_destroy(struct drm_bridge *bridge)
> -{
> - struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
> -
> - drm_bridge_cleanup(bridge);
> - if (gpio_is_valid(ptn_bridge->gpio_pd_n))
> - gpio_free(ptn_bridge->gpio_pd_n);
> - if (gpio_is_valid(ptn_bridge->gpio_rst_n))
> - gpio_free(ptn_bridge->gpio_rst_n);
> - /* Nothing else to free, we've got devm allocated memory */
> -}
> -
> static struct drm_bridge_funcs ptn3460_bridge_funcs = {
> .pre_enable = ptn3460_pre_enable,
> .enable = ptn3460_enable,
> .disable = ptn3460_disable,
> .post_disable = ptn3460_post_disable,
> - .destroy = ptn3460_bridge_destroy,
> };
>
> static int ptn3460_get_modes(struct drm_connector *connector)
> @@ -314,7 +301,7 @@ int ptn3460_init(struct drm_device *dev, struct
> drm_encoder *encoder,
> }
>
> ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs;
> - ret = drm_bridge_init(dev, &ptn_bridge->bridge);
> + ret = drm_bridge_attach(dev, &ptn_bridge->bridge);
> if (ret) {
> DRM_ERROR("Failed to initialize bridge with drm\n");
> goto err;
> @@ -343,3 +330,15 @@ err:
> return ret;
> }
> EXPORT_SYMBOL(ptn3460_init);
> +
> +void ptn3460_destroy(struct drm_bridge *bridge)
> +{
> + struct ptn3460_bridge *ptn_bridge = bridge->driver_private;
> +
> + if (gpio_is_valid(ptn_bridge->gpio_pd_n))
> + gpio_free(ptn_bridge->gpio_pd_n);
> + if (gpio_is_valid(ptn_bridge->gpio_rst_n))
> + gpio_free(ptn_bridge->gpio_rst_n);
> + /* Nothing
Re: [PATCH] dmaengine: pl330: Set residue in tx_status callback
Hi Vinod/Lars,
On 11/26/14, Padmavathi Venna wrote:
> Fill txstate.residue with the amount of bytes remaining in the current
> transfer if the transfer is not complete. This will be of particular
> use to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
>
> I had taken the code from Dylan Reid patch from the
> below link and modified according to the current dmaengine framework.
> http://comments.gmane.org/gmane.linux.kernel.samsung-soc/23007
>
> Cc: Dylan Reid
> Signed-off-by: Padmavathi Venna
> ---
>
> This patch has been tested for audio playback on exynos5420 peach-pit.
>
> drivers/dma/pl330.c | 67
> +-
> 1 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index b7493d2..db880ae 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2182,11 +2182,74 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
> pm_runtime_put_autosuspend(pch->dmac->ddma.dev);
> }
>
> +static inline int
> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
> +{
> + return ((desc->px.src_addr <= sar) &&
> + (sar <= (desc->px.src_addr + desc->px.bytes)));
> +}
> +
> +static inline int
> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
> +{
> + return ((desc->px.dst_addr <= dar) &&
> + (dar <= (desc->px.dst_addr + desc->px.bytes)));
> +}
> +
> static enum dma_status
> pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>struct dma_tx_state *txstate)
> {
> - return dma_cookie_status(chan, cookie, txstate);
> + dma_addr_t sar, dar;
> + struct dma_pl330_chan *pch = to_pchan(chan);
> + void __iomem *regs = pch->dmac->base;
> + struct pl330_thread *thrd = pch->thread;
> + struct dma_pl330_desc *desc;
> + unsigned int residue = 0;
> + unsigned long flags;
> + bool first = true;
> + dma_cookie_t first_c, current_c;
> + dma_cookie_t used;
> + enum dma_status ret;
> +
> + ret = dma_cookie_status(chan, cookie, txstate);
> + if (ret == DMA_COMPLETE || !txstate)
> + return ret;
> +
> + used = txstate->used;
> +
> + spin_lock_irqsave(&pch->lock, flags);
> + sar = readl(regs + SA(thrd->id));
> + dar = readl(regs + DA(thrd->id));
> +
> + list_for_each_entry(desc, &pch->work_list, node) {
> + if (desc->status == BUSY) {
> + current_c = desc->txd.cookie;
> + if (first) {
> + first_c = desc->txd.cookie;
> + first = false;
> + }
> +
> + if (first_c < current_c)
> + residue += desc->px.bytes;
> + else {
> + if (desc->rqcfg.src_inc &&
> pl330_src_addr_in_desc(desc, sar)) {
> + residue += desc->px.bytes;
> + residue -= sar - desc->px.src_addr;
> + } else if (desc->rqcfg.dst_inc &&
> pl330_dst_addr_in_desc(desc, dar)) {
> + residue += desc->px.bytes;
> + residue -= dar - desc->px.dst_addr;
> + }
> + }
> + } else if (desc->status == PREP)
> + residue += desc->px.bytes;
> +
> + if (desc->txd.cookie == used)
> + break;
> + }
> + spin_unlock_irqrestore(&pch->lock, flags);
> + dma_set_residue(txstate, residue);
> + return ret;
> }
>
> static void pl330_issue_pending(struct dma_chan *chan)
> @@ -2631,7 +2694,7 @@ static int pl330_dma_device_slave_caps(struct dma_chan
> *dchan,
> caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> caps->cmd_pause = false;
> caps->cmd_terminate = true;
> - caps->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> + caps->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
>
> return 0;
> }
Any comment on this patch?
Thanks
Padma
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: samsung: exynos4415: Fix build with PM_SLEEP disabled
On 12/01/2014 06:12 PM, Krzysztof Kozlowski wrote:
> Fix following build errors when PM_SLEEP is disabled (e.g. by disabling
> SUSPEND and HIBERNATION):
>
> drivers/clk/samsung/clk-exynos4415.c: In function ‘exynos4415_cmu_init’:
> drivers/clk/samsung/clk-exynos4415.c:982:2: error: ‘exynos4415_ctx’
> undeclared (first use in this function)
> drivers/clk/samsung/clk-exynos4415.c:982:2: note: each undeclared identifier
> is reported only once for each function it appears in
> drivers/clk/samsung/clk-exynos4415.c: In function ‘exynos4415_cmu_dmc_init’:
> drivers/clk/samsung/clk-exynos4415.c:1123:2: error: ‘exynos4415_dmc_ctx’
> undeclared (first use in this function)
> make[3]: *** [drivers/clk/samsung/clk-exynos4415.o] Error 1
>
> Signed-off-by: Krzysztof Kozlowski
> ---
> drivers/clk/samsung/clk-exynos4415.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4415.c
> b/drivers/clk/samsung/clk-exynos4415.c
> index c7208c7a3add..2123fc251e0f 100644
> --- a/drivers/clk/samsung/clk-exynos4415.c
> +++ b/drivers/clk/samsung/clk-exynos4415.c
> @@ -118,12 +118,13 @@ enum exynos4415_plls {
> nr_plls,
> };
>
> +static struct samsung_clk_provider *exynos4415_ctx;
> +
> /*
> * Support for CMU save/restore across system suspends
> */
> #ifdef CONFIG_PM_SLEEP
> static struct samsung_clk_reg_dump *exynos4415_clk_regs;
> -static struct samsung_clk_provider *exynos4415_ctx;
>
> static unsigned long exynos4415_cmu_clk_regs[] __initdata = {
> SRC_LEFTBUS,
> @@ -1031,9 +1032,10 @@ enum exynos4415_dmc_plls {
> nr_dmc_plls,
> };
>
> +static struct samsung_clk_provider *exynos4415_dmc_ctx;
> +
> #ifdef CONFIG_PM_SLEEP
> static struct samsung_clk_reg_dump *exynos4415_dmc_clk_regs;
> -static struct samsung_clk_provider *exynos4415_dmc_ctx;
>
> static unsigned long exynos4415_cmu_dmc_clk_regs[] __initdata = {
> MPLL_LOCK,
>
Reviewed-by: Chanwoo Choi
Thanks,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/3] PM / Domains: Initial PM clock support for genpd
Ulf Hansson writes: > It's quite common for PM domains to use PM clocks. Typically from SOC > specific code, the per device PM clock list is created and > pm_clk_suspend|resume() are invoked to handle clock gating/ungating. > > A step towards consolidation is to integrate PM clock support into > genpd, which is what this patch does. > > In this initial step, the calls to the pm_clk_suspend|resume() are > handled within genpd, but the per device PM clock list still needs to > be created from SOC specific code. It seems reasonable to have gendp to > handle that as well, but that left to future patches to address. > > It's not every users of genpd that are keen on using PM clocks, thus we > need to provide this a configuration option for genpd. Therefore let's > add flag field in the genpd struct to keep this information and define > a new GENDP_FLAG_PM_CLK bit for it. > > Signed-off-by: Ulf Hansson > Acked-by: Geert Uytterhoeven Acked-by: Kevin Hilman Are you also planning to add a way to enable this option from DT? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT 2/2] arm: dts: disable CCI on exynos420 based arndale-octa
On 1 December 2014 at 10:50, Kevin Hilman wrote: > Abhilash Kesavan writes: > >> The arndale-octa board was giving "imprecise external aborts" during >> boot-up with MCPM enabled. CCI enablement of the boot cluster was found >> to be the cause of these aborts (possibly because the secure f/w was not >> allowing it). Hence, disable CCI for the arndale-octa board. >> >> Signed-off-by: Abhilash Kesavan > > Tested-by: Kevin Hilman > > Tested on top of next-20141128 with exynos_defconfig on my Octa board > and I'm not seeing the imprecise aborts anymore. > > Thanks, > > Kevin > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Tested-by: Tyler Baker Tested on top of mainline/master with exynos_defconfig on my arndale-octa board. One hundred boots attempted with no imprecise aborts. Cheers, -- Tyler Baker Tech Lead, LAVA Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT 2/2] arm: dts: disable CCI on exynos420 based arndale-octa
Abhilash Kesavan writes: > The arndale-octa board was giving "imprecise external aborts" during > boot-up with MCPM enabled. CCI enablement of the boot cluster was found > to be the cause of these aborts (possibly because the secure f/w was not > allowing it). Hence, disable CCI for the arndale-octa board. > > Signed-off-by: Abhilash Kesavan Tested-by: Kevin Hilman Tested on top of next-20141128 with exynos_defconfig on my Octa board and I'm not seeing the imprecise aborts anymore. Thanks, Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drm: exynos: mixer: fix using usleep() in atomic context
On 2014-12-01 16:54, Thierry Reding wrote: On Sun, Nov 30, 2014 at 01:35:25AM +0100, tjak...@math.uni-bielefeld.de wrote: From: Tomasz Stanislawski This patch fixes calling usleep_range() after taking reg_slock using spin_lock_irqsave(). The mdelay() is used instead. Waiting in atomic context is not the best idea in general. Hopefully, waiting occurs only when Video Processor fails to reset correctly. Signed-off-by: Tomasz Stanislawski --- drivers/gpu/drm/exynos/exynos_mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index a41c84e..cc7 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -632,7 +632,7 @@ static void vp_win_reset(struct mixer_context *ctx) /* waiting until VP_SRESET_PROCESSING is 0 */ if (~vp_reg_read(res, VP_SRESET) & VP_SRESET_PROCESSING) break; - usleep_range(1, 12000); + mdelay(10); } WARN(tries == 0, "failed to reset Video Processor\n"); } I can't see a reason why you would need to hold the lock around this code. Perhaps a better way to fix this would be to drop the lock before calling vp_win_reset()? Thierry Hmm, I'm pretty new to spinlocks (only have worked with the usual mutex stuff in userspace), but wouldn't that mean that it is then possible for mixer_win_reset to execute while a (previous) vp_win_reset is still running? With best wishes, Tobias -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] pinctrl: exynos: Add BUS1 pin controller for exynos7
Hi Vivek, Please see my comments below. 2014-11-24 22:02 GMT+09:00 Vivek Gautam : > USB and Power regulator on Exynos7 require gpios available > in BUS1 pin controller block. > So adding the BUS1 pinctrl support. > > Signed-off-by: Naveen Krishna Ch > Signed-off-by: Vivek Gautam > Cc: Linus Walleij > --- > > This patch was part of series: > "[PATCH 00/11] Exynos7: Adding USB 3.0 support" > https://lkml.org/lkml/2014/11/21/247 > > Changes since V1: > - Added support for all pin banks which are part of BUS1 pin controller. > > drivers/pinctrl/samsung/pinctrl-exynos.c | 19 +++ > 1 file changed, 19 insertions(+) I have missed this with previous patch, but DT bindings documentation should list all aliases for all supported compatible strings, because they are required for correct operation. There is a small section about aliases in [1] already, so please add there information about aliases for Exynos7 pin controllers along with their names, e.g. Aliases for controllers compatible with "samsung,exynos7-pinctrl": - pinctrl0: pin controller of ALIVE block, - pinctrl1: pin controller of BUS0 block, [...] - pinctrl8: pin controller of BUS1 block. [1] Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt I guess you can do this in separate patch or respin this one with this added. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drm: exynos: mixer: fix using usleep() in atomic context
On Sun, Nov 30, 2014 at 01:35:25AM +0100, tjak...@math.uni-bielefeld.de wrote: > From: Tomasz Stanislawski > > This patch fixes calling usleep_range() after taking reg_slock > using spin_lock_irqsave(). The mdelay() is used instead. > Waiting in atomic context is not the best idea in general. > Hopefully, waiting occurs only when Video Processor fails > to reset correctly. > > Signed-off-by: Tomasz Stanislawski > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c > b/drivers/gpu/drm/exynos/exynos_mixer.c > index a41c84e..cc7 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -632,7 +632,7 @@ static void vp_win_reset(struct mixer_context *ctx) > /* waiting until VP_SRESET_PROCESSING is 0 */ > if (~vp_reg_read(res, VP_SRESET) & VP_SRESET_PROCESSING) > break; > - usleep_range(1, 12000); > + mdelay(10); > } > WARN(tries == 0, "failed to reset Video Processor\n"); > } I can't see a reason why you would need to hold the lock around this code. Perhaps a better way to fix this would be to drop the lock before calling vp_win_reset()? Thierry pgp5SPPtXnkFq.pgp Description: PGP signature
Re: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
On pon, 2014-12-01 at 23:34 +0900, Tomasz Figa wrote: > 2014-12-01 17:37 GMT+09:00 Krzysztof Kozlowski : > > > > On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: > >> Hi Krzysztof, > >> > >> 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski : > >> > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: > >> >> On Wed, Nov 26, 2014 at 3:24 PM, Krzysztof Kozlowski > >> >> 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-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/maj
Re: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
2014-12-01 17:37 GMT+09:00 Krzysztof Kozlowski : > > On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: >> Hi Krzysztof, >> >> 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski : >> > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: >> >> On Wed, Nov 26, 2014 at 3:24 PM, Krzysztof Kozlowski >> >> 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-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] pinctrl: exynos: Add BUS1 pin controller for exynos7
On Fri, Nov 28, 2014 at 9:15 PM, Linus Walleij wrote: > On Fri, Nov 28, 2014 at 4:39 AM, Vivek Gautam > wrote: >> On Fri, Nov 28, 2014 at 9:05 AM, Vivek Gautam >> wrote: > On Mon, Nov 24, 2014 at 6:32 PM, Vivek Gautam wrote: > USB and Power regulator on Exynos7 require gpios available > in BUS1 pin controller block. > So adding the BUS1 pinctrl support. > > Signed-off-by: Naveen Krishna Ch > Signed-off-by: Vivek Gautam > Cc: Linus Walleij >> >> If the change looks good, will it be possible to pick it fo 3.19-rc1 ? >> That will really help enabling USB IP on exynos7. > > As you know the Exynos driver has a maintainer, Tomasz Figa, I will not > merge patches without his ACK. > > Apart from that, there are *again* a lot of Exynos patches flying around and > I start to loose track of them. If they do not apply together and start to > conflict I will just ask Tomasz to stack them and provide a pull request > again. True, this merge cycle has surely got number of exynos7 related patches in flight, which are difficult to keep track of. Thanks for asking Tomasz. -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: Add dts file for odroid XU3 board
Add DTS for the Hardkernel Odroid XU3. The name of the DTS file is kept the
same as the vendors naming, which means it's prefixed with exynos5422
instead of exynos5800 as the SoC name even though it includes the
exyno5800 dtsi.
Signed-off-by: Sjoerd Simons
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/exynos5422-odroidxu3.dts | 325 +
2 files changed, 326 insertions(+)
create mode 100644 arch/arm/boot/dts/exynos5422-odroidxu3.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 38c89ca..0a898cc 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
exynos5420-arndale-octa.dtb \
exynos5420-peach-pit.dtb \
exynos5420-smdk5420.dtb \
+ exynos5422-odroidxu3.dtb \
exynos5440-sd5v1.dtb \
exynos5440-ssdk5440.dtb \
exynos5800-peach-pi.dtb
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
new file mode 100644
index 000..fe37004
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
@@ -0,0 +1,325 @@
+/*
+ * Hardkernel Odroid XU3 board device tree source
+ *
+ * Copyright (c) 2014 Collabora Ltd.
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+/dts-v1/;
+#include "exynos5800.dtsi"
+
+/ {
+ model = "Hardkernel Odroid XU3";
+ compatible = "hardkernel,odroid-xu3", "samsung,exynos5800",
"samsung,exynos5";
+
+ memory {
+ reg = <0x2000 0x8000>;
+ };
+
+ fimd@1440 {
+ status = "okay";
+ };
+
+ firmware@02073000 {
+ compatible = "samsung,secure-firmware";
+ reg = <0x02073000 0x1000>;
+ };
+
+ fixed-rate-clocks {
+ oscclk {
+ compatible = "samsung,exynos5420-oscclk";
+ clock-frequency = <2400>;
+ };
+ };
+
+ hsi2c_4: i2c@12CA {
+ status = "okay";
+
+ s2mps11_pmic@66 {
+ compatible = "samsung,s2mps11-pmic";
+ reg = <0x66>;
+ s2mps11,buck2-ramp-delay = <12>;
+ s2mps11,buck34-ramp-delay = <12>;
+ s2mps11,buck16-ramp-delay = <12>;
+ s2mps11,buck6-ramp-enable = <1>;
+ s2mps11,buck2-ramp-enable = <1>;
+ s2mps11,buck3-ramp-enable = <1>;
+ s2mps11,buck4-ramp-enable = <1>;
+
+ s2mps11_osc: clocks {
+ #clock-cells = <1>;
+ clock-output-names = "s2mps11_ap",
+ "s2mps11_cp", "s2mps11_bt";
+ };
+
+ regulators {
+ ldo1_reg: LDO1 {
+ regulator-name = "vdd_ldo1";
+ regulator-min-microvolt = <100>;
+ regulator-max-microvolt = <100>;
+ regulator-always-on;
+ };
+
+ ldo3_reg: LDO3 {
+ regulator-name = "vdd_ldo3";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+ };
+
+ ldo5_reg: LDO5 {
+ regulator-name = "vdd_ldo5";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+ };
+
+ ldo6_reg: LDO6 {
+ regulator-name = "vdd_ldo6";
+ regulator-min-microvolt = <100>;
+ regulator-max-microvolt = <100>;
+ regulator-always-on;
+ };
+
+ ldo7_reg: LDO7 {
+ regulator-name = "vdd_ldo7";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+
Re: [PATCH V2] PM / Domains: Add pm_genpd_lookup() API to lookup domain by firmware node
On Mon, Dec 1, 2014 at 3:56 PM, Ulf Hansson wrote: > On 29 November 2014 at 01:30, Rafael J. Wysocki wrote: >> On Friday, November 28, 2014 11:38:35 AM Ulf Hansson wrote: >>> In a step to move away from using genpd's name based APIs, such as the >>> pm_genpd_add_subdomain_names(), provide an API to lookup an already >>> initialized generic PM domain by its firmware node. >>> >>> This API would typically be a called from SOC specific code, to fetch a >>> handle to the domain. Especially convenient to configure subdomains and >>> when the hierarchy of the domains are described in DT. >>> >>> Do note, before pm_genpd_init() is invoked to initialize a generic PM >>> domain, it's the callers responsibility to assign the new ->fwnode >>> pointer in the struct generic_pm_domain, to enable pm_genpd_lookup() to >>> find the domain. >>> >>> Signed-off-by: Ulf Hansson >> >> I have no problems with that, but do you have a user for it? > > Amit Daniel Kachhap, posted a patchset which is adding subdomains > configured from DT for Exynos. It's from there the idea to this patch > comes from. Amit is about to post a new version, based on this patch. Hi Ulf, I was trying to use this API but i felt there is a better way of just exporting of_genpd_get_from_provider as a similar API is used to register the power domain. Check if this make sense www.spinics.net/lists/arm-kernel/msg382665.html. Regards, Amit > > https://lkml.org/lkml/2014/11/24/290 > > Kind regards > Uffe > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 1/3] PM / Domains: Initial PM clock support for genpd
It's quite common for PM domains to use PM clocks. Typically from SOC
specific code, the per device PM clock list is created and
pm_clk_suspend|resume() are invoked to handle clock gating/ungating.
A step towards consolidation is to integrate PM clock support into
genpd, which is what this patch does.
In this initial step, the calls to the pm_clk_suspend|resume() are
handled within genpd, but the per device PM clock list still needs to
be created from SOC specific code. It seems reasonable to have gendp to
handle that as well, but that left to future patches to address.
It's not every users of genpd that are keen on using PM clocks, thus we
need to provide this a configuration option for genpd. Therefore let's
add flag field in the genpd struct to keep this information and define
a new GENDP_FLAG_PM_CLK bit for it.
Signed-off-by: Ulf Hansson
Acked-by: Geert Uytterhoeven
---
Changes in v4:
Rename define.
Changes in v3:
Moved define out of struct definition.
Don't use bitops.h
Rename define to GENDP_PM_CLK.
Changes in v2:
Set ->start() callback to pm_clk_resume() and fixed comment.
---
drivers/base/power/domain.c | 7 +++
include/linux/pm_domain.h | 4
2 files changed, 11 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 735c473..6a103a3 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1928,6 +1929,12 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
genpd->domain.ops.complete = pm_genpd_complete;
genpd->dev_ops.save_state = pm_genpd_default_save_state;
genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
+
+ if (genpd->flags & GENPD_FLAG_PM_CLK) {
+ genpd->dev_ops.stop = pm_clk_suspend;
+ genpd->dev_ops.start = pm_clk_resume;
+ }
+
mutex_lock(&gpd_list_lock);
list_add(&genpd->gpd_list_node, &gpd_list);
mutex_unlock(&gpd_list_lock);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 8cbd32e..6cd20d5 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -17,6 +17,9 @@
#include
#include
+/* Defines used for the flags field in the struct generic_pm_domain */
+#define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */
+
enum gpd_status {
GPD_STATE_ACTIVE = 0, /* PM domain is active */
GPD_STATE_WAIT_MASTER, /* PM domain's master is being waited for */
@@ -76,6 +79,7 @@ struct generic_pm_domain {
struct device *dev);
void (*detach_dev)(struct generic_pm_domain *domain,
struct device *dev);
+ unsigned int flags; /* Bit field of configs for genpd */
};
static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 2/3] ARM: shmobile: Convert to genpd flags for PM clocks for r8a7779
Instead of using the dev_ops ->stop|start() callbacks for genpd, let's
convert to use genpd's flag field and set it to GENPD_FLAG_PM_CLK.
Signed-off-by: Ulf Hansson
Acked-by: Geert Uytterhoeven
---
Changes in v4:
Convert to renamed define.
Changes in v3:
Convert to renamed define.
Changes in v2:
None.
---
arch/arm/mach-shmobile/pm-r8a7779.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/mach-shmobile/pm-r8a7779.c
b/arch/arm/mach-shmobile/pm-r8a7779.c
index 82fe3d7..44a74c4 100644
--- a/arch/arm/mach-shmobile/pm-r8a7779.c
+++ b/arch/arm/mach-shmobile/pm-r8a7779.c
@@ -83,9 +83,8 @@ static void r8a7779_init_pm_domain(struct r8a7779_pm_domain
*r8a7779_pd)
{
struct generic_pm_domain *genpd = &r8a7779_pd->genpd;
+ genpd->flags = GENPD_FLAG_PM_CLK;
pm_genpd_init(genpd, NULL, false);
- genpd->dev_ops.stop = pm_clk_suspend;
- genpd->dev_ops.start = pm_clk_resume;
genpd->dev_ops.active_wakeup = pd_active_wakeup;
genpd->power_off = pd_power_down;
genpd->power_on = pd_power_up;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 0/3] PM / Domains: Add initial PM clock support to genpd
Changes in v4: -Applied acks/tested by tags. -Change name of define, affects all patches. Changes in v3: -Fixed comments from Rafael for patch 1. -Change name of define, affects all patches. Changes in v2: -Small fixes in patch 1. It's quite common for PM domains to use PM clocks. Typically from SOC specific code, the per device PM clock list is created and pm_clk_suspend|resume() are invoked to handle clock gating/ungating. A step towards consolidation is to integrate PM clock support into genpd, which is what this patchset does. In this initial step, the calls to the pm_clk_suspend|resume() are handled within genpd, but the per device PM clock list still needs to be created from SOC specific code. It seems reasonable to have gendp to handle that as well, but that left to future patchsets to address. It's not every users of genpd that are keen on using PM clocks thus we need to provide this a configuration option for genpd. In patch 2 and patch 3, we convert some of the SH-MOBILE SOCs to start using the new PM clock support in genpd. Ulf Hansson (3): PM / Domains: Initial PM clock support for genpd ARM: shmobile: Convert to genpd flags for PM clocks for r8a7779 ARM: shmobile: Convert to genpd flags for PM clocks for R-mobile arch/arm/mach-shmobile/pm-r8a7779.c | 3 +-- arch/arm/mach-shmobile/pm-rmobile.c | 3 +-- drivers/base/power/domain.c | 7 +++ include/linux/pm_domain.h | 4 4 files changed, 13 insertions(+), 4 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 3/3] ARM: shmobile: Convert to genpd flags for PM clocks for R-mobile
Instead of using the dev_ops ->stop|start() callbacks for genpd, let's convert to use genpd's flag field and set it to GENPD_FLAG_PM_CLK. Signed-off-by: Ulf Hansson Acked-by: Geert Uytterhoeven Tested-by: Geert Uytterhoeven --- Changes in v4: Convert to renamed define. Changes in v3: Convert to renamed define. Changes in v2: None. --- arch/arm/mach-shmobile/pm-rmobile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c index 717e641..6f7d56e 100644 --- a/arch/arm/mach-shmobile/pm-rmobile.c +++ b/arch/arm/mach-shmobile/pm-rmobile.c @@ -106,9 +106,8 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd) struct generic_pm_domain *genpd = &rmobile_pd->genpd; struct dev_power_governor *gov = rmobile_pd->gov; + genpd->flags = GENPD_FLAG_PM_CLK; pm_genpd_init(genpd, gov ? : &simple_qos_governor, false); - genpd->dev_ops.stop = pm_clk_suspend; - genpd->dev_ops.start= pm_clk_resume; genpd->dev_ops.active_wakeup= rmobile_pd_active_wakeup; genpd->power_off= rmobile_pd_power_down; genpd->power_on = rmobile_pd_power_up; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] PM/ Domains: Export of_genpd_get_from_provider function.
This function looks up a PM domain form the provider. This will be
useful to add parent/child domain relationship from the SoC specific
code. The caller of the function must make sure that PM domain provider
is already registered.
Signed-off-by: Amit Daniel Kachhap
---
This patch may solve the same purpose which is done by earlier posts
1) https://lkml.org/lkml/2014/11/24/319 - In this there were comments
for not using the PM domain name.
2) Ulf Hansson posted a patch
http://www.spinics.net/lists/linux-samsung-soc/msg39745.html which also solves
similar purpose but I feel this is slightly complex and involves changing
the genpd structure.
drivers/base/power/domain.c |3 ++-
include/linux/pm_domain.h |8
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index d822753..5d7a2c1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2129,7 +2129,7 @@ EXPORT_SYMBOL_GPL(of_genpd_del_provider);
* Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
* on failure.
*/
-static struct generic_pm_domain *of_genpd_get_from_provider(
+struct generic_pm_domain *of_genpd_get_from_provider(
struct of_phandle_args *genpdspec)
{
struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
@@ -2149,6 +2149,7 @@ static struct generic_pm_domain
*of_genpd_get_from_provider(
return genpd;
}
+EXPORT_SYMBOL_GPL(of_genpd_get_from_provider);
/**
* genpd_dev_pm_detach - Detach a device from its PM domain.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9690827..4954e24 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -282,6 +282,8 @@ typedef struct generic_pm_domain *(*genpd_xlate_t)(struct
of_phandle_args *args,
int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
void *data);
void of_genpd_del_provider(struct device_node *np);
+struct generic_pm_domain *of_genpd_get_from_provider(
+ struct of_phandle_args *genpdspec);
struct generic_pm_domain *__of_genpd_xlate_simple(
struct of_phandle_args *genpdspec,
@@ -299,6 +301,12 @@ static inline int __of_genpd_add_provider(struct
device_node *np,
}
static inline void of_genpd_del_provider(struct device_node *np) {}
+static inline struct generic_pm_domain *of_genpd_get_from_provider(
+ struct of_phandle_args *genpdspec)
+{
+ return NULL;
+}
+
#define __of_genpd_xlate_simpleNULL
#define __of_genpd_xlate_onecell NULL
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT 2/2] arm: dts: disable CCI on exynos420 based arndale-octa
On pon, 2014-12-01 at 11:09 +, Russell King - ARM Linux wrote: > On Mon, Dec 01, 2014 at 10:03:28AM +0100, Krzysztof Kozlowski wrote: > > On pią, 2014-11-28 at 21:09 +0530, Abhilash Kesavan wrote: > > > Hello Krzysztof, > > > > > > On Fri, Nov 28, 2014 at 8:49 PM, Krzysztof Kozlowski > > > wrote: > > > > On pią, 2014-11-28 at 20:20 +0530, Abhilash Kesavan wrote: > > > >> The arndale-octa board was giving "imprecise external aborts" during > > > >> boot-up with MCPM enabled. CCI enablement of the boot cluster was found > > > >> to be the cause of these aborts (possibly because the secure f/w was > > > >> not > > > >> allowing it). Hence, disable CCI for the arndale-octa board. > > > >> > > > >> Signed-off-by: Abhilash Kesavan > > > >> --- > > > >> arch/arm/boot/dts/exynos5420-arndale-octa.dts |4 > > > >> arch/arm/boot/dts/exynos5420.dtsi |2 +- > > > >> 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > I tested these 2 patches on Arndale Octa but there are no improvements. > > > > I still got imprecise aborts (some not fatal and sometimes killing init > > > > with full backtrace). > > > > > > Thanks for testing. Are you testing this with exynos_defconfig with no > > > other changes ? Can you please confirm from the bootlog that MCPM and > > > CCI are not being initialized. > > > > > That was exynos_defconfig with disabled DRM and enabled some debug, > > next-20141128. > > > > When I tried only exynos_defconfig (with disabled DRM) it worked fine... > > So the imprecise aborts were caused by one of following debug options: > > > > DEBUG_SECTION_MISMATCH > > DYNAMIC_DEBUG > > DEBUG_ATOMIC_SLEEP > > DEBUG_PREEMPT > > PROVE_LOCKING > > LOCKUP_DETECTOR > > DEBUG_LOCK_ALLOC > > PROVE_RCU > > DEBUG_RT_MUTEXES > > DEBUG_MUTEXES > > DEBUG_SPINLOCK > > DEBUG_LIST > > DEBUG_PAGEALLOC > > SPARSE_RCU_POINTER > > DEBUG_FS > > PM_DEBUG > > PM_ADVANCED_DEBUG > > GPIO_SYSFS > > > > > Can you remove these 2 patches and on linux-next check if you are > > > getting aborts even with 5420_MCPM disabled. > > > > I tried this already and imprecise aborts shown, however with my > > debugging options above. > > > > Overall the patches seems to work properly (although the debugging issue > > needs to be resolved still), so: > > > > On Arndale Octa (Exynos 5420): > > Tested-by: Krzysztof Kozlowski > > Reading this message, it seems that this should *not* be given a tested-by, > because it seems from what you've reported above, they don't work correctly. > > If you have to turn debugging options off in order to get the kernel to > apparently run correctly after applying some patches, it means those > patches themselves are probably buggy, rather than the debug itself > being buggy. > > I'd suggest that you have some further work to do (a manual bisect of the > config options you've disabled) to discover which is the cause of the > problem. > > It could be that the code introduces something like a use-after-free bug. Maybe I was to eager to add Tested-by but (without debugging options) one imprecise-abort disappears with the patchset. Other issues seem not to be fixed. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT 2/2] arm: dts: disable CCI on exynos420 based arndale-octa
On Mon, Dec 01, 2014 at 10:03:28AM +0100, Krzysztof Kozlowski wrote: > On pią, 2014-11-28 at 21:09 +0530, Abhilash Kesavan wrote: > > Hello Krzysztof, > > > > On Fri, Nov 28, 2014 at 8:49 PM, Krzysztof Kozlowski > > wrote: > > > On pią, 2014-11-28 at 20:20 +0530, Abhilash Kesavan wrote: > > >> The arndale-octa board was giving "imprecise external aborts" during > > >> boot-up with MCPM enabled. CCI enablement of the boot cluster was found > > >> to be the cause of these aborts (possibly because the secure f/w was not > > >> allowing it). Hence, disable CCI for the arndale-octa board. > > >> > > >> Signed-off-by: Abhilash Kesavan > > >> --- > > >> arch/arm/boot/dts/exynos5420-arndale-octa.dts |4 > > >> arch/arm/boot/dts/exynos5420.dtsi |2 +- > > >> 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > I tested these 2 patches on Arndale Octa but there are no improvements. > > > I still got imprecise aborts (some not fatal and sometimes killing init > > > with full backtrace). > > > > Thanks for testing. Are you testing this with exynos_defconfig with no > > other changes ? Can you please confirm from the bootlog that MCPM and > > CCI are not being initialized. > > > That was exynos_defconfig with disabled DRM and enabled some debug, > next-20141128. > > When I tried only exynos_defconfig (with disabled DRM) it worked fine... > So the imprecise aborts were caused by one of following debug options: > > DEBUG_SECTION_MISMATCH > DYNAMIC_DEBUG > DEBUG_ATOMIC_SLEEP > DEBUG_PREEMPT > PROVE_LOCKING > LOCKUP_DETECTOR > DEBUG_LOCK_ALLOC > PROVE_RCU > DEBUG_RT_MUTEXES > DEBUG_MUTEXES > DEBUG_SPINLOCK > DEBUG_LIST > DEBUG_PAGEALLOC > SPARSE_RCU_POINTER > DEBUG_FS > PM_DEBUG > PM_ADVANCED_DEBUG > GPIO_SYSFS > > > Can you remove these 2 patches and on linux-next check if you are > > getting aborts even with 5420_MCPM disabled. > > I tried this already and imprecise aborts shown, however with my > debugging options above. > > Overall the patches seems to work properly (although the debugging issue > needs to be resolved still), so: > > On Arndale Octa (Exynos 5420): > Tested-by: Krzysztof Kozlowski Reading this message, it seems that this should *not* be given a tested-by, because it seems from what you've reported above, they don't work correctly. If you have to turn debugging options off in order to get the kernel to apparently run correctly after applying some patches, it means those patches themselves are probably buggy, rather than the debug itself being buggy. I'd suggest that you have some further work to do (a manual bisect of the config options you've disabled) to discover which is the cause of the problem. It could be that the code introduces something like a use-after-free bug. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] drm/panel: add s6e63j0x03 LCD Panel driver
On Fri, Nov 28, 2014 at 08:39:50PM +0900, Inki Dae wrote:
> This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD Panel driver
> which uses mipi_dsi bus to communicate with Panel.
s/Panel/panel/. Also I prefer to have more information in the commit
message than this. Physical size and resolution would be good. Also a
small comment as to which device this panel can be found in would be
good.
> +config DRM_PANEL_S6E63J0X03
> + tristate "S6E63J0X03 DSI video mode panel"
> + depends on OF
> + select DRM_MIPI_DSI
> + select VIDEOMODE_HELPERS
> +
> endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 8b92921..7f36dc2 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
> obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
> obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
> +obj-$(CONFIG_DRM_PANEL_S6E63J0X03) += panel-s6e63j0x03.o
> diff --git a/drivers/gpu/drm/panel/panel-s6e63j0x03.c
> b/drivers/gpu/drm/panel/panel-s6e63j0x03.c
[...]
> +struct s6e63j0x03 {
> + struct device *dev;
> + struct drm_panel panel;
> + struct backlight_device *bl_dev;
> +
> + struct regulator_bulk_data supplies[2];
> + struct gpio_desc *reset_gpio;
> + u32 power_on_delay;
> + u32 power_off_delay;
> + u32 reset_delay;
> + u32 init_delay;
> + bool flip_horizontal;
> + bool flip_vertical;
> + struct videomode vm;
Specifying the video mode in DT should be optional and only used to
override the default mode provided by the driver.
> + u32 width_mm;
> + u32 height_mm;
There's no need for these to be sized types. unsigned int will do just
fine.
> + int power;
What's the difference between this and bl_dev->properties.power?
> +
> + /* This field is tested by functions directly accessing DSI bus before
> + * transfer, transfer is skipped if it is set. In case of transfer
> + * failure or unexpected response the field is set to error value.
> + * Such construct allows to eliminate many checks in higher level
> + * functions.
> + */
The format of this comment is wrong, it should be:
/*
* ...
*/
> + int error;
> +};
> +
> +static const unsigned char GAMMA_10[] = {
...
These array names should not be all-caps.
> +static const unsigned char *gamma_tbl[MAX_GAMMA_CNT] = {
> + GAMMA_10,
> + GAMMA_30,
> + GAMMA_60,
> + GAMMA_90,
> + GAMMA_120,
> + GAMMA_150,
> + GAMMA_200,
> + GAMMA_200,
> + GAMMA_240,
> + GAMMA_300,
> + GAMMA_300
> +};
Why not just do this using a two-dimensional array?
> +
> +static inline struct s6e63j0x03 *panel_to_s6e63j0x03(struct drm_panel *panel)
> +{
> + return container_of(panel, struct s6e63j0x03, panel);
> +}
> +
> +static int s6e63j0x03_clear_error(struct s6e63j0x03 *ctx)
> +{
> + int ret = ctx->error;
> +
> + ctx->error = 0;
> + return ret;
> +}
The only place where you ever call this ignores the return value.
> +
> +static void s6e63j0x03_dcs_write(struct s6e63j0x03 *ctx, const void *data,
> size_t len)
> +{
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> + int ret;
> +
> + if (ctx->error < 0)
> + return;
> +
> + ret = mipi_dsi_dcs_write(dsi, data, len);
> + if (ret < 0) {
> + dev_err(ctx->dev, "error %d writing dcs seq: %*ph\n", ret, len,
> + data);
> + ctx->error = ret;
> + }
> +}
The only reason for these wrappers seems to be the ctx->error handling.
But that error handling is flawed because it silently ignores errors.
The only location where you check for errors is in .prepare(), but you
only do so after all commands have been executed, so you loose all
context about what exactly went wrong.
I'd prefer if the driver just did straightforward checking of all
commands that could possibly fail.
> +static void s6e63j0x03_set_maximum_return_packet_size(struct s6e63j0x03 *ctx,
> + unsigned int size)
> +{
> + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> + const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +
> + if (ops && ops->transfer) {
> + unsigned char buf[] = {size, 0};
> + struct mipi_dsi_msg msg = {
> + .channel = dsi->channel,
> + .type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
> + .tx_len = sizeof(buf),
> + .tx_buf = buf
> + };
> + int ret;
> +
> + ret = ops->transfer(dsi->host, &msg);
> + if (ret < 0) {
> + dev_err(ctx->dev, "failed to transfer message.\n");
> + ctx->error = ret;
> + }
> + }
> +}
Use mipi_dsi_set_maximum_return_packet_size().
> +static void s6e63j0x03_set_
Re: [PATCH V2] PM / Domains: Add pm_genpd_lookup() API to lookup domain by firmware node
On 29 November 2014 at 01:30, Rafael J. Wysocki wrote: > On Friday, November 28, 2014 11:38:35 AM Ulf Hansson wrote: >> In a step to move away from using genpd's name based APIs, such as the >> pm_genpd_add_subdomain_names(), provide an API to lookup an already >> initialized generic PM domain by its firmware node. >> >> This API would typically be a called from SOC specific code, to fetch a >> handle to the domain. Especially convenient to configure subdomains and >> when the hierarchy of the domains are described in DT. >> >> Do note, before pm_genpd_init() is invoked to initialize a generic PM >> domain, it's the callers responsibility to assign the new ->fwnode >> pointer in the struct generic_pm_domain, to enable pm_genpd_lookup() to >> find the domain. >> >> Signed-off-by: Ulf Hansson > > I have no problems with that, but do you have a user for it? Amit Daniel Kachhap, posted a patchset which is adding subdomains configured from DT for Exynos. It's from there the idea to this patch comes from. Amit is about to post a new version, based on this patch. https://lkml.org/lkml/2014/11/24/290 Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 3/3] ARM: shmobile: Convert to genpd flags for PM clocks for R-mobile
On Fri, Nov 28, 2014 at 9:30 AM, Ulf Hansson wrote:
> Instead of using the dev_ops ->stop|start() callbacks for genpd, let's
> convert to use genpd's flag field and set it to GENPD_PM_CLK.
>
> Signed-off-by: Ulf Hansson
Tested-by: Geert Uytterhoeven
(tested on r8a7740/armadillo-multiplatform and sh73a0/kzm9g-multiplatform).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 0/3] PM / Domains: Add initial PM clock support to genpd
On Fri, Nov 28, 2014 at 9:30 AM, Ulf Hansson wrote:
> Changes in v3:
> -Fixed comments from Rafael for patch 1.
> -Change name of define, affects all patches.
>
> Changes in v2:
> -Small fixes in patch 1.
>
>
> It's quite common for PM domains to use PM clocks. Typically from SOC specific
> code, the per device PM clock list is created and pm_clk_suspend|resume() are
> invoked to handle clock gating/ungating.
>
> A step towards consolidation is to integrate PM clock support into genpd,
> which
> is what this patchset does.
>
> In this initial step, the calls to the pm_clk_suspend|resume() are handled
> within genpd, but the per device PM clock list still needs to be created from
> SOC specific code. It seems reasonable to have gendp to handle that as well,
> but
> that left to future patchsets to address.
>
> It's not every users of genpd that are keen on using PM clocks thus we need
> to provide this a configuration option for genpd.
>
> In patch 2 and patch 3, we convert some of the SH-MOBILE SOCs to start using
> the
> new PM clock support in genpd.
>
>
> Ulf Hansson (3):
> PM / Domains: Initial PM clock support for genpd
> ARM: shmobile: Convert to genpd flags for PM clocks for r8a7779
> ARM: shmobile: Convert to genpd flags for PM clocks for R-mobile
>
> arch/arm/mach-shmobile/pm-r8a7779.c | 3 +--
> arch/arm/mach-shmobile/pm-rmobile.c | 3 +--
> drivers/base/power/domain.c | 7 +++
> include/linux/pm_domain.h | 4
> 4 files changed, 13 insertions(+), 4 deletions(-)
For the series:
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] clk: samsung: exynos4415: Fix build with PM_SLEEP disabled
Fix following build errors when PM_SLEEP is disabled (e.g. by disabling
SUSPEND and HIBERNATION):
drivers/clk/samsung/clk-exynos4415.c: In function ‘exynos4415_cmu_init’:
drivers/clk/samsung/clk-exynos4415.c:982:2: error: ‘exynos4415_ctx’ undeclared
(first use in this function)
drivers/clk/samsung/clk-exynos4415.c:982:2: note: each undeclared identifier is
reported only once for each function it appears in
drivers/clk/samsung/clk-exynos4415.c: In function ‘exynos4415_cmu_dmc_init’:
drivers/clk/samsung/clk-exynos4415.c:1123:2: error: ‘exynos4415_dmc_ctx’
undeclared (first use in this function)
make[3]: *** [drivers/clk/samsung/clk-exynos4415.o] Error 1
Signed-off-by: Krzysztof Kozlowski
---
drivers/clk/samsung/clk-exynos4415.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos4415.c
b/drivers/clk/samsung/clk-exynos4415.c
index c7208c7a3add..2123fc251e0f 100644
--- a/drivers/clk/samsung/clk-exynos4415.c
+++ b/drivers/clk/samsung/clk-exynos4415.c
@@ -118,12 +118,13 @@ enum exynos4415_plls {
nr_plls,
};
+static struct samsung_clk_provider *exynos4415_ctx;
+
/*
* Support for CMU save/restore across system suspends
*/
#ifdef CONFIG_PM_SLEEP
static struct samsung_clk_reg_dump *exynos4415_clk_regs;
-static struct samsung_clk_provider *exynos4415_ctx;
static unsigned long exynos4415_cmu_clk_regs[] __initdata = {
SRC_LEFTBUS,
@@ -1031,9 +1032,10 @@ enum exynos4415_dmc_plls {
nr_dmc_plls,
};
+static struct samsung_clk_provider *exynos4415_dmc_ctx;
+
#ifdef CONFIG_PM_SLEEP
static struct samsung_clk_reg_dump *exynos4415_dmc_clk_regs;
-static struct samsung_clk_provider *exynos4415_dmc_ctx;
static unsigned long exynos4415_cmu_dmc_clk_regs[] __initdata = {
MPLL_LOCK,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT 2/2] arm: dts: disable CCI on exynos420 based arndale-octa
On pią, 2014-11-28 at 21:09 +0530, Abhilash Kesavan wrote: > Hello Krzysztof, > > On Fri, Nov 28, 2014 at 8:49 PM, Krzysztof Kozlowski > wrote: > > On pią, 2014-11-28 at 20:20 +0530, Abhilash Kesavan wrote: > >> The arndale-octa board was giving "imprecise external aborts" during > >> boot-up with MCPM enabled. CCI enablement of the boot cluster was found > >> to be the cause of these aborts (possibly because the secure f/w was not > >> allowing it). Hence, disable CCI for the arndale-octa board. > >> > >> Signed-off-by: Abhilash Kesavan > >> --- > >> arch/arm/boot/dts/exynos5420-arndale-octa.dts |4 > >> arch/arm/boot/dts/exynos5420.dtsi |2 +- > >> 2 files changed, 5 insertions(+), 1 deletion(-) > > > > I tested these 2 patches on Arndale Octa but there are no improvements. > > I still got imprecise aborts (some not fatal and sometimes killing init > > with full backtrace). > > Thanks for testing. Are you testing this with exynos_defconfig with no > other changes ? Can you please confirm from the bootlog that MCPM and > CCI are not being initialized. > That was exynos_defconfig with disabled DRM and enabled some debug, next-20141128. When I tried only exynos_defconfig (with disabled DRM) it worked fine... So the imprecise aborts were caused by one of following debug options: DEBUG_SECTION_MISMATCH DYNAMIC_DEBUG DEBUG_ATOMIC_SLEEP DEBUG_PREEMPT PROVE_LOCKING LOCKUP_DETECTOR DEBUG_LOCK_ALLOC PROVE_RCU DEBUG_RT_MUTEXES DEBUG_MUTEXES DEBUG_SPINLOCK DEBUG_LIST DEBUG_PAGEALLOC SPARSE_RCU_POINTER DEBUG_FS PM_DEBUG PM_ADVANCED_DEBUG GPIO_SYSFS > Can you remove these 2 patches and on linux-next check if you are > getting aborts even with 5420_MCPM disabled. I tried this already and imprecise aborts shown, however with my debugging options above. Overall the patches seems to work properly (although the debugging issue needs to be resolved still), so: On Arndale Octa (Exynos 5420): Tested-by: Krzysztof Kozlowski Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] pinctrl: exynos: Fix GPIO setup failure because domain clock being gated
On nie, 2014-11-30 at 21:19 +0900, Tomasz Figa wrote: > Hi Krzysztof, > > 2014-11-28 23:08 GMT+09:00 Krzysztof Kozlowski : > > On pią, 2014-11-28 at 15:04 +0100, Linus Walleij wrote: > >> On Wed, Nov 26, 2014 at 3:24 PM, Krzysztof Kozlowski > >> 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-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] drm/exynos: Add DECON driver
Hi Ajay,
On 28 November 2014 at 16:45, Ajay Kumar wrote:
> This series is based on exynos-drm-next branch of Inki Dae's tree at:
> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>
> DECON(Display and Enhancement Controller) is the new IP
> in exynos7 SOC for generating video signals using pixel data.
>
> DECON driver can be used to drive 2 different interfaces on Exynos7:
> DECON-INT(video controller) and DECON-EXT(Mixer for HDMI)
>
> The existing FIMD driver code was used as a template to create
> DECON driver. Only DECON-INT is supported as of now, and
> DECON-EXT support will be added later.
>
> Signed-off-by: Akshu Agrawal
> Signed-off-by: Ajay Kumar
> ---
> Changes since V1:
> -- Address comments from Pankaj and do few cleanups.
>
Thanks, but still I can see this needs modification, please see my comments:
> .../devicetree/bindings/video/exynos7-decon.txt| 67 ++
> drivers/gpu/drm/exynos/Kconfig | 13 +-
> drivers/gpu/drm/exynos/Makefile|1 +
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 1037
>
> drivers/gpu/drm/exynos/exynos_drm_drv.c|4 +
> drivers/gpu/drm/exynos/exynos_drm_drv.h|1 +
> include/video/exynos7_decon.h | 346 +++
> 7 files changed, 1466 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/exynos7-decon.txt
> create mode 100644 drivers/gpu/drm/exynos/exynos7_drm_decon.c
> create mode 100644 include/video/exynos7_decon.h
>
[snip]
> +static int decon_mgr_initialize(struct exynos_drm_manager *mgr,
> + struct drm_device *drm_dev)
> +{
> + struct decon_context *ctx = mgr_to_decon(mgr);
> + struct exynos_drm_private *priv = drm_dev->dev_private;
> + int ret;
> +
> + mgr->drm_dev = drm_dev;
> + mgr->pipe = ctx->pipe = priv->pipe++;
> +
Do we really need 'pipe' in exynos_drm_manager and decon_context both?
> + /* attach this sub driver to iommu mapping if supported. */
> + if (is_drm_iommu_supported(mgr->drm_dev)) {
[snip]
> +static void decon_win_mode_set(struct exynos_drm_manager *mgr,
> + struct exynos_drm_overlay *overlay)
> +{
> + struct decon_context *ctx = mgr_to_decon(mgr);
> + struct decon_win_data *win_data;
> + int win, padding;
> +
> + if (!overlay) {
> + DRM_ERROR("overlay is NULL\n");
> + return;
> + }
> +
> + win = overlay->zpos;
> + if (win == DEFAULT_ZPOS)
> + win = ctx->default_win;
> +
> + if (win < 0 || win >= WINDOWS_NR)
> + return;
> +
> +
> + win_data = &ctx->win_data[win];
> +
As I mentioned in V1, since these 5 lines are getting repeating better
we move it in one static function. It will help in reducing code
footprint.
[snip]
> +
> +/**
> + * shadow_protect_win() - disable updating values from shadow registers at
> vsync
> + *
> + * @win: window to protect registers for
> + * @protect: 1 to protect (disable updates)
> + */
> +static void decon_shadow_protect_win(struct decon_context *ctx,
> + int win, bool protect)
> +{
> + u32 reg, bits, val;
> +
> + reg = SHADOWCON;
How about using SHADOWCON directly instead of using local variable?
> + bits = SHADOWCON_WINx_PROTECT(win);
> +
> + val = readl(ctx->regs + reg);
> + if (protect)
> + val |= bits;
> + else
> + val &= ~bits;
> + writel(val, ctx->regs + reg);
> +}
> +
> +static void decon_win_commit(struct exynos_drm_manager *mgr, int zpos)
> +{
> + struct decon_context *ctx = mgr_to_decon(mgr);
> + struct decon_win_data *win_data;
> + int win = zpos;
> + unsigned long val, alpha, blendeq;
> + unsigned int last_x;
> + unsigned int last_y;
> +
> + if (ctx->suspended)
> + return;
> +
> + if (win == DEFAULT_ZPOS)
> + win = ctx->default_win;
> +
> + if (win < 0 || win >= WINDOWS_NR)
> + return;
> +
> + win_data = &ctx->win_data[win];
> +
> + /* If suspended, enable this on resume */
> + if (ctx->suspended) {
> + win_data->resume = true;
> + return;
> + }
> +
> + /*
[snip]
> +static int decon_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct decon_context *ctx;
> + struct resource *res;
> + int ret = -EINVAL;
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->manager.type = EXYNOS_DISPLAY_TYPE_LCD;
> + ctx->manager.ops = &decon_manager_ops;
> +
> + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC,
