2013-09-23 20:32:20

by Rob Herring

[permalink] [raw]
Subject: [PATCH] TTY: hvc_dcc: probe for a JTAG connection before registering

From: Rob Herring <[email protected]>

Enabling the ARM DCC console and using without a JTAG connection will
simply hang the system. Since distros like to turn on all options, this
is a reoccurring problem to debug. We can do better by checking if
anything is attached and handling characters. There is no way to probe
this, so send a newline and check that it is handled.

Cc: Paolo Pisati <[email protected]>
Cc: Tim Gardner <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 44fbeba..44c8aff 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -86,6 +86,21 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
return i;
}

+static bool hvc_dcc_check(void)
+{
+ unsigned long time = jiffies;
+
+ /* Write a test character to check if it is handled */
+ __dcc_putchar('\n');
+
+ while (jiffies < time + (HZ / 10)) {
+ if (!(__dcc_getstatus() & DCC_STATUS_TX))
+ return true;
+ }
+
+ return false;
+}
+
static const struct hv_ops hvc_dcc_get_put_ops = {
.get_chars = hvc_dcc_get_chars,
.put_chars = hvc_dcc_put_chars,
@@ -93,6 +108,9 @@ static const struct hv_ops hvc_dcc_get_put_ops = {

static int __init hvc_dcc_console_init(void)
{
+ if (!hvc_dcc_check())
+ return -ENODEV;
+
hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
return 0;
}
@@ -100,6 +118,9 @@ console_initcall(hvc_dcc_console_init);

static int __init hvc_dcc_init(void)
{
+ if (!hvc_dcc_check())
+ return -ENODEV;
+
hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
return 0;
}
--
1.8.1.2


2013-09-23 21:10:30

by Tim Gardner

[permalink] [raw]
Subject: Re: [PATCH] TTY: hvc_dcc: probe for a JTAG connection before registering

On 09/23/2013 01:30 PM, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Enabling the ARM DCC console and using without a JTAG connection will
> simply hang the system. Since distros like to turn on all options, this
> is a reoccurring problem to debug. We can do better by checking if
> anything is attached and handling characters. There is no way to probe
> this, so send a newline and check that it is handled.
>
> Cc: Paolo Pisati <[email protected]>
> Cc: Tim Gardner <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Jiri Slaby <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
> index 44fbeba..44c8aff 100644
> --- a/drivers/tty/hvc/hvc_dcc.c
> +++ b/drivers/tty/hvc/hvc_dcc.c
> @@ -86,6 +86,21 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
> return i;
> }
>
> +static bool hvc_dcc_check(void)
> +{
> + unsigned long time = jiffies;
> +
> + /* Write a test character to check if it is handled */
> + __dcc_putchar('\n');
> +
> + while (jiffies < time + (HZ / 10)) {
> + if (!(__dcc_getstatus() & DCC_STATUS_TX))
> + return true;
> + }

I think you should use one of the jiffies time comparison functions, e.g.,

time = jiffies + (HZ / 10);
while (time_before(jiffies,time) {
...
}

What is the possibility that __dcc_getstatus() will return garbage if no
JTAG is connected ? In that event you're likely to get a false positive
on DCC_STATUS_TX.

rtg


--
Tim Gardner [email protected]

2013-09-23 21:35:56

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] TTY: hvc_dcc: probe for a JTAG connection before registering

On 09/23/2013 04:10 PM, Tim Gardner wrote:
> On 09/23/2013 01:30 PM, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> Enabling the ARM DCC console and using without a JTAG connection will
>> simply hang the system. Since distros like to turn on all options, this
>> is a reoccurring problem to debug. We can do better by checking if
>> anything is attached and handling characters. There is no way to probe
>> this, so send a newline and check that it is handled.
>>
>> Cc: Paolo Pisati <[email protected]>
>> Cc: Tim Gardner <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Jiri Slaby <[email protected]>
>> Signed-off-by: Rob Herring <[email protected]>
>> ---
>> drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
>> index 44fbeba..44c8aff 100644
>> --- a/drivers/tty/hvc/hvc_dcc.c
>> +++ b/drivers/tty/hvc/hvc_dcc.c
>> @@ -86,6 +86,21 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
>> return i;
>> }
>>
>> +static bool hvc_dcc_check(void)
>> +{
>> + unsigned long time = jiffies;
>> +
>> + /* Write a test character to check if it is handled */
>> + __dcc_putchar('\n');
>> +
>> + while (jiffies < time + (HZ / 10)) {
>> + if (!(__dcc_getstatus() & DCC_STATUS_TX))
>> + return true;
>> + }
>
> I think you should use one of the jiffies time comparison functions, e.g.,
>
> time = jiffies + (HZ / 10);
> while (time_before(jiffies,time) {
> ...
> }

Yes.

>
> What is the possibility that __dcc_getstatus() will return garbage if no
> JTAG is connected ? In that event you're likely to get a false positive
> on DCC_STATUS_TX.

That probably would not be architecturally compliant. The status bits
are cleared by JTAG reading the read side of the register. If the bit
stays set, we'll return false. If the bit is always clear, then we
wouldn't hang in the first place and should be fine. If you get random
values, then I'm sorry and good luck. You are still no worse off than
before this patch.

Rob

2013-09-25 02:06:08

by Rob Herring

[permalink] [raw]
Subject: [PATCH v2] TTY: hvc_dcc: probe for a JTAG connection before registering

From: Rob Herring <[email protected]>

Enabling the ARM DCC console and using without a JTAG connection will
simply hang the system. Since distros like to turn on all options, this
is a reoccurring problem to debug. We can do better by checking if
anything is attached and handling characters. There is no way to probe
this, so send a newline and check that it is handled.

Cc: Paolo Pisati <[email protected]>
Cc: Tim Gardner <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
v2: use time_is_after_jiffies

drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 44fbeba..3502a7b 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -86,6 +86,21 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
return i;
}

+static bool hvc_dcc_check(void)
+{
+ unsigned long time = jiffies + (HZ / 10);
+
+ /* Write a test character to check if it is handled */
+ __dcc_putchar('\n');
+
+ while (time_is_after_jiffies(time)) {
+ if (!(__dcc_getstatus() & DCC_STATUS_TX))
+ return true;
+ }
+
+ return false;
+}
+
static const struct hv_ops hvc_dcc_get_put_ops = {
.get_chars = hvc_dcc_get_chars,
.put_chars = hvc_dcc_put_chars,
@@ -93,6 +108,9 @@ static const struct hv_ops hvc_dcc_get_put_ops = {

static int __init hvc_dcc_console_init(void)
{
+ if (!hvc_dcc_check())
+ return -ENODEV;
+
hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
return 0;
}
@@ -100,6 +118,9 @@ console_initcall(hvc_dcc_console_init);

static int __init hvc_dcc_init(void)
{
+ if (!hvc_dcc_check())
+ return -ENODEV;
+
hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
return 0;
}
--
1.8.1.2

2013-10-17 19:16:40

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2] TTY: hvc_dcc: probe for a JTAG connection before registering

On Tue, Sep 24, 2013 at 9:05 PM, Rob Herring <[email protected]> wrote:
> From: Rob Herring <[email protected]>
>
> Enabling the ARM DCC console and using without a JTAG connection will
> simply hang the system. Since distros like to turn on all options, this
> is a reoccurring problem to debug. We can do better by checking if
> anything is attached and handling characters. There is no way to probe
> this, so send a newline and check that it is handled.
>
> Cc: Paolo Pisati <[email protected]>
> Cc: Tim Gardner <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Jiri Slaby <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> v2: use time_is_after_jiffies
>
> drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)

Greg, Jiri,

Can you please apply.

Rob

>
> diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
> index 44fbeba..3502a7b 100644
> --- a/drivers/tty/hvc/hvc_dcc.c
> +++ b/drivers/tty/hvc/hvc_dcc.c
> @@ -86,6 +86,21 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
> return i;
> }
>
> +static bool hvc_dcc_check(void)
> +{
> + unsigned long time = jiffies + (HZ / 10);
> +
> + /* Write a test character to check if it is handled */
> + __dcc_putchar('\n');
> +
> + while (time_is_after_jiffies(time)) {
> + if (!(__dcc_getstatus() & DCC_STATUS_TX))
> + return true;
> + }
> +
> + return false;
> +}
> +
> static const struct hv_ops hvc_dcc_get_put_ops = {
> .get_chars = hvc_dcc_get_chars,
> .put_chars = hvc_dcc_put_chars,
> @@ -93,6 +108,9 @@ static const struct hv_ops hvc_dcc_get_put_ops = {
>
> static int __init hvc_dcc_console_init(void)
> {
> + if (!hvc_dcc_check())
> + return -ENODEV;
> +
> hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
> return 0;
> }
> @@ -100,6 +118,9 @@ console_initcall(hvc_dcc_console_init);
>
> static int __init hvc_dcc_init(void)
> {
> + if (!hvc_dcc_check())
> + return -ENODEV;
> +
> hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
> return 0;
> }
> --
> 1.8.1.2
>

2013-10-17 20:20:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] TTY: hvc_dcc: probe for a JTAG connection before registering

On Thu, Oct 17, 2013 at 02:16:37PM -0500, Rob Herring wrote:
> On Tue, Sep 24, 2013 at 9:05 PM, Rob Herring <[email protected]> wrote:
> > From: Rob Herring <[email protected]>
> >
> > Enabling the ARM DCC console and using without a JTAG connection will
> > simply hang the system. Since distros like to turn on all options, this
> > is a reoccurring problem to debug. We can do better by checking if
> > anything is attached and handling characters. There is no way to probe
> > this, so send a newline and check that it is handled.
> >
> > Cc: Paolo Pisati <[email protected]>
> > Cc: Tim Gardner <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Jiri Slaby <[email protected]>
> > Signed-off-by: Rob Herring <[email protected]>
> > ---
> > v2: use time_is_after_jiffies
> >
> > drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
>
> Greg, Jiri,
>
> Can you please apply.

I already did, didn't you get an email saying it was applied to my tree?

2013-10-17 20:31:50

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2] TTY: hvc_dcc: probe for a JTAG connection before registering

On 10/17/2013 03:20 PM, Greg Kroah-Hartman wrote:
> On Thu, Oct 17, 2013 at 02:16:37PM -0500, Rob Herring wrote:
>> On Tue, Sep 24, 2013 at 9:05 PM, Rob Herring <[email protected]> wrote:
>>> From: Rob Herring <[email protected]>
>>>
>>> Enabling the ARM DCC console and using without a JTAG connection will
>>> simply hang the system. Since distros like to turn on all options, this
>>> is a reoccurring problem to debug. We can do better by checking if
>>> anything is attached and handling characters. There is no way to probe
>>> this, so send a newline and check that it is handled.
>>>
>>> Cc: Paolo Pisati <[email protected]>
>>> Cc: Tim Gardner <[email protected]>
>>> Cc: Greg Kroah-Hartman <[email protected]>
>>> Cc: Jiri Slaby <[email protected]>
>>> Signed-off-by: Rob Herring <[email protected]>
>>> ---
>>> v2: use time_is_after_jiffies
>>>
>>> drivers/tty/hvc/hvc_dcc.c | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>
>> Greg, Jiri,
>>
>> Can you please apply.
>
> I already did, didn't you get an email saying it was applied to my tree?

Ah, yes. Found it buried in my work email. Sorry about that.

Rob