All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: JeffyChen <jeffy.chen@rock-chips.com>
Cc: Tomasz Figa <tfiga@chromium.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Caesar Wang <wxt@rock-chips.com>,
	Elaine Zhang <zhangqing@rock-chips.com>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH] soc: rockchip: power-domain: remove PM clocks
Date: Thu, 1 Mar 2018 09:33:22 +0100	[thread overview]
Message-ID: <CAMuHMdU9PFAXwo+1Z7Baw1fanst9yFKZ3ohoSTiQiMKTaYMVVw@mail.gmail.com> (raw)
In-Reply-To: <5A977638.8010506@rock-chips.com>

Hi Jeffy,

On Thu, Mar 1, 2018 at 4:40 AM, JeffyChen <jeffy.chen@rock-chips.com> wrote:
> if i'm reading the code right, the PM clk means:
> 1/ the clocks which would be enabled while power on
> 2/ these clocks are optional, it's ok if anything wrong with them
> 3/ controlled by pm_domain(or USE_PM_CLK_RUNTIME_OPS & pm_clk_add_notifier)
>
> and currently we're adding all clocks of the attached device as PM clk in
> rockchip PM domain driver, which seems wrong. because we might have these
> kinds of clocks:
> 1/ critical, should block power on if anything wrong with it(failed to get/
> prepare/ enable)
> 2/ optional, could ignore it if anything wrong
> 3/ only required in some special cases, for example register r/w, and
> doesn't need to stay enabled while power on
>
> so maybe we can:
> 1/ let the device(dts) or driver decide which clock is PM clk, and add it
> using *pm_clk_add* APIs (even of_pm_clk_add_clks() if all clocks are pm clk)
>
> 2/ add support for critical PM clk, which would return error to the driver
> if anything wrong
>
> 3/ make sure PM clk always be controlled(otherwise it might be unexpected
> disabled by other clocks under the same clk parent?):
>  a) make sure Runtime PM is always enabled. and as discussed, we can select
> PM in ARCH_ROCKCHIP

On Renesas SoCs, we only add the device's module clock with pm_clk_add().
Drivers that don't care about properties of the module clock just call
pm_runtime_*(). That way the same driver works on different SoCs using
the same device, with and without power and/or clock domains.

Drivers that care about properties of the module clock (mainly frequency)
can still use the clk_*() API for that. Other (optional) clocks must be
handled by the device driver itself.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

WARNING: multiple messages have this Message-ID (diff)
From: geert@linux-m68k.org (Geert Uytterhoeven)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] soc: rockchip: power-domain: remove PM clocks
Date: Thu, 1 Mar 2018 09:33:22 +0100	[thread overview]
Message-ID: <CAMuHMdU9PFAXwo+1Z7Baw1fanst9yFKZ3ohoSTiQiMKTaYMVVw@mail.gmail.com> (raw)
In-Reply-To: <5A977638.8010506@rock-chips.com>

Hi Jeffy,

On Thu, Mar 1, 2018 at 4:40 AM, JeffyChen <jeffy.chen@rock-chips.com> wrote:
> if i'm reading the code right, the PM clk means:
> 1/ the clocks which would be enabled while power on
> 2/ these clocks are optional, it's ok if anything wrong with them
> 3/ controlled by pm_domain(or USE_PM_CLK_RUNTIME_OPS & pm_clk_add_notifier)
>
> and currently we're adding all clocks of the attached device as PM clk in
> rockchip PM domain driver, which seems wrong. because we might have these
> kinds of clocks:
> 1/ critical, should block power on if anything wrong with it(failed to get/
> prepare/ enable)
> 2/ optional, could ignore it if anything wrong
> 3/ only required in some special cases, for example register r/w, and
> doesn't need to stay enabled while power on
>
> so maybe we can:
> 1/ let the device(dts) or driver decide which clock is PM clk, and add it
> using *pm_clk_add* APIs (even of_pm_clk_add_clks() if all clocks are pm clk)
>
> 2/ add support for critical PM clk, which would return error to the driver
> if anything wrong
>
> 3/ make sure PM clk always be controlled(otherwise it might be unexpected
> disabled by other clocks under the same clk parent?):
>  a) make sure Runtime PM is always enabled. and as discussed, we can select
> PM in ARCH_ROCKCHIP

On Renesas SoCs, we only add the device's module clock with pm_clk_add().
Drivers that don't care about properties of the module clock just call
pm_runtime_*(). That way the same driver works on different SoCs using
the same device, with and without power and/or clock domains.

Drivers that care about properties of the module clock (mainly frequency)
can still use the clk_*() API for that. Other (optional) clocks must be
handled by the device driver itself.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

  reply	other threads:[~2018-03-01  8:33 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-28 11:11 [PATCH] soc: rockchip: power-domain: remove PM clocks Jeffy Chen
2018-02-28 11:11 ` Jeffy Chen
2018-02-28 11:59 ` Heiko Stübner
2018-02-28 11:59   ` Heiko Stübner
2018-02-28 12:17 ` Geert Uytterhoeven
2018-02-28 12:17   ` Geert Uytterhoeven
2018-02-28 12:29   ` Tomasz Figa
2018-02-28 12:29     ` Tomasz Figa
2018-02-28 12:32     ` Geert Uytterhoeven
2018-02-28 12:32       ` Geert Uytterhoeven
2018-02-28 12:49       ` Tomasz Figa
2018-02-28 12:49         ` Tomasz Figa
2018-02-28 13:11         ` Geert Uytterhoeven
2018-02-28 13:11           ` Geert Uytterhoeven
2018-02-28 14:07           ` Tomasz Figa
2018-02-28 14:07             ` Tomasz Figa
2018-03-01  3:40             ` JeffyChen
2018-03-01  3:40               ` JeffyChen
2018-03-01  8:33               ` Geert Uytterhoeven [this message]
2018-03-01  8:33                 ` Geert Uytterhoeven
2018-03-01  9:09                 ` JeffyChen
2018-03-01  9:09                   ` JeffyChen
2018-03-01 10:18                 ` Ulf Hansson
2018-03-01 10:18                   ` Ulf Hansson
2018-03-01 10:37                   ` Geert Uytterhoeven
2018-03-01 10:37                     ` Geert Uytterhoeven
2018-03-01 11:22                     ` Ulf Hansson
2018-03-01 11:22                       ` Ulf Hansson
2018-03-01 11:54                       ` Geert Uytterhoeven
2018-03-01 11:54                         ` Geert Uytterhoeven
2018-02-28 12:36   ` JeffyChen
2018-02-28 12:36     ` JeffyChen

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=CAMuHMdU9PFAXwo+1Z7Baw1fanst9yFKZ3ohoSTiQiMKTaYMVVw@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=heiko@sntech.de \
    --cc=jeffy.chen@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=tfiga@chromium.org \
    --cc=ulf.hansson@linaro.org \
    --cc=wxt@rock-chips.com \
    --cc=zhangqing@rock-chips.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.