Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
On Mon, 2015-01-12 at 15:24 +, Julien Grall wrote: > Ok. I'm fine with it. Thanks, I've applied the whole series, fixing the comment you noted in #3 as I went. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
Hi Ian,
On 12/01/15 15:22, Ian Campbell wrote:
> On Thu, 2015-01-08 at 13:30 +, Julien Grall wrote:
>> On 08/01/15 13:22, Ian Campbell wrote:
>>> On Thu, 2015-01-08 at 13:15 +, Julien Grall wrote:
Hi Ian,
On 08/01/15 11:53, Ian Campbell wrote:
> +ret = dt_property_read_string(chosen, "stdout-path",
> &stdout);
> +if ( ret >= 0 )
> +{
> +printk("Taking dtuart configuration from
> /chosen/stdout-path\n");
> +if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
> + >= sizeof(opt_dtuart) )
> +printk("WARNING: /chosen/stdout-path too long,
> truncated\n");
I would add XENLOG_WARNING here and ...
> +}
> +else if ( ret != -EINVAL /* Not present */ )
> +printk("Failed to read /chosen/stdout-path (%d)\n", ret);
XENLOG_ERROR here.
>>>
>>> In practice these only go via the earlyprintk mechanism, since the
>>> console can't be setup yet. I'm not sure it's worthwhile tagging such
>>> messages.
>>
>> earlyprintk is transparent for the console code. Tagging may help if we
>> decide to implement other kind of console later (VGA, PCI UART...).
>>
>> Anyway, I doesn't change much things here as the message is tagged as
>> WARNING by default. So it will be always printing.
>
> It turns out that none of the existing prints in this function use the
> tags, and I think its of marginal use in this context so I don't think
> it is necessary to go changing them all, or to only use the tags for
> these two messages.
Ok. I'm fine with it.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
On Thu, 2015-01-08 at 13:30 +, Julien Grall wrote:
> On 08/01/15 13:22, Ian Campbell wrote:
> > On Thu, 2015-01-08 at 13:15 +, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 08/01/15 11:53, Ian Campbell wrote:
> >>> +ret = dt_property_read_string(chosen, "stdout-path",
> >>> &stdout);
> >>> +if ( ret >= 0 )
> >>> +{
> >>> +printk("Taking dtuart configuration from
> >>> /chosen/stdout-path\n");
> >>> +if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
> >>> + >= sizeof(opt_dtuart) )
> >>> +printk("WARNING: /chosen/stdout-path too long,
> >>> truncated\n");
> >>
> >> I would add XENLOG_WARNING here and ...
> >>
> >>> +}
> >>> +else if ( ret != -EINVAL /* Not present */ )
> >>> +printk("Failed to read /chosen/stdout-path (%d)\n", ret);
> >>
> >> XENLOG_ERROR here.
> >
> > In practice these only go via the earlyprintk mechanism, since the
> > console can't be setup yet. I'm not sure it's worthwhile tagging such
> > messages.
>
> earlyprintk is transparent for the console code. Tagging may help if we
> decide to implement other kind of console later (VGA, PCI UART...).
>
> Anyway, I doesn't change much things here as the message is tagged as
> WARNING by default. So it will be always printing.
It turns out that none of the existing prints in this function use the
tags, and I think its of marginal use in this context so I don't think
it is necessary to go changing them all, or to only use the tags for
these two messages.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
On 08/01/15 13:22, Ian Campbell wrote:
> On Thu, 2015-01-08 at 13:15 +, Julien Grall wrote:
>> Hi Ian,
>>
>> On 08/01/15 11:53, Ian Campbell wrote:
>>> +ret = dt_property_read_string(chosen, "stdout-path", &stdout);
>>> +if ( ret >= 0 )
>>> +{
>>> +printk("Taking dtuart configuration from
>>> /chosen/stdout-path\n");
>>> +if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
>>> + >= sizeof(opt_dtuart) )
>>> +printk("WARNING: /chosen/stdout-path too long,
>>> truncated\n");
>>
>> I would add XENLOG_WARNING here and ...
>>
>>> +}
>>> +else if ( ret != -EINVAL /* Not present */ )
>>> +printk("Failed to read /chosen/stdout-path (%d)\n", ret);
>>
>> XENLOG_ERROR here.
>
> In practice these only go via the earlyprintk mechanism, since the
> console can't be setup yet. I'm not sure it's worthwhile tagging such
> messages.
earlyprintk is transparent for the console code. Tagging may help if we
decide to implement other kind of console later (VGA, PCI UART...).
Anyway, I doesn't change much things here as the message is tagged as
WARNING by default. So it will be always printing.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
On Thu, 2015-01-08 at 13:15 +, Julien Grall wrote:
> Hi Ian,
>
> On 08/01/15 11:53, Ian Campbell wrote:
> > +ret = dt_property_read_string(chosen, "stdout-path", &stdout);
> > +if ( ret >= 0 )
> > +{
> > +printk("Taking dtuart configuration from
> > /chosen/stdout-path\n");
> > +if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
> > + >= sizeof(opt_dtuart) )
> > +printk("WARNING: /chosen/stdout-path too long,
> > truncated\n");
>
> I would add XENLOG_WARNING here and ...
>
> > +}
> > +else if ( ret != -EINVAL /* Not present */ )
> > +printk("Failed to read /chosen/stdout-path (%d)\n", ret);
>
> XENLOG_ERROR here.
In practice these only go via the earlyprintk mechanism, since the
console can't be setup yet. I'm not sure it's worthwhile tagging such
messages.
>
> Otherwise:
>
> Reviewed-by: Julien Grall
>
> Regards,
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
Hi Ian,
On 08/01/15 11:53, Ian Campbell wrote:
> +ret = dt_property_read_string(chosen, "stdout-path", &stdout);
> +if ( ret >= 0 )
> +{
> +printk("Taking dtuart configuration from
> /chosen/stdout-path\n");
> +if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
> + >= sizeof(opt_dtuart) )
> +printk("WARNING: /chosen/stdout-path too long,
> truncated\n");
I would add XENLOG_WARNING here and ...
> +}
> +else if ( ret != -EINVAL /* Not present */ )
> +printk("Failed to read /chosen/stdout-path (%d)\n", ret);
XENLOG_ERROR here.
Otherwise:
Reviewed-by: Julien Grall
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 4/4] dt-uart: support /chosen/stdout-path property.
ePAPR v1.1 section 3.5 defines the /chosen/stdout-path property to
refer to the device to be used for boot console output, so if no
dtuart property is given try to use that instead. This will make Xen
find a suitable console by default on DT platforms which include this
property.
As it happens the dtuart option has the exact same syntax as
stdout-path, so we can just copy the value into that buffer if it is
empty. If the string is too large for the buffer we truncate and warn
but continue in the hopes that enough of the path survived (i.e. only
the options part was dropped) to get something out.
FWIW support for this was added to Linux in v3.19-rc1 (7914a7c5651a
"of: support passing console options with stdout-path") and a fairly
large number of the dts files shipped with Linux have already included
a stdout-path property for quite a while now.
Signed-off-by: Ian Campbell
---
v2:
- simplified after removing unnecessary compat support for ',' in
previous patch, declared stdout in the now smaller scope.
- constify struct dt_device_node pointer
- warn if stdout-path is truncated.
---
xen/arch/arm/domain_build.c |2 ++
xen/drivers/char/dt-uart.c | 22 ++
2 files changed, 24 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index de180d8..c33a73c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -424,6 +424,7 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
* bootargs (from module #1, above).
* * remove bootargs, xen,dom0-bootargs, xen,xen-bootargs,
* linux,initrd-start and linux,initrd-end.
+ * * remove stdout-path.
* * remove bootargs, linux,uefi-system-table,
* linux,uefi-mmap-start, linux,uefi-mmap-size,
* linux,uefi-mmap-desc-size, and linux,uefi-mmap-desc-ver
@@ -434,6 +435,7 @@ static int write_properties(struct domain *d, struct
kernel_info *kinfo,
if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
dt_property_name_is_equal(prop, "linux,initrd-start") ||
dt_property_name_is_equal(prop, "linux,initrd-end") ||
+ dt_property_name_is_equal(prop, "stdout-path") ||
dt_property_name_is_equal(prop, "linux,uefi-system-table") ||
dt_property_name_is_equal(prop, "linux,uefi-mmap-start") ||
dt_property_name_is_equal(prop, "linux,uefi-mmap-size") ||
diff --git a/xen/drivers/char/dt-uart.c b/xen/drivers/char/dt-uart.c
index b9a0956..07d1388 100644
--- a/xen/drivers/char/dt-uart.c
+++ b/xen/drivers/char/dt-uart.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
/*
* Configure UART port with a string:
@@ -46,6 +47,27 @@ void __init dt_uart_init(void)
if ( !strcmp(opt_dtuart, "") )
{
+const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
+
+if ( chosen )
+{
+const char *stdout;
+
+ret = dt_property_read_string(chosen, "stdout-path", &stdout);
+if ( ret >= 0 )
+{
+printk("Taking dtuart configuration from
/chosen/stdout-path\n");
+if ( strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart))
+ >= sizeof(opt_dtuart) )
+printk("WARNING: /chosen/stdout-path too long,
truncated\n");
+}
+else if ( ret != -EINVAL /* Not present */ )
+printk("Failed to read /chosen/stdout-path (%d)\n", ret);
+}
+}
+
+if ( !strcmp(opt_dtuart, "") )
+{
printk("No dtuart path configured\n");
return;
}
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
