Quoting Marek Szyprowski (2017-03-30 00:24:15)
> Hi Michael,
>
> On 2017-03-29 22:22, Michael Turquette wrote:
> > Quoting Marek Szyprowski (2017-03-22 04:35:40)
> >> Registers for some clocks might be located in the SOC area, which are under the
> >> power domain. To enable access to those registers respective domain has to be
> >> turned on. Additionally, registers for such clocks will usually loose its
> >> contents when power domain is turned off, so additional saving and restoring of
> >> them might be needed in the clock controller driver.
> >>
> >> This patch adds basic infrastructure in the clocks core to allow implementing
> >> driver for such clocks under power domains. Clock provider can supply a
> >> struct device pointer, which is the used by clock core for tracking and managing
> >> clock's controller runtime pm state. Each clk_prepare() operation
> >> will first call pm_runtime_get_sync() on the supplied device, while
> >> clk_unprepare() will do pm_runtime_put_sync() at the end.
> >>
> >> Additional calls to pm_runtime_get/put functions are required to ensure that any
> >> register access (like calculating/changing clock rates and unpreparing/disabling
> >> unused clocks on boot) will be done with clock controller in runtime resumend
> >> state.
> > The above is a bit confusing. Is clk_prepare really special? It seems to
> > me that every single clk_ops callback will need this?
>
> clk_prepare/unprepare are special, because they allow sleeping, so they
> are natural
> candidates for the place for calling runtime PM operations.
> clk_enable()/disable()
> is called under a spinlock, so runtime pm cannot be called efficiently
> there, but
> core guarantees that they are called after clk_prepare(), so accessing
> hw registers
> is safe. The only remaining calls are not guaranteed to be called always
> after
> clk_prepare(), so those additional calls and checks in runtime pm are
> needed there.
Right, so any call that holds prepare_lock and might touch the hardware
needs to first call pm_runtime_get_sync.
>
> > That leads to my second question: why put this in the clk core? Can the
> > clk provider driver simply add pm_runtime_{get,put} calls into its
> > clk_ops callbacks instead? The clk core does not directly touch hardware
> > (e.g. reading registers) so putting the pm runtime calls into the
> > provider callbacks should be sufficient.
>
> In theory is should be possible to duplicate all kind of clock build blocks
> (gates, muxes, dividers, ...) with additional runtime pm calls. This
> would however
> end in much more code and a bit more complicated locking. Implementing
> it in clk
> core made the code simpler. It also turned out that runtime pm
> integration is
> needed for more that a single clock provider: besides Samsung SoCs
> (Exynos 5433
> and newer, Exynos 4412 ISP, Exynos Audio Subsystem, hacks in Exynos 542x
> can be
> also replaced by runtime PM calls), Ulf mentioned that exactly similar
> pattern
> is used for some UX500 SoCs (STE). More will probably come once the
> feature is
> in, because for now such drivers simply forces runtime active state as a
> workaround or don't instantiate related power domains at all.
I agree with the above, but I'm also wondering about the folks that use
regmap internally to their own clock provider drivers. I guess they will
have the option to either do their own thing or use this framework-level
solution.
I've started reviewing the code itself and will respond to those mails
separately.
Thanks,
Mike
>
> It is not that uncommon to have runtime PM integrated in the framework
> (examples:
> mmc, scsi). Please not that this is optional for clock providers - if
> they don't
> enable runtime PM for the provided clock controller device during clock
> registration, the clock core will behave exactly the same way as now.
>
> >> When one wants to register clock controller, which make use of this feature, he
> >> has to:
> >> 1. Provide a struct device to the core when registering the provider.
> >> 2. Ensure to enable runtime PM for that device before registering clocks.
> >> 3. Make sure that the runtime PM status of the controller device reflects
> >> the HW state.
> > Third question: is there a case where more than one device is required?
> > Is is possible that a single pm_runtime_get call against a single device
> > will not be enough for some clk provider drivers? If so then this
> > solution does not scale very well and the clk provider driver will have
> > to implement this in the clk_ops callbacks (as I mentioned above in my
> > second question).
>
> This is a generic question about runtime PM. There are various methods to
> model hardware relations to control pm/runtime pm state of a set of devices:
> child-parent-bus relations (setting child to active state also activates a
> parent), gen_pd power domains and sub-domains and recently merged device pm
> links, which allows to model relations across the typical child-parent tree
> hierarchy. IMHO device core pm related framework provides enough features to
> solve the case when one needs to control more than one device - what is
> worth to mention - in all cases the client only need to call pm_runtime
> funtions on the ONE leaf device, everything else will be handled by pm core.
>
> > Fourth & final question: I'm under the impression that pm runtime calls
> > be be nested and re-enter, but I want to make sure (Ulf?). For instance
> > it is highly likely that this new feature would cause something like:
> >
> > pm_runtime_get() - called by random driver
> > -> clk_prepare_enable() - genpd enables functioal clocks
> > -> pm_runtime_get() - called by clk_pm_runtime_get in clk core
> > -> clk_prepare_enable() - genpd enables interface or bus clocks
> >
> > I guess this is safe from the pm_runtime_get/genpd perspective, but want
> > to make sure first.
>
> Yes, this will work fine after recent fixes. Tested with Exynos IIS ASoC
> driver (which is also a clock provider), which in turn is a client for
> Exynos Audio Subsystem clock provider.
>
> > Thanks,
>
> >> [...]
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
