* [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
@ 2016-06-21 10:15 Dirk Behme
2016-06-21 10:20 ` Dirk Behme
2016-06-22 15:58 ` Julien Grall
0 siblings, 2 replies; 6+ messages in thread
From: Dirk Behme @ 2016-06-21 10:15 UTC (permalink / raw)
To: xen-devel, Julien Grall, Stefano Stabellini; +Cc: Dirk Behme
Some clocks might be used by Xen (drivers) and not by the Linux kernel. If
these are not registered by the Linux kernel, they might be disabled by the
Linux kernel's clk_disable_unused() as the kernel doesn't know that
they are used (by Xen drivers). The clock of the serial console handled by
Xen is one example for this. It might be disabled by clk_disable_unused() which
stops the whole serial output, even from Xen, then.
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.
To fix this, add the clocks used by Xen to the hypervisor node. The Linux
kernel has to register the clocks from the hypervisor node, then.
Therefore, collect all clocks from nodes used by Xen. These are marked
with DOMID_XEN. Afterwards, add a 'clocks' node to the hypervisor node
containing all these clocks. The Linux kernel can register all these clocks,
preventing them from being disabled, then.
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
xen/arch/arm/domain_build.c | 20 ++++++++++++++++++++
xen/arch/arm/kernel.h | 7 +++++++
2 files changed, 27 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2e4c295..fccf87e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -657,6 +657,10 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
if ( res )
return res;
+ res = fdt_property(fdt, "clocks", kinfo->clk.dtclocks, kinfo->clk.cnt);
+ if ( res )
+ return res;
+
res = fdt_end_node(fdt);
return res;
@@ -1213,9 +1217,11 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
{ /* sentinel */ },
};
struct dt_device_node *child;
+ unsigned int len;
int res;
const char *name;
const char *path;
+ const char *clocks;
path = dt_node_full_name(node);
@@ -1246,6 +1252,20 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
if ( dt_device_used_by(node) == DOMID_XEN )
{
DPRINT(" Skip it (used by Xen)\n");
+
+ /*
+ * Remember the clock used by the skipped node
+ * We add it later to the hypervisor node to make the
+ * Linux kernel aware of its usage
+ */
+ clocks = dt_get_property(node, "clocks", &len);
+ if ( kinfo->clk.cnt + len >= MAX_DT_CLOCKS ) {
+ printk("Failed to remember the clock node of %s\n", path);
+ printk("Use the Linux kernel command 'clk_ignore_unused'\n");
+ return 0;
+ }
+ memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
+ kinfo->clk.cnt += len;
return 0;
}
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index c1b07d4..527b279 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -10,6 +10,12 @@
#include <xen/device_tree.h>
#include <asm/setup.h>
+#define MAX_DT_CLOCKS 256
+struct dtclk {
+ unsigned char dtclocks[MAX_DT_CLOCKS];
+ unsigned int cnt;
+};
+
struct kernel_info {
#ifdef CONFIG_ARM_64
enum domain_type type;
@@ -18,6 +24,7 @@ struct kernel_info {
void *fdt; /* flat device tree */
paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
struct meminfo mem;
+ struct dtclk clk;
/* kernel entry point */
paddr_t entry;
--
2.8.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
2016-06-21 10:15 [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node Dirk Behme
@ 2016-06-21 10:20 ` Dirk Behme
2016-06-22 15:58 ` Julien Grall
1 sibling, 0 replies; 6+ messages in thread
From: Dirk Behme @ 2016-06-21 10:20 UTC (permalink / raw)
To: xen-devel, Julien Grall, Stefano Stabellini
On 21.06.2016 12:15, Dirk Behme wrote:
> Some clocks might be used by Xen (drivers) and not by the Linux kernel. If
> these are not registered by the Linux kernel, they might be disabled by the
> Linux kernel's clk_disable_unused() as the kernel doesn't know that
> they are used (by Xen drivers). The clock of the serial console handled by
> Xen is one example for this. It might be disabled by clk_disable_unused() which
> stops the whole serial output, even from Xen, then.
>
> 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.
>
> To fix this, add the clocks used by Xen to the hypervisor node. The Linux
> kernel has to register the clocks from the hypervisor node, then.
>
> Therefore, collect all clocks from nodes used by Xen. These are marked
> with DOMID_XEN. Afterwards, add a 'clocks' node to the hypervisor node
> containing all these clocks. The Linux kernel can register all these clocks,
> preventing them from being disabled, then.
Just for the logs, the Linux kernel counterpart is
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438067.html
Dirk
> ---
> xen/arch/arm/domain_build.c | 20 ++++++++++++++++++++
> xen/arch/arm/kernel.h | 7 +++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 2e4c295..fccf87e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -657,6 +657,10 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
> if ( res )
> return res;
>
> + res = fdt_property(fdt, "clocks", kinfo->clk.dtclocks, kinfo->clk.cnt);
> + if ( res )
> + return res;
> +
> res = fdt_end_node(fdt);
>
> return res;
> @@ -1213,9 +1217,11 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> { /* sentinel */ },
> };
> struct dt_device_node *child;
> + unsigned int len;
> int res;
> const char *name;
> const char *path;
> + const char *clocks;
>
> path = dt_node_full_name(node);
>
> @@ -1246,6 +1252,20 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> if ( dt_device_used_by(node) == DOMID_XEN )
> {
> DPRINT(" Skip it (used by Xen)\n");
> +
> + /*
> + * Remember the clock used by the skipped node
> + * We add it later to the hypervisor node to make the
> + * Linux kernel aware of its usage
> + */
> + clocks = dt_get_property(node, "clocks", &len);
> + if ( kinfo->clk.cnt + len >= MAX_DT_CLOCKS ) {
> + printk("Failed to remember the clock node of %s\n", path);
> + printk("Use the Linux kernel command 'clk_ignore_unused'\n");
> + return 0;
> + }
> + memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
> + kinfo->clk.cnt += len;
> return 0;
> }
>
> diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> index c1b07d4..527b279 100644
> --- a/xen/arch/arm/kernel.h
> +++ b/xen/arch/arm/kernel.h
> @@ -10,6 +10,12 @@
> #include <xen/device_tree.h>
> #include <asm/setup.h>
>
> +#define MAX_DT_CLOCKS 256
> +struct dtclk {
> + unsigned char dtclocks[MAX_DT_CLOCKS];
> + unsigned int cnt;
> +};
> +
> struct kernel_info {
> #ifdef CONFIG_ARM_64
> enum domain_type type;
> @@ -18,6 +24,7 @@ struct kernel_info {
> void *fdt; /* flat device tree */
> paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
> struct meminfo mem;
> + struct dtclk clk;
>
> /* kernel entry point */
> paddr_t entry;
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
2016-06-21 10:15 [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node Dirk Behme
2016-06-21 10:20 ` Dirk Behme
@ 2016-06-22 15:58 ` Julien Grall
2016-06-23 15:02 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2016-06-22 15:58 UTC (permalink / raw)
To: Dirk Behme, xen-devel, Stefano Stabellini
Hello Dirk,
On 21/06/16 11:15, Dirk Behme wrote:
> Some clocks might be used by Xen (drivers) and not by the Linux kernel. If
> these are not registered by the Linux kernel, they might be disabled by the
> Linux kernel's clk_disable_unused() as the kernel doesn't know that
> they are used (by Xen drivers). The clock of the serial console handled by
> Xen is one example for this. It might be disabled by clk_disable_unused() which
> stops the whole serial output, even from Xen, then.
>
> 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.
>
> To fix this, add the clocks used by Xen to the hypervisor node. The Linux
> kernel has to register the clocks from the hypervisor node, then.
>
> Therefore, collect all clocks from nodes used by Xen. These are marked
> with DOMID_XEN. Afterwards, add a 'clocks' node to the hypervisor node
> containing all these clocks. The Linux kernel can register all these clocks,
> preventing them from being disabled, then.
>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> xen/arch/arm/domain_build.c | 20 ++++++++++++++++++++
> xen/arch/arm/kernel.h | 7 +++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 2e4c295..fccf87e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -657,6 +657,10 @@ static int make_hypervisor_node(const struct kernel_info *kinfo,
> if ( res )
> return res;
>
> + res = fdt_property(fdt, "clocks", kinfo->clk.dtclocks, kinfo->clk.cnt);
You may create an empty property if there is no clocks.
> + if ( res )
> + return res;
> +
> res = fdt_end_node(fdt);
>
> return res;
> @@ -1213,9 +1217,11 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> { /* sentinel */ },
> };
> struct dt_device_node *child;
> + unsigned int len;
> int res;
> const char *name;
> const char *path;
> + const char *clocks;
>
> path = dt_node_full_name(node);
>
> @@ -1246,6 +1252,20 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> if ( dt_device_used_by(node) == DOMID_XEN )
> {
> DPRINT(" Skip it (used by Xen)\n");
> +
> + /*
> + * Remember the clock used by the skipped node
> + * We add it later to the hypervisor node to make the
> + * Linux kernel aware of its usage
> + */
> + clocks = dt_get_property(node, "clocks", &len);
len is only updated when the property exists.
> + if ( kinfo->clk.cnt + len >= MAX_DT_CLOCKS ) {
The bracket should be a newline.
> + printk("Failed to remember the clock node of %s\n", path);
> + printk("Use the Linux kernel command 'clk_ignore_unused'\n");
> + return 0;
I don't think this is tolerable. We need to fix it once and for all.
I understand that xen does not provide a realloc function. Is there
another way we can get a rid of this limit?
I am wondering if we can use the member domain_list of dt_device_node to
link the device having a clock property. And then latter one, allocate
the memory + copying the data.
> + }
> + memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
> + kinfo->clk.cnt += len;
> return 0;
> }
>
> diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
> index c1b07d4..527b279 100644
> --- a/xen/arch/arm/kernel.h
> +++ b/xen/arch/arm/kernel.h
> @@ -10,6 +10,12 @@
> #include <xen/device_tree.h>
> #include <asm/setup.h>
>
> +#define MAX_DT_CLOCKS 256
How did you define this value?
> +struct dtclk {
> + unsigned char dtclocks[MAX_DT_CLOCKS];
> + unsigned int cnt;
> +};
> +
> struct kernel_info {
> #ifdef CONFIG_ARM_64
> enum domain_type type;
> @@ -18,6 +24,7 @@ struct kernel_info {
> void *fdt; /* flat device tree */
> paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
> struct meminfo mem;
> + struct dtclk clk;
>
> /* kernel entry point */
> paddr_t entry;
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
2016-06-22 15:58 ` Julien Grall
@ 2016-06-23 15:02 ` Julien Grall
2016-06-23 15:59 ` Dirk Behme
0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2016-06-23 15:02 UTC (permalink / raw)
To: Dirk Behme, xen-devel, Stefano Stabellini
On 22/06/16 16:58, Julien Grall wrote:
> On 21/06/16 11:15, Dirk Behme wrote:
>> + printk("Failed to remember the clock node of %s\n", path);
>> + printk("Use the Linux kernel command
>> 'clk_ignore_unused'\n");
>> + return 0;
>
> I don't think this is tolerable. We need to fix it once and for all.
>
> I understand that xen does not provide a realloc function. Is there
> another way we can get a rid of this limit?
Note that I would wait that we agree on the device tree bindings before
reworking this patch. It will avoid you to waste time if we decide to
move towards a different solution.
> I am wondering if we can use the member domain_list of dt_device_node to
> link the device having a clock property. And then latter one, allocate
> the memory + copying the data.
>
>> + }
>> + memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
>> + kinfo->clk.cnt += len;
>> return 0;
>> }
>>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
2016-06-23 15:02 ` Julien Grall
@ 2016-06-23 15:59 ` Dirk Behme
2016-06-23 16:02 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Dirk Behme @ 2016-06-23 15:59 UTC (permalink / raw)
To: Julien Grall, Stefano Stabellini; +Cc: xen-devel, Dirk Behme
On 23.06.2016 17:02, Julien Grall wrote:
> On 22/06/16 16:58, Julien Grall wrote:
>> On 21/06/16 11:15, Dirk Behme wrote:
>>> + printk("Failed to remember the clock node of %s\n",
>>> path);
>>> + printk("Use the Linux kernel command
>>> 'clk_ignore_unused'\n");
>>> + return 0;
>>
>> I don't think this is tolerable. We need to fix it once and for all.
>>
>> I understand that xen does not provide a realloc function. Is there
>> another way we can get a rid of this limit?
>
> Note that I would wait that we agree on the device tree bindings
> before reworking this patch.
You mean the 'clocks' binding for the hypervisor node? I.e. as proposed in
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02885.html
create a patch for
- linux/Documentation/devicetree/bindings/arm/xen.txt
- xen/docs/misc/arm/device-tree/guest.txt
and proceed only with the code changes when this is accepted?
Best regards
Dirk
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
2016-06-23 15:59 ` Dirk Behme
@ 2016-06-23 16:02 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2016-06-23 16:02 UTC (permalink / raw)
To: Dirk Behme, Stefano Stabellini; +Cc: xen-devel, Dirk Behme
Hi Dirk,
On 23/06/16 16:59, Dirk Behme wrote:
> On 23.06.2016 17:02, Julien Grall wrote:
>> On 22/06/16 16:58, Julien Grall wrote:
>>> On 21/06/16 11:15, Dirk Behme wrote:
>>>> + printk("Failed to remember the clock node of %s\n",
>>>> path);
>>>> + printk("Use the Linux kernel command
>>>> 'clk_ignore_unused'\n");
>>>> + return 0;
>>>
>>> I don't think this is tolerable. We need to fix it once and for all.
>>>
>>> I understand that xen does not provide a realloc function. Is there
>>> another way we can get a rid of this limit?
>>
>> Note that I would wait that we agree on the device tree bindings
>> before reworking this patch.
>
>
> You mean the 'clocks' binding for the hypervisor node? I.e. as proposed in
>
> https://lists.xen.org/archives/html/xen-devel/2016-06/msg02885.html
>
> create a patch for
>
> - linux/Documentation/devicetree/bindings/arm/xen.txt
> - xen/docs/misc/arm/device-tree/guest.txt
>
> and proceed only with the code changes when this is accepted?
The second patch (xen/docs/misc/*) will be a verbatim copy of the first
one. So writing only the first one is good for now.
I will defer the implementation in Xen but keep the Linux one to give an
overview to the maintainers (ARM and clocks)
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-23 16:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-21 10:15 [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node Dirk Behme
2016-06-21 10:20 ` Dirk Behme
2016-06-22 15:58 ` Julien Grall
2016-06-23 15:02 ` Julien Grall
2016-06-23 15:59 ` Dirk Behme
2016-06-23 16:02 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.