From: sstabellini@kernel.org (Stefano Stabellini)
To: linux-arm-kernel@lists.infradead.org
Subject: [Xen-devel] [PATCH v4] xen/arm: Add a clock property
Date: Thu, 14 Jul 2016 11:25:21 +0100 (BST) [thread overview]
Message-ID: <alpine.DEB.2.10.1607141118490.26575@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <146844380895.73491.4867379517577413421@resonance>
On Wed, 13 Jul 2016, Michael Turquette wrote:
> Quoting Dirk Behme (2016-07-13 11:56:30)
> > On 13.07.2016 20:43, Stefano Stabellini wrote:
> > > On Wed, 13 Jul 2016, Dirk Behme wrote:
> > >> On 13.07.2016 00:26, Michael Turquette wrote:
> > >>> Quoting Dirk Behme (2016-07-12 00:46:45)
> > >>>> Clocks described by this property are reserved for use by Xen, and the OS
> > >>>> must not alter their state any way, such as disabling or gating a clock,
> > >>>> or modifying its rate. Ensuring this may impose constraints on parent
> > >>>> clocks or other resources used by the clock tree.
> > >>>
> > >>> Note that clk_prepare_enable will not prevent the rate from changing
> > >>> (clk_set_rate) or a parent from changing (clk_set_parent). The only way
> > >>> to do this currently would be to set the following flags on the effected
> > >>> clocks:
> > >>>
> > >>> CLK_SET_RATE_GATE
> > >>> CLK_SET_PARENT_GATE
> > >>
> > >>
> > >>
> > >> Regarding setting flags, I think we already talked about that. I think the
> > >> conclusion was that in our case its not possible to manipulate the flags in
> > >> the OS as this isn't intended to be done in cases like ours. Therefore no API
> > >> is exported for this.
> > >>
> > >> I.e. if we need to set these flags, we have to do that in Xen where we add the
> > >> clocks to the hypervisor node in the device tree. And not in the kernel patch
> > >> discussed here.
> > >
> > > These are internal Linux flags, aren't they?
> >
> >
> > I've been under the impression that you can set clock "flags" via the
> > device tree. Seems I need to re-check that ;)
>
> Right, you cannot set flags from the device tree. Also, setting these
> flags is done by the clock provider driver, not a consumer. Xen is the
> consumer.
Just for disambiguation, I guess you are referring to the Xen code in
Linux (such as arch/arm/xen/enlighten.c), right?
We have "Xen", the hypervisor, which runs before Linux and it is a
separate component running at EL2, and we have Xen support in Linux,
such as arch/arm/xen and drivers/xen. These are drivers to communicate
with the Xen hypervisor and other components in a Xen system.
In this case, the Xen hypervisor would be the owner of the clock. Xen
support in Linux, specifically arch/arm/xen/enlighten.c, only comes into
the picture to "warn" Linux that shouldn't really be messing with the
clocks. I guess that from a Linux clock driver point of view, it is a
consumer.
I hope that helps.
> > > They cannot be set in Xen.
> > >
> > > If the only way to make sure that clk_set_rate and clk_set_parent fail
> > > on one of the clocks "owned" by Xen is to set CLK_SET_RATE_GATE and
> > > CLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing
> > > a new internal Linux API.
> > >
> > >
> > >>> And then enable the clocks. All calls to clk_set_parent and clk_set_rate
> > >>> with those clocks in the path will fail, so long as they are prepared
> > >>> and enabled. This implementation detail is specific to Linux and
> > >>> definitely should not go into the binding.
> > >>>
> > >>>>
> > >>>> This property is used to proxy clocks for devices Xen has taken ownership
> > >>>> of, such as UARTs, for which the associated clock controller(s) remain
> > >>>> under the control of Dom0.
> > >>>
> > >>> I'm still not completely sure about this type of layering and whether or
> > >>> not it is sane. If you want Xen to manage the clock controller, AND you
> > >>> want Linux guests or dom0 to consume those clocks and manipulate them in
> > >>> other drivers, then this solution won't work.
> > >>>
> > >>> Regards,
> > >>> Mike
> > >>>
> > >>>>
> > >>>> Up to now, the workaround for this has been to use the Linux kernel
> > >>>> command line parameter 'clk_ignore_unused'. See Xen bug
> > >>>>
> > >>>> http://bugs.xenproject.org/xen/bug/45
> > >>>>
> > >>>> too.
> > >>>>
> > >>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > >>>> ---
> > >>>> Changes in v4: Switch to the xen.txt description proposed by Mark:
> > >>>> https://www.spinics.net/lists/arm-kernel/msg516158.html
> > >>>>
> > >>>> Changes in v3: Use the xen.txt description proposed by Michael. Thanks!
> > >>>>
> > >>>> Changes in v2: Drop the Linux implementation details like
> > >>>> clk_disable_unused
> > >>>> in xen.txt.
> > >>>>
> > >>>> Documentation/devicetree/bindings/arm/xen.txt | 12 +++++++
> > >>>> arch/arm/xen/enlighten.c | 47
> > >>>> +++++++++++++++++++++++++++
> > >>>> 2 files changed, 59 insertions(+)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/arm/xen.txt
> > >>>> b/Documentation/devicetree/bindings/arm/xen.txt
> > >>>> index c9b9321..437e50b 100644
> > >>>> --- a/Documentation/devicetree/bindings/arm/xen.txt
> > >>>> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> > >>>> @@ -17,6 +17,18 @@ the following properties:
> > >>>> A GIC node is also required.
> > >>>> This property is unnecessary when booting Dom0 using ACPI.
> > >>>>
> > >>>> +Optional properties:
> > >>>> +
> > >>>> +- clocks: a list of phandle + clock-specifier pairs
> > >>>> + Clocks described by this property are reserved for use by Xen, and the
> > >>>> + OS must not alter their state any way, such as disabling or gating a
> > >>>> + clock, or modifying its rate. Ensuring this may impose constraints on
> > >>>> + parent clocks or other resources used by the clock tree.
> > >>>> +
> > >>>> + Note: this property is used to proxy clocks for devices Xen has taken
> > >>>> + ownership of, such as UARTs, for which the associated clock
> > >>>> + controller(s) remain under the control of Dom0.
> > >>>> +
> > >>>> To support UEFI on Xen ARM virtual platforms, Xen populates the FDT
> > >>>> "uefi" node
> > >>>> under /hypervisor with following parameters:
> > >>>>
> > >>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > >>>> index 47acb36..5c546d0 100644
> > >>>> --- a/arch/arm/xen/enlighten.c
> > >>>> +++ b/arch/arm/xen/enlighten.c
> > >>>> @@ -24,6 +24,7 @@
> > >>>> #include <linux/of_fdt.h>
> > >>>> #include <linux/of_irq.h>
> > >>>> #include <linux/of_address.h>
> > >>>> +#include <linux/clk-provider.h>
> > >>>> #include <linux/cpuidle.h>
> > >>>> #include <linux/cpufreq.h>
> > >>>> #include <linux/cpu.h>
> > >>>> @@ -444,6 +445,52 @@ static int __init xen_pm_init(void)
> > >>>> }
> > >>>> late_initcall(xen_pm_init);
> > >>>>
> > >>>> +/*
> > >>>> + * Check if we want to register some clocks, that they
> > >>>> + * are not freed because unused by clk_disable_unused().
> > >>>> + * E.g. the serial console clock.
> > >>>> + */
> > >>>> +static int __init xen_arm_register_clks(void)
> > >>>> +{
> > >>>> + struct clk *clk;
> > >>>> + struct device_node *xen_node;
> > >>>> + unsigned int i, count;
> > >>>> + int ret = 0;
> > >>>> +
> > >>>> + xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
> > >>>> + if (!xen_node) {
> > >>>> + pr_err("Xen support was detected before, but it has
> > >>>> disappeared\n");
> > >>>> + return -EINVAL;
> > >>>> + }
> > >>>> +
> > >>>> + count = of_clk_get_parent_count(xen_node);
> > >>>> + if (!count)
> > >>>> + goto out;
> > >>>> +
> > >>>> + for (i = 0; i < count; i++) {
> > >>>> + clk = of_clk_get(xen_node, i);
> > >>>> + if (IS_ERR(clk)) {
> > >>>> + pr_err("Xen failed to register clock %i. Error:
> > >>>> %li\n",
> > >>>> + i, PTR_ERR(clk));
> > >>>> + ret = PTR_ERR(clk);
> > >>>> + goto out;
> > >>>> + }
> > >>>> +
> > >>>> + ret = clk_prepare_enable(clk);
> > >>>> + if (ret < 0) {
> > >>>> + pr_err("Xen failed to enable clock %i. Error:
> > >>>> %i\n",
> > >>>> + i, ret);
> > >>>> + goto out;
> > >>>> + }
> > >>>> + }
> > >>>> +
> > >>>> + ret = 0;
> > >>>> +
> > >>>> +out:
> > >>>> + of_node_put(xen_node);
> > >>>> + return ret;
> > >>>> +}
> > >>>> +late_initcall(xen_arm_register_clks);
> > >>>>
> > >>>> /* empty stubs */
> > >>>> void xen_arch_pre_suspend(void) { }
> > >>>> --
> > >>
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel at lists.xen.org
> > > https://lists.xen.org/xen-devel
> > >
> >
>
next prev parent reply other threads:[~2016-07-14 10:25 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-12 7:46 [PATCH v4] xen/arm: Add a clock property Dirk Behme
2016-07-12 22:26 ` Michael Turquette
2016-07-13 8:35 ` Dirk Behme
2016-07-13 18:43 ` Stefano Stabellini
2016-07-13 18:56 ` [Xen-devel] " Dirk Behme
2016-07-13 21:03 ` Michael Turquette
2016-07-14 6:31 ` Dirk Behme
2016-07-14 10:14 ` Julien Grall
2016-07-14 10:32 ` Dirk Behme
2016-07-14 10:38 ` Stefano Stabellini
2016-07-14 10:49 ` Dirk Behme
2016-07-14 15:55 ` Stefano Stabellini
2016-07-14 16:30 ` Dirk Behme
2016-07-14 17:14 ` Julien Grall
2016-07-15 7:53 ` Dirk Behme
2016-07-22 0:07 ` Michael Turquette
2016-07-22 1:16 ` Stefano Stabellini
2016-07-27 5:05 ` Dirk Behme
2016-07-28 11:17 ` Julien Grall
2016-07-28 14:35 ` Dirk Behme
2016-07-14 10:25 ` Stefano Stabellini [this message]
2016-07-13 18:35 ` Stefano Stabellini
2016-07-13 18:47 ` [Xen-devel] " Dirk Behme
2016-07-13 19:07 ` Stefano Stabellini
2016-07-14 6:11 ` Dirk Behme
2016-07-14 10:28 ` Stefano Stabellini
2016-07-20 9:43 ` Geert Uytterhoeven
2016-07-20 11:01 ` Julien Grall
2016-07-20 11:49 ` Geert Uytterhoeven
2016-07-20 12:10 ` Julien Grall
2016-07-20 12:46 ` Geert Uytterhoeven
2016-07-20 12:53 ` Dirk Behme
2016-07-20 13:21 ` Julien Grall
2016-07-22 0:14 ` Michael Turquette
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=alpine.DEB.2.10.1607141118490.26575@sstabellini-ThinkPad-X260 \
--to=sstabellini@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).