Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1563435

Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due disabled power domain

From Javier Martinez Canillas <javier@osg.samsung.com>
Newsgroups linux.kernel
Subject Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due disabled power domain
Date 2017-01-20 11:10 +0100
Message-ID <t1EiS-5VC-5@gated-at.bofh.it> (permalink)
References (1 earlier) <t1lJg-2Aj-17@gated-at.bofh.it> <t0Msa-5wG-15@gated-at.bofh.it> <t1lJf-2Aj-15@gated-at.bofh.it> <t1mlY-2PW-17@gated-at.bofh.it> <t1CAp-4Pq-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hello Marek,

On 01/20/2017 05:08 AM, Marek Szyprowski wrote:
> Hi Javier,
> 

[snip]

>> Ok, I misunderstood the relationship between runtime PM and the power domains
>> then. I thought the power domains were only powered on when the runtime PM
>> framework resumed an associated device (i.e: pm_runtime_get_sync() is called).
> 
> Power domains are implemented transparently for the drivers. Even when driver
> doesn't support runtime pm, but its device is in the power domain, the core will
> ensure that the domain will be turned on all the time the driver is bound to the
> device.
>

I got it now, thank a lot for your explanation.
 
>> But even if this isn't the case, shouldn't the reset in probe only be needed
>> if CONFIG_PM isn't enabled? (IOW, $SUBJECT but with another commit message).
> 
> This looks like an over-engineering. I don't like polluting driver code with
> conditional statements like IS_ENABLED(CONFIG_*). It should not hurt to reset
> the device in driver probe, especially just in case the device was left in
> some partially configured/working state by bootloader or previous kernel run
> (if started from kexec). Adding this conditional code to avoid some issues
> with power domain or clocks configuration also suggests that one should
> instead solve the problem elsewhere. Driver should be able to access device
> registers in its probe() in any case without the additional hacks.
>

Fair enough, I already posted the patch but I'll ask it to be dropped.

[snip]

>>
>> This seems to be caused by some needed clocks to access the power domains
>> to be gated, since I don't get these erros when passing clk_ignore_unused
>> as parameter in the kernel command line.
> 
> I think that those issues were fixes by the following patch:
> https://patchwork.kernel.org/patch/9484607/
> It still didn't reach mainline, but I hope it will go as a fix to v4.10.
>

That won't be enough since the CLK_ACLK432_SCALER needs to be ungated for
Exynos5422/5800 as mentioned on another mail in this thread.

> Please test if this solves your issue. Please not that adding more clocks
> to the power domain drivers will solve only the problem with turning domain
> on/off, but the are more cases where those clocks should be turned on (like
> IOMMU integration), so marking them as critical solves that problem too.
>

Ok, I understand your point.
 
> Best regards

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 1/2] [media] exynos-gsc: Fix unbalanced pm_runtime_enable() error Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-18 01:40 +0100
  [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-18 01:40 +0100
    Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Marek Szyprowski <m.szyprowski@samsung.com> - 2017-01-19 15:20 +0100
      Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-19 16:00 +0100
        Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-19 19:00 +0100
          Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Marek Szyprowski <m.szyprowski@samsung.com> - 2017-01-20 09:40 +0100
            Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-20 11:10 +0100
              Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-20 11:50 +0100
        Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Marek Szyprowski <m.szyprowski@samsung.com> - 2017-01-20 09:20 +0100
          Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-20 11:10 +0100
          Re: [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due  disabled power domain Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-20 14:40 +0100
  Re: [PATCH 1/2] [media] exynos-gsc: Fix unbalanced  pm_runtime_enable() error Javier Martinez Canillas <javier@osg.samsung.com> - 2017-01-19 15:20 +0100
  Re: [PATCH 1/2] [media] exynos-gsc: Fix unbalanced pm_runtime_enable()  error Marek Szyprowski <m.szyprowski@samsung.com> - 2017-01-19 15:20 +0100

csiph-web