xen/arm: Add a clock property
Subsystems (4)
PATCH v4 1/1 xen/arm: Add a clock property
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.
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.
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.
---
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(+)
Documentation/devicetree/bindings/arm/xen.txt +12 -0
@@ -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:
arch/arm/xen/enlighten.c +47 -0
59 lines not on this page · raw message
Signature · 1 line
2.8.0
PATCH v4 1/1 xen/arm: Add a clock property
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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
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.
Quoted 4 lines
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
Quoted 113 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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 ) { }Signature · 1 line
2.8.0
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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.
Best regards
Dirk
Quoted 129 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 111 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-1@@ -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 ) { } --
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
Quoted 17 lines · Dirk Behme
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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
Quoted 8 lines
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? 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.
Quoted 137 lines
Quoted 136 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 118 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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 */
voidxen_arch_pre_suspend(void){ }
--
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 23 lines · Stefano Stabellini
Quoted 22 lines · Dirk Behme
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATERegarding 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.
Quoted 5 lines
Quoted 3 lines
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 ;)
Best regards
Dirk
Quoted 151 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
https://lists.xen.org/xen-devel
PATCH v4 1/1 xen/arm: Add a clock property
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 28 lines · Stefano Stabellini
Quoted 27 lines · Dirk Behme
Quoted 25 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATERegarding 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.
Quoted 5 lines
Quoted 1 line
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.
Regards,
Mike
Quoted 158 lines
Best regards
Dirk
Quoted 151 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
https://lists.xen.org/xen-devel
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 32 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 31 lines · Stefano Stabellini
Quoted 28 lines · Dirk Behme
Quoted 17 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATEQuoted 8 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
Quoted 143 lines
Quoted 143 lines
Quoted 143 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 135 lines
Quoted 135 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 117 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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 */
voidxen_arch_pre_suspend(void){ }
PATCH v4 1/1 xen/arm: Add a clock property
Hi Dirk,
Quoted 54 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 8 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
I think it is a good start, although I would like to see some rationale in the code and commit message about the behavior of the Linux with those clocks after this patch because it does not match the "contract".
Regards,
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 61 lines · Julien Grall
Hi Dirk,
Quoted 58 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
Quoted 8 lines
Quoted 4 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.I think it is a good start, although I would like to see some rationale in the code and commit message about the behavior of the Linux with those clocks after this patch because it does not match the "contract".
I'd be happy about any wording proposals :)
Best regards
Dirk
Quoted 61 lines · Julien Grall
Hi Dirk,
Quoted 58 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
Quoted 8 lines
Quoted 4 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.I think it is a good start, although I would like to see some rationale in the code and commit message about the behavior of the Linux with those clocks after this patch because it does not match the "contract".
I'd be happy about any wording proposals :)
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Hi Dirk,
Quoted 54 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 8 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
I think it is a good start, although I would like to see some rationale in the code and commit message about the behavior of the Linux with those clocks after this patch because it does not match the "contract".
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
In reply to Dirk BehmeQuoted 54 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
It might be a step in the right direction, but it doesn't really prevent clk_set_rate from changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.
Michael, do you have any suggestions on how it would be possible to set CLK_SET_RATE_GATE and CLK_SET_PARENT_GATE for those clocks in a proper way?
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 59 lines · Stefano Stabellini
Quoted 58 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regarding clk_set_rate from changing properties, too?
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
Quoted 77 lines · Dirk Behme
Quoted 67 lines · Stefano Stabellini
Quoted 66 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.
Quoted 7 lines
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "
clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regardingclk_set_ratefrom changing properties, too?
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding, one which will have to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient or if we need something different to solve the problem completely.
Once we understand that, I am happy to accept a partial implementation in Linux, as long as it is a step in the right direction. Does it make sense?
Quoted 27 lines
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 86 lines · Stefano Stabellini
Quoted 77 lines · Dirk Behme
Quoted 67 lines · Stefano Stabellini
Quoted 66 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Quoted 7 lines
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "
clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regardingclk_set_ratefrom changing properties, too?
Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient
Even if it's not sufficient, you can't change it.
Quoted 2 lines
or if we need something different to solve the
problem completely.
You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?
Quoted 34 lines
Once we understand that, I am happy to accept a partial implementation in Linux, as long as it is a step in the right direction. Does it make sense?
Quoted 27 lines
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
PATCH v4 1/1 xen/arm: Add a clock property
Hello,
Quoted 114 lines · Dirk Behme
Quoted 95 lines · Stefano Stabellini
Quoted 87 lines · Dirk Behme
Quoted 76 lines · Stefano Stabellini
Quoted 67 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 8 lines
Quoted 3 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we
are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.Quoted 4 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Let me ask then: Do we have a practical example where it's not sufficient
practically?Quoted 6 lines
To my understanding, Xen people have been happy with the "
clk_ignore_unused"
workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care
regardingclk_set_ratefrom changing properties, too?Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 7 lines
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficientEven if it's not sufficient, you can't change it.
I think you misunderstood Stefano's comment. Whilst the clock bindings is set in stone, the binding you are adding in this patch is not yet fixed. There is no requirement to follow what was defined in clock-bindings.txt. I agree it would be convenient, but as mentioned by Stefano this will need to be supported for a long time by Xen, Linux, and any other consumer (i.e BSD kernels). So we have to be careful on how it has been defined.
I would wait the answer of Michael on Stefano's question before taking any decision here.
Quoted 8 lines
Quoted 4 lines
or if we need something different to solve the
problem completely.You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
The Linux kernel is not the only consumer of the device tree bindings. This is also used by other OS such as FreeBSD where you might already be able (I have not actually checked) to forbid a user to change the clock rate.
Quoted 7 lines
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?
Very easy. What does prevent a driver to change the clock rate? Nothing but the flags mentioned by Michael. There are already drivers which modify the clock rate, thankfully those clocks are not shared with the UART for now.
But we cannot rule out that it will not be possible in the future. Think about a clock that would be used by another guest (I know it is still theoretical as we have not yet solved the problem).
Regards,
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 126 lines · Julien Grall
Hello,
Quoted 123 lines · Dirk Behme
Quoted 97 lines · Stefano Stabellini
Quoted 89 lines · Dirk Behme
Quoted 78 lines · Stefano Stabellini
Quoted 69 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 46 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 43 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
Quoted 1 line
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 ;)Quoted 5 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we
are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.Quoted 4 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Let me ask then: Do we have a practical example where it's not sufficient
practically?Quoted 6 lines
To my understanding, Xen people have been happy with the "
clk_ignore_unused"
workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care
regardingclk_set_ratefrom changing properties, too?Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient
Quoted 5 lines
Quoted 1 line
Even if it's not sufficient, you can't change it.
I think you misunderstood Stefano's comment. Whilst the clock bindings is set in stone, the binding you are adding in this patch is not yet fixed.
It has to be
a) identical what we pick from UART, timer, etc
b) compatible to the kernel's clock API
No?
With these two requirements I have some difficulties to imagine how it could be different to the clock binding from clock-bindings.txt?
Quoted 4 lines
There is no requirement to follow what was defined in clock-bindings.txt. I agree it would be convenient, but as mentioned by Stefano this will need to be supported for a long time by Xen, Linux, and any other consumer (i.e BSD kernels).
Sounds like an additional argument for clock-bindings.txt ;)
Quoted 5 lines
So we have to be careful on
how it has been defined.I would wait the answer of Michael on Stefano's question before taking any decision here.
Fine with me :)
Best regards
Dirk
Quoted 28 lines
Quoted 6 lines
Quoted 2 lines
or if we need something different to solve the
problem completely.You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
The Linux kernel is not the only consumer of the device tree bindings. This is also used by other OS such as FreeBSD where you might already be able (I have not actually checked) to forbid a user to change the clock rate.
Quoted 7 lines
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?Very easy. What does prevent a driver to change the clock rate? Nothing but the flags mentioned by Michael. There are already drivers which modify the clock rate, thankfully those clocks are not shared with the UART for now.
But we cannot rule out that it will not be possible in the future. Think about a clock that would be used by another guest (I know it is still theoretical as we have not yet solved the problem).
Quoted 126 lines · Julien Grall
Hello,
Quoted 123 lines · Dirk Behme
Quoted 97 lines · Stefano Stabellini
Quoted 89 lines · Dirk Behme
Quoted 78 lines · Stefano Stabellini
Quoted 69 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 46 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 43 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
Quoted 1 line
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 ;)Quoted 5 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we
are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.Quoted 4 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Let me ask then: Do we have a practical example where it's not sufficient
practically?Quoted 6 lines
To my understanding, Xen people have been happy with the "
clk_ignore_unused"
workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care
regardingclk_set_ratefrom changing properties, too?Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient
Quoted 5 lines
Quoted 1 line
Even if it's not sufficient, you can't change it.
I think you misunderstood Stefano's comment. Whilst the clock bindings is set in stone, the binding you are adding in this patch is not yet fixed.
It has to be
a) identical what we pick from UART, timer, etc
b) compatible to the kernel's clock API
No?
With these two requirements I have some difficulties to imagine how it could be different to the clock binding from clock-bindings.txt?
Quoted 4 lines
There is no requirement to follow what was defined in clock-bindings.txt. I agree it would be convenient, but as mentioned by Stefano this will need to be supported for a long time by Xen, Linux, and any other consumer (i.e BSD kernels).
Sounds like an additional argument for clock-bindings.txt ;)
Quoted 5 lines
So we have to be careful on
how it has been defined.I would wait the answer of Michael on Stefano's question before taking any decision here.
Fine with me :)
Best regards
Dirk
Quoted 20 lines
Quoted 6 lines
Quoted 2 lines
or if we need something different to solve the
problem completely.You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
The Linux kernel is not the only consumer of the device tree bindings. This is also used by other OS such as FreeBSD where you might already be able (I have not actually checked) to forbid a user to change the clock rate.
Quoted 7 lines
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?
Quoted 8 lines
Very easy. What does prevent a driver to change the clock rate? Nothing but the flags mentioned by Michael. There are already drivers which modify the clock rate, thankfully those clocks are not shared with the UART for now.
But we cannot rule out that it will not be possible in the future. Think about a clock that would be used by another guest (I know it is still theoretical as we have not yet solved the problem).
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Hello,
Quoted 114 lines · Dirk Behme
Quoted 95 lines · Stefano Stabellini
Quoted 87 lines · Dirk Behme
Quoted 76 lines · Stefano Stabellini
Quoted 67 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 8 lines
Quoted 3 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we
are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.Quoted 4 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Let me ask then: Do we have a practical example where it's not sufficient
practically?Quoted 6 lines
To my understanding, Xen people have been happy with the "
clk_ignore_unused"
workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care
regardingclk_set_ratefrom changing properties, too?Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 7 lines
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficientEven if it's not sufficient, you can't change it.
I think you misunderstood Stefano's comment. Whilst the clock bindings is set in stone, the binding you are adding in this patch is not yet fixed. There is no requirement to follow what was defined in clock-bindings.txt. I agree it would be convenient, but as mentioned by Stefano this will need to be supported for a long time by Xen, Linux, and any other consumer (i.e BSD kernels). So we have to be careful on how it has been defined.
I would wait the answer of Michael on Stefano's question before taking any decision here.
Quoted 8 lines
Quoted 4 lines
or if we need something different to solve the
problem completely.You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
The Linux kernel is not the only consumer of the device tree bindings. This is also used by other OS such as FreeBSD where you might already be able (I have not actually checked) to forbid a user to change the clock rate.
Quoted 7 lines
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?
Very easy. What does prevent a driver to change the clock rate? Nothing but the flags mentioned by Michael. There are already drivers which modify the clock rate, thankfully those clocks are not shared with the UART for now.
But we cannot rule out that it will not be possible in the future. Think about a clock that would be used by another guest (I know it is still theoretical as we have not yet solved the problem).
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Quoted 86 lines · Stefano Stabellini
Quoted 77 lines · Dirk Behme
Quoted 67 lines · Stefano Stabellini
Quoted 66 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.Quoted 7 lines
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "
clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regardingclk_set_ratefrom changing properties, too?
Quoted 4 lines
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding,
I don't think so.
We are just using the existing one
, pick it from other device tree nodes (e.g. serial, timer etc) and add it to the hypervisor node. And then use this existing one with the existing well defined clock API.
Quoted 4 lines
one which will have
to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient
Even if it's not sufficient, you can't change it.
Quoted 2 lines
or if we need something different to solve the
problem completely.
You might need anything additionally. E.g. an extension of the Linux kernel clock API to be able to modify the flags was proposed.
Best regards
Dirk
P.S.: I still would be interested if we do have a practical example where it's not sufficient practically?
Quoted 34 lines
Once we understand that, I am happy to accept a partial implementation in Linux, as long as it is a step in the right direction. Does it make sense?
Quoted 27 lines
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 77 lines · Dirk Behme
Quoted 67 lines · Stefano Stabellini
Quoted 66 lines · Dirk Behme
Quoted 49 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.
Quoted 7 lines
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "
clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regardingclk_set_ratefrom changing properties, too?
Let me premise that I appreciate what you are trying to achieve with this patch and I don't want to feature-creep it.
However we are defining a new Device Tree binding, one which will have to be supported for a long time by both Xen and Linux, so at the very least we need to have the full picture. We need to understand if the binding if sufficient or if we need something different to solve the problem completely.
Once we understand that, I am happy to accept a partial implementation in Linux, as long as it is a step in the right direction. Does it make sense?
Quoted 8 lines
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Quoted 19 lines
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 59 lines · Stefano Stabellini
Quoted 58 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
Quoted 7 lines
Quoted 3 lines
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete.
Let me ask then: Do we have a practical example where it's not sufficient practically?
To my understanding, Xen people have been happy with the "clk_ignore_unused" workaround since ~2 years, now [1]. And I think the "clk_ignore_unused" workaround does mainly the same like the patch discussed here. It doesn't care regarding clk_set_rate from changing properties, too?
While I agree that the patch theoretically incomplete, if nobody has a real world example I would think that from practical point of view it's sufficient in a first step.
If this is the case, I'd propose to fix the practical issue in a first step with a patch (this one) which is sufficient to fix the issues the Xen users have. And update the code for theoretical future issues in a second step.
Best regards
Dirk
[1] http://bugs.xenproject.org/xen/bug/45
Quoted 11 lines
We need to understand at least what it would take
to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
PATCH v4 1/1 xen/arm: Add a clock property
In reply to Stefano StabelliniQuoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 63 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
Quoted 8 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?
No, there is no way for a consumer to do that. The provider must do it.
Quoted 5 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use clk_set_rate_range() to "lock" in a rate. This is good because it is exactly what a clock consumer should do:
1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Is it correct that you would prefer some sort of never_touch_this_clk() api?
Regards,
Mike
Quoted 1 line
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
Quoted 69 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 68 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.
Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set CLK_SET_RATE_GATE and CLK_SET_PARENT_GATE for a given clock?
Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
Quoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 4 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?From my understading, yes,
never_touch_this_clk()would make things easier.
PATCH v4 1/1 xen/arm: Add a clock property
Hi Michael, Stefano and Julien,
Quoted 99 lines · Stefano Stabellini
Quoted 69 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 68 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
Quoted 14 lines
Quoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 4 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?From my understading, yes,
never_touch_this_clk()would make things easier.
Would it be somehow worth to wait for anything like this never_touch_this_clk() api? Or should we try to proceed with clk_prepare_enable() like done in this patch for the moment?
Best regards
Dirk
PATCH v4 1/1 xen/arm: Add a clock property
Hi Dirk,
Quoted 128 lines · Dirk Behme
Hi Michael, Stefano and Julien,
Quoted 125 lines · Stefano Stabellini
Quoted 82 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 81 lines · Dirk Behme
Quoted 66 lines · Michael Turquette
Quoted 49 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 8 lines
Quoted 3 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same valueQuoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 2 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
From my understading, yes,
never_touch_this_clk()would make things easier.Would it be somehow worth to wait for anything like this
never_touch_this_clk()api? Or should we try to proceed withclk_prepare_enable()like done in this patch for the moment?
I am not sure who will write the new api never_touch_this_clk(). Could you suggest an implementation based on the discussion?
Regards,
Quoted 5 lines
Best regards
Dirk
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 154 lines · Julien Grall
Hi Dirk,
Quoted 151 lines · Dirk Behme
Hi Michael, Stefano and Julien,
Quoted 146 lines · Stefano Stabellini
Quoted 89 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 88 lines · Dirk Behme
Quoted 71 lines · Michael Turquette
Quoted 57 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 49 lines · Stefano Stabellini
Quoted 48 lines · Dirk Behme
Quoted 46 lines · Michael Turquette
Quoted 22 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 10 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
Quoted 1 line
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 ;)Quoted 5 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Quoted 8 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take
to have a complete solution.Quoted 8 lines
Quoted 5 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper
way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 18 lines
Quoted 5 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and
clock-specifier pairs under the Xen node?Upon further reflection, I think that your clock consumer can probably
useclk_set_rate_range()to "lock" in a rate. This is good because it is
exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same valueQuoted 4 lines
The problem with this solution is that it requires the consumer to have
knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?Who is usually the component with knowledge of the clock rate to set? If
it's a device driver, then neither the Xen hypervisor, nor the Xen core
drivers in Linux would know anything about it. (Unless the clock rate is
specified on device tree via assigned-clock-rates of course.)Quoted 6 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()
api?From my understading, yes,
never_touch_this_clk()would make things easier.
Quoted 6 lines
Quoted 3 lines
Would it be somehow worth to wait for anything like this
never_touch_this_clk()api? Or should we try to proceed withclk_prepare_enable()like done in this patch for the moment?I am not sure who will write the new api
never_touch_this_clk(). Could you suggest an implementation based on the discussion?
As this was a proposal from Michael, I'm hoping for Michael here, somehow ;) At least for a hint if anything like never_touch_this_clk() would be realistic to get accepted. And if so, how this could look like.
If this is unrealistic, I think we should go the proposed clk_prepare_enable() way, as it seems this is the best we could do at the moment without never_touch_this_clk().
Best regards
Dirk
Quoted 154 lines · Julien Grall
Hi Dirk,
Quoted 151 lines · Dirk Behme
Hi Michael, Stefano and Julien,
Quoted 146 lines · Stefano Stabellini
Quoted 89 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 88 lines · Dirk Behme
Quoted 71 lines · Michael Turquette
Quoted 57 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 49 lines · Stefano Stabellini
Quoted 48 lines · Dirk Behme
Quoted 46 lines · Michael Turquette
Quoted 22 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 10 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
Quoted 1 line
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 ;)Quoted 5 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Quoted 8 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take
to have a complete solution.Quoted 8 lines
Quoted 5 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper
way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 18 lines
Quoted 5 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and
clock-specifier pairs under the Xen node?Upon further reflection, I think that your clock consumer can probably
useclk_set_rate_range()to "lock" in a rate. This is good because it is
exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same valueQuoted 4 lines
The problem with this solution is that it requires the consumer to have
knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?Who is usually the component with knowledge of the clock rate to set? If
it's a device driver, then neither the Xen hypervisor, nor the Xen core
drivers in Linux would know anything about it. (Unless the clock rate is
specified on device tree via assigned-clock-rates of course.)Quoted 6 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()
api?From my understading, yes,
never_touch_this_clk()would make things easier.
Quoted 6 lines
Quoted 3 lines
Would it be somehow worth to wait for anything like this
never_touch_this_clk()api? Or should we try to proceed withclk_prepare_enable()like done in this patch for the moment?I am not sure who will write the new api
never_touch_this_clk(). Could you suggest an implementation based on the discussion?
As this was a proposal from Michael, I'm hoping for Michael here, somehow ;) At least for a hint if anything like never_touch_this_clk() would be realistic to get accepted. And if so, how this could look like.
If this is unrealistic, I think we should go the proposed clk_prepare_enable() way, as it seems this is the best we could do at the moment without never_touch_this_clk().
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Hi Dirk,
Quoted 128 lines · Dirk Behme
Hi Michael, Stefano and Julien,
Quoted 125 lines · Stefano Stabellini
Quoted 82 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 81 lines · Dirk Behme
Quoted 66 lines · Michael Turquette
Quoted 49 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 48 lines · Stefano Stabellini
Quoted 45 lines · Dirk Behme
Quoted 36 lines · Michael Turquette
Quoted 20 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 9 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 6 lines
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?
Quoted 8 lines
Quoted 3 lines
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.Ok, thanks, then I think we can forget about using flags for the issue we are
discussing here.Best regards
Quoted 7 lines
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here,
then? From the discussion until here, it sounds to me that it's the best
option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same valueQuoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 2 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
From my understading, yes,
never_touch_this_clk()would make things easier.Would it be somehow worth to wait for anything like this
never_touch_this_clk()api? Or should we try to proceed withclk_prepare_enable()like done in this patch for the moment?
I am not sure who will write the new api never_touch_this_clk(). Could you suggest an implementation based on the discussion?
Regards,
Quoted 5 lines
Best regards
Dirk
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Hi Michael, Stefano and Julien,
Quoted 99 lines · Stefano Stabellini
Quoted 69 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 68 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor a given clock?Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
Quoted 14 lines
Quoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 4 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?From my understading, yes,
never_touch_this_clk()would make things easier.
Would it be somehow worth to wait for anything like this never_touch_this_clk() api? Or should we try to proceed with clk_prepare_enable() like done in this patch for the moment?
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 69 lines · Michael Turquette
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 68 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.
Quoted 5 lines
Quoted 3 lines
Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?No, there is no way for a consumer to do that. The provider must do it.
All right. But could we design a new device tree binding which the Xen hypervisor would use to politely ask the clock provider in Linux to set CLK_SET_RATE_GATE and CLK_SET_PARENT_GATE for a given clock?
Xen would have to modify the DTB before booting Linux with the new binding.
Quoted 15 lines
Quoted 4 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use
clk_set_rate_range()to "lock" in a rate. This is good because it is exactly what a clock consumer should do:1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
Quoted 3 lines
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Who is usually the component with knowledge of the clock rate to set? If it's a device driver, then neither the Xen hypervisor, nor the Xen core drivers in Linux would know anything about it. (Unless the clock rate is specified on device tree via assigned-clock-rates of course.)
Quoted 2 lines
Is it correct that you would prefer some sort of
never_touch_this_clk()api?
From my understading, yes, never_touch_this_clk() would make things easier.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoting Stefano Stabellini (2016-07-14 03:38:04)
Quoted 63 lines · Dirk Behme
Quoted 54 lines · Michael Turquette
Quoted 42 lines
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
Quoted 8 lines
It might be a step in the right direction, but it doesn't really prevent
clk_set_ratefrom changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.Michael, do you have any suggestions on how it would be possible to set
CLK_SET_RATE_GATEandCLK_SET_PARENT_GATEfor those clocks in a proper way?
No, there is no way for a consumer to do that. The provider must do it.
Quoted 5 lines
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
Upon further reflection, I think that your clock consumer can probably use clk_set_rate_range() to "lock" in a rate. This is good because it is exactly what a clock consumer should do:
1) get the clk
2) enable the clk
3) set the required rate for the clock
4) set rate range constraints, or conversely,
5) lock in an exact rate; set the min/max rate to the same value
The problem with this solution is that it requires the consumer to have knowledge of the rates that it wants for that clock, which I guess is something that Linux kernels in a Xen setup do not want/need?
Is it correct that you would prefer some sort of never_touch_this_clk() api?
Regards,
Mike
Quoted 1 line
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 54 lines · Dirk Behme
Quoted 42 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 41 lines · Stefano Stabellini
Quoted 38 lines · Dirk Behme
Quoted 30 lines · Michael Turquette
Quoted 16 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 7 lines
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_enablewill 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:Quoted 2 lines
CLK_SET_RATE_GATE
CLK_SET_PARENT_GATERegarding 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.Quoted 5 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Quoted 7 lines
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
It might be a step in the right direction, but it doesn't really prevent clk_set_rate from changing properties of a clock owned by Xen. This patch is incomplete. We need to understand at least what it would take to have a complete solution.
Michael, do you have any suggestions on how it would be possible to set CLK_SET_RATE_GATE and CLK_SET_PARENT_GATE for those clocks in a proper way?
Like you wrote, I would imagine it needs to be done by the clock provider driver. Maybe to do that, it would be easier to have a new device tree property on the clock node, rather than listing phandle and clock-specifier pairs under the Xen node?
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 32 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 31 lines · Stefano Stabellini
Quoted 28 lines · Dirk Behme
Quoted 17 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATEQuoted 8 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Ok, thanks, then I think we can forget about using flags for the issue we are discussing here.
Best regards
Dirk
P.S.: Would it be an option to merge the v4 patch we are discussing here, then? From the discussion until here, it sounds to me that it's the best option we have at the moment. Maybe improving it in the future, then.
Quoted 141 lines
Quoted 141 lines
Quoted 141 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
In reply to Michael TurquetteQuoted 32 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 31 lines · Stefano Stabellini
Quoted 28 lines · Dirk Behme
Quoted 17 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATEQuoted 8 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Quoted 153 lines
Quoted 152 lines
Quoted 151 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 32 lines · Michael Turquette
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 31 lines · Stefano Stabellini
Quoted 28 lines · Dirk Behme
Quoted 17 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATEQuoted 8 lines
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?
Quoted 6 lines
Quoted 2 lines
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.
Quoted 146 lines
Quoted 146 lines
Quoted 146 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 137 lines
Quoted 136 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 118 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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 */
voidxen_arch_pre_suspend(void){ }
--
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoting Dirk Behme (2016-07-13 11:56:30)
Quoted 28 lines · Stefano Stabellini
Quoted 27 lines · Dirk Behme
Quoted 25 lines · Michael Turquette
Quoted 6 lines
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATERegarding 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.
Quoted 5 lines
Quoted 1 line
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.
Regards,
Mike
Quoted 158 lines
Best regards
Dirk
Quoted 151 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 23 lines · Stefano Stabellini
Quoted 22 lines · Dirk Behme
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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_GATERegarding 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.
Quoted 5 lines
Quoted 3 lines
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 ;)
Best regards
Dirk
Quoted 146 lines
They cannot be set in Xen.
If the only way to make sure that
clk_set_rateandclk_set_parentfail on one of the clocks "owned" by Xen is to setCLK_SET_RATE_GATEandCLK_SET_PARENT_GATE, we'll have to do that. Even if it means introducing a new internal Linux API.Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--
Quoted 5 lines
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 17 lines · Dirk Behme
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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
Quoted 8 lines
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? 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.
Quoted 133 lines
Quoted 133 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 115 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
/* empty stubs */
voidxen_arch_pre_suspend(void){ }
--
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 6 lines · Michael Turquette
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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.
Quoted 7 lines
Note that
clk_prepare_enablewill 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.
Best regards
Dirk
Quoted 126 lines
And then enable the clocks. All calls to
clk_set_parentandclk_set_ratewith 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.Quoted 4 lines
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,
MikeQuoted 108 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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);
Quoted 3 lines
arch/arm/xen/enlighten.c static int __init xen_pm_init(void)
Quoted 3 lines
arch/arm/xen/enlighten.c +0
-1/* empty stubs */ void xen_arch_pre_suspend (void ) { } --
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoting Dirk Behme (2016-07-12 00:46:45)
Quoted 4 lines
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
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.
Quoted 4 lines
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
Quoted 113 lines
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +47
-0@@ -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 ) { }Signature · 1 line
2.8.0
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
In reply to Dirk BehmeQuoted 76 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +8
-0@@ -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 ) +{
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 464
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+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 ; + }
Given that this is a late initcall, the following should work:
if (!xen_domain())
return -ENODEV;PATCH v4 1/1 xen/arm: Add a clock property
Quoted 90 lines · Stefano Stabellini
Quoted 76 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +8
-0@@ -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 ) +{arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 464
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 2 lines
if (!xen_domain())
return -ENODEV;Hmm, sorry, "should work" for what?
We need the xen_node from device tree, anyway.
Best regards
Dirk
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
Quoted 96 lines · Dirk Behme
Quoted 95 lines · Stefano Stabellini
Quoted 81 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 5 lines
Quoted 2 lines
if (!xen_domain()) return -ENODEV;Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.
In that case, what don't you just use the global xen_node in this file?
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 101 lines · Stefano Stabellini
Quoted 100 lines · Dirk Behme
Quoted 97 lines · Stefano Stabellini
Quoted 81 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
if (!xen_domain()) return -ENODEV;
Quoted 7 lines
Quoted 1 line
Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.In that case, what don't you just use the global
xen_nodein this file?
With the recent code there is no global xen_node any more:
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02878.html
-- cut --
The code in this file has changed quite a lot with the support of ACPI. For instance "xen_node" is not anymore a global variable. Can you rebase your rework on top of the branch for-linus-4.8?
You can find the branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
-- cut --
Best regards
Dirk
Stefano Stabellini
maintainerPATCH v4 1/1 xen/arm: Add a clock property
Quoted 114 lines · Dirk Behme
Quoted 113 lines · Stefano Stabellini
Quoted 104 lines · Dirk Behme
Quoted 103 lines · Stefano Stabellini
Quoted 89 lines · Dirk Behme
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.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.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +4
-0@@ -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, andthe
+ OS must not alter their state any way, such as disabling or gating a
+ clock, or modifying its rate. Ensuring this may impose constraints onDocumentation/devicetree/bindings/arm/xen.txt +3
-0+ parent clocks or other resources used by the clock tree. + + Note: this property is used to proxy clocks for devices Xen hastaken
Documentation/devicetree/bindings/arm/xen.txt +3
-0+ 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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 5 lines
Quoted 2 lines
if (!xen_domain()) return -ENODEV;Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.In that case, what don't you just use the global
xen_nodein this file?With the recent code there is no global
xen_nodeany more:https://lists.xen.org/archives/html/xen-devel/2016-06/msg02878.html
Ops, I was looking at upstream. I forgot about those.
Stefano Stabellini
maintainerQuoted 114 lines · Dirk Behme
Quoted 113 lines · Stefano Stabellini
Quoted 104 lines · Dirk Behme
Quoted 103 lines · Stefano Stabellini
Quoted 89 lines · Dirk Behme
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.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.Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +4
-0@@ -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, andthe
+ OS must not alter their state any way, such as disabling or gating a
+ clock, or modifying its rate. Ensuring this may impose constraints onDocumentation/devicetree/bindings/arm/xen.txt +3
-0+ parent clocks or other resources used by the clock tree. + + Note: this property is used to proxy clocks for devices Xen hastaken
Documentation/devicetree/bindings/arm/xen.txt +3
-0+ 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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 5 lines
Quoted 2 lines
if (!xen_domain()) return -ENODEV;Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.In that case, what don't you just use the global
xen_nodein this file?With the recent code there is no global
xen_nodeany more:https://lists.xen.org/archives/html/xen-devel/2016-06/msg02878.html
Ops, I was looking at upstream. I forgot about those.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 101 lines · Stefano Stabellini
Quoted 100 lines · Dirk Behme
Quoted 97 lines · Stefano Stabellini
Quoted 81 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
if (!xen_domain()) return -ENODEV;
Quoted 7 lines
Quoted 1 line
Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.In that case, what don't you just use the global
xen_nodein this file?
With the recent code there is no global xen_node any more:
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02878.html
-- cut --
The code in this file has changed quite a lot with the support of ACPI. For instance "xen_node" is not anymore a global variable. Can you rebase your rework on top of the branch for-linus-4.8?
You can find the branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
-- cut --
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 96 lines · Dirk Behme
Quoted 95 lines · Stefano Stabellini
Quoted 81 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +9
-0@@ -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;arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 465
Quoted 10 lines
arch/arm/xen/enlighten.c +9
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 5 lines
Quoted 2 lines
if (!xen_domain()) return -ENODEV;Hmm, sorry, "should work" for what?
As a Xen check, if (!xen_domain()) is the common pattern.
Quoted 1 line
We need the
xen_nodefrom device tree, anyway.
In that case, what don't you just use the global xen_node in this file?
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 90 lines · Stefano Stabellini
Quoted 76 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +8
-0@@ -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 ) +{arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 464
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+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 ; + }Given that this is a late initcall, the following should work:
Quoted 2 lines
if (!xen_domain())
return -ENODEV;Hmm, sorry, "should work" for what?
We need the xen_node from device tree, anyway.
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Stefano Stabellini
maintainerQuoted 76 lines · Dirk Behme
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.
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.
Up to now, the workaround for this has been to use the Linux kernel command line parameter '
clk_ignore_unused'. See Xen bughttp://bugs.xenproject.org/xen/bug/45
too.
Signed-off-by Dirk Behme <[email protected]>---
Changes in v4: Switch to the xen.txt description proposed by Mark:https://www.spinics.net/lists/arm-kernel/msg516158.htmlChanges 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(+)Documentation/devicetree/bindings/arm/xen.txt +12
-0@@ -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:arch/arm/xen/enlighten.c +8
-0@@ -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 ) +{
arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 464
Quoted 10 lines
arch/arm/xen/enlighten.c +10
-0+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 ; + }
Given that this is a late initcall, the following should work:
if (!xen_domain())
return -ENODEV;_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
PATCH v4 1/1 xen/arm: Add a clock property
In reply to Dirk BehmeHi Dirk,
Quoted 8 lines · Dirk Behme
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.
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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT? Then the (virtual) serial device driver can get and enable the clock?
Alternatively, you can add a (virtual) clock controller, and power-domains and clock properties to all affected devices (I assume there can be others, besides virtual UARTs?), and let it be handled by Runtime PM, without the (virtual) device drivers having to care about clocks at all.
Thanks!
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
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 1 line · Geert Uytterhoeven
Hi Dirk,
Hi Geert,
Quoted 6 lines · Dirk Behme
Quoted 5 lines
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.
Quoted 8 lines
Quoted 3 lines
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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT? Then the (virtual) serial device driver can get and enable the clock?
There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Quoted 5 lines
Alternatively, you can add a (virtual) clock controller, and power-domains and clock properties to all affected devices (I assume there can be others, besides virtual UARTs?), and let it be handled by Runtime PM, without the (virtual) device drivers having to care about clocks at all.
I am not sure to understand here. The problem is not because we provide virtual device to DOM0 but because the physical devices will be completely hidden (i.e remove from the DT) from DOM0.
Those devices will be removed from the Device Tree and may not have a corresponding virtual device. So we cannot replicate the clocks property in the device.
In a previous mail [1], Stefano suggested to add a new property for the clock to mention that the clock should not changed (i.e rate, disable...). How would that fit?
Regards,
[1]
https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01614.html
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Hi Julien,
Quoted 14 lines · Julien Grall
Quoted 13 lines · Geert Uytterhoeven
Quoted 9 lines · Dirk Behme
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.
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.
Quoted 8 lines
Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Why is it removed?
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
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 19 lines · Geert Uytterhoeven
Hi Julien,
Quoted 16 lines · Julien Grall
Quoted 15 lines · Geert Uytterhoeven
Quoted 9 lines · Dirk Behme
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.
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 not familiar with using XEN at all, but I'm a bit puzzled...
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
Regards,
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Hi Julien,
Quoted 24 lines · Julien Grall
Quoted 23 lines · Geert Uytterhoeven
Quoted 21 lines · Julien Grall
Quoted 19 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
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
PATCH v4 1/1 xen/arm: Add a clock property
Quoted 33 lines · Geert Uytterhoeven
Hi Julien,
Quoted 30 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 17 lines · Julien Grall
Quoted 16 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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 not familiar with using XEN at all, but I'm a bit puzzled...
Quoted 7 lines
Quoted 3 lines
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Why is it removed?
Quoted 7 lines
Quoted 2 lines
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Yes, we are trying this with "moving" the clocks of the removed nodes to the Xen hypervisor node:
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02605.html
The question discussed here is how to deal with these clocks properly in Linux, then.
Best regards
Dirk
Quoted 33 lines · Geert Uytterhoeven
Hi Julien,
Quoted 30 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 17 lines · Julien Grall
Quoted 16 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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 not familiar with using XEN at all, but I'm a bit puzzled...
Quoted 7 lines
Quoted 3 lines
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Why is it removed?
Quoted 7 lines
Quoted 2 lines
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Yes, we are trying this with "moving" the clocks of the removed nodes to the Xen hypervisor node:
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02605.html
The question discussed here is how to deal with these clocks properly in Linux, then.
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
PATCH v4 1/1 xen/arm: Add a clock property
In reply to Geert UytterhoevenQuoted 1 line · Geert Uytterhoeven
Hi Julien,
Hello Geert,
Quoted 29 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 27 lines
Quoted 24 lines · Julien Grall
Quoted 14 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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.Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Xen does not disable the clock(s). The problem is Linux thinks the clock is not used and therefore will disable the clock.
Actually Xen does not have any knowledge how to handle clocks (i.e setting rate, enabling/disabling). I would recommend you to read the binding description in the patch and not the implementation which is IHMO misleading.
Quoted 3 lines
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Xen is removing the node from the DT. Not removing the node will not change the problem. Linux is disabling the clock because there is no driver which used those clocks.
As the clock subsystem will disable any unused clocks (from Linux point of view), the UART will get disabled and the console will be lost.
What we want to achieve with this patch is to let Linux knows that this clock is in use by someone else (hypervisor or guest) and:
* If the clock is not shared: don't disable it
* If the clock is shared: don't change the rate
Quoted 3 lines
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
I don't have any knowledge on the Renesas SH/R-Mobile. However, we expect some cooperation between DOM0 and Xen to handle the power.
For instance managing the clocks in Xen would require to implement clock driver for each SOC because it does not seem to have a generic way to do it.
Given that Linux already knows a lot about the clock, we want to hand over the clock control to the hardware domain (i.e dom0). This means that we have to find a way to cooperate with it to keep enable clocks that we care about.
Regards,
Signature · 1 line
Julien Grall
PATCH v4 1/1 xen/arm: Add a clock property
Quoting Julien Grall (2016-07-20 06:21:45)
Quoted 59 lines · Geert Uytterhoeven
Quoted 1 line
Hi Julien,
Hello Geert,
Quoted 29 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 27 lines
Quoted 24 lines · Julien Grall
Quoted 14 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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.Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Xen does not disable the clock(s). The problem is Linux thinks the clock is not used and therefore will disable the clock.
Actually Xen does not have any knowledge how to handle clocks (i.e setting rate, enabling/disabling). I would recommend you to read the binding description in the patch and not the implementation which is IHMO misleading.
Quoted 3 lines
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Xen is removing the node from the DT. Not removing the node will not change the problem. Linux is disabling the clock because there is no driver which used those clocks.
Quoted 7 lines
As the clock subsystem will disable any unused clocks (from Linux point of view), the UART will get disabled and the console will be lost.
What we want to achieve with this patch is to let Linux knows that this clock is in use by someone else (hypervisor or guest) and:
* If the clock is not shared: don't disable it
* If the clock is shared: don't change the rate
Thanks for the explanation above. It helped clarify things for me.
So, can you have some sort of dummy driver in Linux that claims the resources needed by Xen and calls the necessary Linux apis to insure that Xen's needs are satisfied? As I mentioned in my previous reply from a couple of minutes ago:
struct clk *clk = clk_get(xen_dev, ...);clk_prepare_enable(clk);clk_set_rate_range(...);
I'm not sure where the rate info should come from, but this represents a correct use of clk consumer apis.
Regards,
Mike
Quoted 20 lines
Quoted 4 lines
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
I don't have any knowledge on the Renesas SH/R-Mobile. However, we expect some cooperation between DOM0 and Xen to handle the power.
For instance managing the clocks in Xen would require to implement clock driver for each SOC because it does not seem to have a generic way to do it.
Given that Linux already knows a lot about the clock, we want to hand over the clock control to the hardware domain (i.e dom0). This means that we have to find a way to cooperate with it to keep enable clocks that we care about.
Regards,
Signature · 1 line
Julien Grall
Quoting Julien Grall (2016-07-20 06:21:45)
Quoted 59 lines · Geert Uytterhoeven
Quoted 1 line
Hi Julien,
Hello Geert,
Quoted 29 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 27 lines
Quoted 24 lines · Julien Grall
Quoted 14 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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.Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Xen does not disable the clock(s). The problem is Linux thinks the clock is not used and therefore will disable the clock.
Actually Xen does not have any knowledge how to handle clocks (i.e setting rate, enabling/disabling). I would recommend you to read the binding description in the patch and not the implementation which is IHMO misleading.
Quoted 3 lines
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Xen is removing the node from the DT. Not removing the node will not change the problem. Linux is disabling the clock because there is no driver which used those clocks.
Quoted 7 lines
As the clock subsystem will disable any unused clocks (from Linux point of view), the UART will get disabled and the console will be lost.
What we want to achieve with this patch is to let Linux knows that this clock is in use by someone else (hypervisor or guest) and:
* If the clock is not shared: don't disable it
* If the clock is shared: don't change the rate
Thanks for the explanation above. It helped clarify things for me.
So, can you have some sort of dummy driver in Linux that claims the resources needed by Xen and calls the necessary Linux apis to insure that Xen's needs are satisfied? As I mentioned in my previous reply from a couple of minutes ago:
struct clk *clk = clk_get(xen_dev, ...);clk_prepare_enable(clk);clk_set_rate_range(...);
I'm not sure where the rate info should come from, but this represents a correct use of clk consumer apis.
Regards,
Mike
Quoted 11 lines
Quoted 4 lines
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
I don't have any knowledge on the Renesas SH/R-Mobile. However, we expect some cooperation between DOM0 and Xen to handle the power.
For instance managing the clocks in Xen would require to implement clock driver for each SOC because it does not seem to have a generic way to do it.
Quoted 9 lines
Given that Linux already knows a lot about the clock, we want to hand over the clock control to the hardware domain (i.e dom0). This means that we have to find a way to cooperate with it to keep enable clocks that we care about.
Regards,
Signature · 1 line
Julien Grall
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 1 line · Geert Uytterhoeven
Hi Julien,
Hello Geert,
Quoted 29 lines · Julien Grall
Quoted 28 lines · Geert Uytterhoeven
Quoted 27 lines
Quoted 24 lines · Julien Grall
Quoted 14 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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.Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Xen does not disable the clock(s). The problem is Linux thinks the clock is not used and therefore will disable the clock.
Actually Xen does not have any knowledge how to handle clocks (i.e setting rate, enabling/disabling). I would recommend you to read the binding description in the patch and not the implementation which is IHMO misleading.
Quoted 3 lines
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
Xen is removing the node from the DT. Not removing the node will not change the problem. Linux is disabling the clock because there is no driver which used those clocks.
As the clock subsystem will disable any unused clocks (from Linux point of view), the UART will get disabled and the console will be lost.
What we want to achieve with this patch is to let Linux knows that this clock is in use by someone else (hypervisor or guest) and:
* If the clock is not shared: don't disable it
* If the clock is shared: don't change the rate
Quoted 3 lines
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
I don't have any knowledge on the Renesas SH/R-Mobile. However, we expect some cooperation between DOM0 and Xen to handle the power.
For instance managing the clocks in Xen would require to implement clock driver for each SOC because it does not seem to have a generic way to do it.
Given that Linux already knows a lot about the clock, we want to hand over the clock control to the hardware domain (i.e dom0). This means that we have to find a way to cooperate with it to keep enable clocks that we care about.
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Hi Julien,
Quoted 24 lines · Julien Grall
Quoted 23 lines · Geert Uytterhoeven
Quoted 21 lines · Julien Grall
Quoted 19 lines · Geert Uytterhoeven
Quoted 11 lines · Dirk Behme
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.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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
There is no DT node for the Xen console (hvc). The UART used by Xen will be
completely removed from the Device tree.Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
IMHO then it's Xen's responsability to make sure not to disable the clock(s).
Who removes the device node from the DT? If Xen, can't it just remember which clocks were present in removed device nodes?
What to do on SoCs where the serial device is part of a power area (e.g. Renesas SH/R-Mobile)? Who will make sure it is not powered down?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 19 lines · Geert Uytterhoeven
Hi Julien,
Quoted 16 lines · Julien Grall
Quoted 15 lines · Geert Uytterhoeven
Quoted 9 lines · Dirk Behme
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.
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 not familiar with using XEN at all, but I'm a bit puzzled...
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Why is it removed?
Because the device is used exclusively by Xen and DOM0 should not touch it at all (IRQs and MMIOs are not mapped).
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Hi Julien,
Quoted 14 lines · Julien Grall
Quoted 13 lines · Geert Uytterhoeven
Quoted 9 lines · Dirk Behme
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.
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.
Quoted 8 lines
Quoted 5 lines
I'm not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT?
Then the (virtual) serial device driver can get and enable the clock?There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Why is it removed?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Quoted 1 line · Geert Uytterhoeven
Hi Dirk,
Hi Geert,
Quoted 6 lines · Dirk Behme
Quoted 5 lines
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.
Quoted 8 lines
Quoted 3 lines
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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT? Then the (virtual) serial device driver can get and enable the clock?
There is no DT node for the Xen console (hvc). The UART used by Xen will be completely removed from the Device tree.
Quoted 5 lines
Alternatively, you can add a (virtual) clock controller, and power-domains and clock properties to all affected devices (I assume there can be others, besides virtual UARTs?), and let it be handled by Runtime PM, without the (virtual) device drivers having to care about clocks at all.
I am not sure to understand here. The problem is not because we provide virtual device to DOM0 but because the physical devices will be completely hidden (i.e remove from the DT) from DOM0.
Those devices will be removed from the Device Tree and may not have a corresponding virtual device. So we cannot replicate the clocks property in the device.
In a previous mail [1], Stefano suggested to add a new property for the clock to mention that the clock should not changed (i.e rate, disable...). How would that fit?
Regards,
[1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01614.html
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Hi Dirk,
Quoted 8 lines · Dirk Behme
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.
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 not familiar with using XEN at all, but I'm a bit puzzled...
Can't you just add a clocks property to the (virtual) serial device node in DT? Then the (virtual) serial device driver can get and enable the clock?
Alternatively, you can add a (virtual) clock controller, and power-domains and clock properties to all affected devices (I assume there can be others, besides virtual UARTs?), and let it be handled by Runtime PM, without the (virtual) device drivers having to care about clocks at all.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel