[PATCH 1/2] .gitignore: Add grub-core/build-grub-module-verifier

[PATCH 1/2] .gitignore: Add grub-core/build-grub-module-verifier

[Joonas Lahtinen]

Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 18ab8e8..32f634c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -246,3 +246,5 @@ build-aux/test-driver
 /garbage-gen
 /garbage-gen.exe
 /grub-fs-tester
+grub-core/build-grub-module-verifier
+
-- 
2.7.4

[PATCH 2/2] serial: Poll USB devices if usbX serial port is missing

[Joonas Lahtinen]

If usbX serial port is missing, try to poll USB devices. This makes them
appear at least on Intel NUC devices DN2820FYKH and DCCP847DYE.

This fixes cases where you try to configure usbX serial at the beginning
of a script, like:

  serial usb0 --speed=115200 --word=8 --parity=none --stop=1
  terminal_output --append serial_usb0
  terminal_input --append serial_usb0

Without this patch the above would fail with:

  serial port `usb0' isn't found

Strangely, adding 'usb' command before the serial initialization
made it work, and even moving the terminal_output and input command
to be before. This is due to the terminal_output command detecting
"serial_usb" and running USB polling if the output is missing. 'usb'
command from usbtest module always polls USB devices.

As it is the logical thing to configure the serial port before adding
it as an output or input, this patch makes things works as expected.
Most online resources mentioning GRUB and USB serial adapters also do
mention the above order, so there must be quite many struggling with
this problem on platforms where USB needs to be polled in order for
the detection to happen.

Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 grub-core/term/serial.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index db80b3b..1f856a2 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -191,6 +191,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
   struct grub_serial_port *port;
   struct grub_serial_config config;
   grub_err_t err;
+  int again;
 
   if (state[OPTION_UNIT].set)
     {
@@ -212,7 +213,25 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
   if (!name)
     name = "com0";
 
-  port = grub_serial_find (name);
+  again = 0;
+  while(1)
+    {
+      port = grub_serial_find (name);
+
+      if (port || again)
+	break;
+
+      if (grub_memcmp (name, "usb", sizeof ("usb") - 1) == 0)
+	{
+	  grub_usb_poll_devices(1);
+	  again = 1;
+	}
+      else
+	{
+	  break;
+	}
+    }
+
   if (!port)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, 
 		       N_("serial port `%s' isn't found"),
-- 
2.7.4

Re: [PATCH 2/2] serial: Poll USB devices if usbX serial port is missing

[Andrei Borzenkov]

10.11.2016 16:45, Joonas Lahtinen пишет:
> If usbX serial port is missing, try to poll USB devices. This makes them
> appear at least on Intel NUC devices DN2820FYKH and DCCP847DYE.
> 
> This fixes cases where you try to configure usbX serial at the beginning
> of a script, like:
> 
>   serial usb0 --speed=115200 --word=8 --parity=none --stop=1
>   terminal_output --append serial_usb0
>   terminal_input --append serial_usb0
> 
> Without this patch the above would fail with:
> 
>   serial port `usb0' isn't found
> 
> Strangely, adding 'usb' command before the serial initialization
> made it work, and even moving the terminal_output and input command
> to be before. This is due to the terminal_output command detecting
> "serial_usb" and running USB polling if the output is missing. 'usb'
> command from usbtest module always polls USB devices.
> 
> As it is the logical thing to configure the serial port before adding
> it as an output or input, this patch makes things works as expected.
> Most online resources mentioning GRUB and USB serial adapters also do
> mention the above order, so there must be quite many struggling with
> this problem on platforms where USB needs to be polled in order for
> the detection to happen.
> 
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  grub-core/term/serial.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
> index db80b3b..1f856a2 100644
> --- a/grub-core/term/serial.c
> +++ b/grub-core/term/serial.c
> @@ -191,6 +191,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
>    struct grub_serial_port *port;
>    struct grub_serial_config config;
>    grub_err_t err;
> +  int again;
>  
>    if (state[OPTION_UNIT].set)
>      {
> @@ -212,7 +213,25 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
>    if (!name)
>      name = "com0";
>  
> -  port = grub_serial_find (name);
> +  again = 0;
> +  while(1)
> +    {
> +      port = grub_serial_find (name);
> +
> +      if (port || again)
> +	break;
> +
> +      if (grub_memcmp (name, "usb", sizeof ("usb") - 1) == 0)
> +	{
> +	  grub_usb_poll_devices(1);

This creates hard dependency of serial module to usb. Please use
grub_term_poll_usb as terminal module does.

> +	  again = 1;
> +	}
> +      else
> +	{
> +	  break;
> +	}
> +    }
> +
>    if (!port)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, 
>  		       N_("serial port `%s' isn't found"),
> 

[PATCH v2] serial: Poll USB devices if usbX serial port is missing

[Joonas Lahtinen]

If usbX serial port is missing, try to poll USB devices. This makes them
appear at least on Intel NUC devices DN2820FYKH and DCCP847DYE.

This fixes cases where you try to configure usbX serial at the beginning
of a script, like:

  serial usb0 --speed=115200 --word=8 --parity=none --stop=1
  terminal_output --append serial_usb0
  terminal_input --append serial_usb0

Without this patch the above would fail with:

  serial port `usb0' isn't found

Strangely, adding 'usb' command before the serial initialization
made it work, and even moving the terminal_output and input command
to be before. This is due to the terminal_output command detecting
"serial_usb" and running USB polling if the output is missing. 'usb'
command from usbtest module always polls USB devices.

As it is the logical thing to configure the serial port before adding
it as an output or input, this patch makes things works as expected.
Most online resources mentioning GRUB and USB serial adapters also do
mention the above order, so there must be quite many struggling with
this problem on platforms where USB needs to be polled in order for
the detection to happen.

v2:
- Use grub_term_poll_usb to avoid module dependency (Andrei)

Cc: Andrei Borzenkov <arvidjaar@gmail.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 grub-core/term/serial.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index db80b3b..1a009a9 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -191,6 +191,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
   struct grub_serial_port *port;
   struct grub_serial_config config;
   grub_err_t err;
+  int again;
 
   if (state[OPTION_UNIT].set)
     {
@@ -212,7 +213,26 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
   if (!name)
     name = "com0";
 
-  port = grub_serial_find (name);
+  again = 0;
+  while(1)
+    {
+      port = grub_serial_find (name);
+
+      if (port || again)
+	break;
+
+      if (grub_memcmp (name, "usb", sizeof ("usb") - 1) == 0
+          && grub_term_poll_usb)
+	{
+	  grub_term_poll_usb (1);
+	  again = 1;
+	}
+      else
+	{
+	  break;
+	}
+    }
+
   if (!port)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, 
 		       N_("serial port `%s' isn't found"),
-- 
2.7.4

Re: [PATCH v2] serial: Poll USB devices if usbX serial port is missing

[Andrei Borzenkov]

On Fri, Nov 18, 2016 at 1:36 PM, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
> If usbX serial port is missing, try to poll USB devices. This makes them
> appear at least on Intel NUC devices DN2820FYKH and DCCP847DYE.
>
> This fixes cases where you try to configure usbX serial at the beginning
> of a script, like:
>
>   serial usb0 --speed=115200 --word=8 --parity=none --stop=1
>   terminal_output --append serial_usb0
>   terminal_input --append serial_usb0
>
> Without this patch the above would fail with:
>
>   serial port `usb0' isn't found
>
> Strangely, adding 'usb' command before the serial initialization
> made it work, and even moving the terminal_output and input command
> to be before. This is due to the terminal_output command detecting
> "serial_usb" and running USB polling if the output is missing. 'usb'
> command from usbtest module always polls USB devices.
>
 As it is the logical thing to configure the serial port before adding
> it as an output or input, this patch makes things works as expected.
> Most online resources mentioning GRUB and USB serial adapters also do
> mention the above order, so there must be quite many struggling with
> this problem on platforms where USB needs to be polled in order for
> the detection to happen.
>
> v2:
> - Use grub_term_poll_usb to avoid module dependency (Andrei)
>

Hmm ... note that terminal_input/terminal_output also auto-load
modules that handle serial ports. So this still will not be
equivalent, it will require user to manually pre-load USB. Did you
actually test this version of patch? Do you pre-load USB modules
somewhere?

The problem is, infrastructure to handle auto-load lists currently
belongs to normal mode, while serial may sensibly be used before
normal is loaded. Not sure what we can do here.

> Cc: Andrei Borzenkov <arvidjaar@gmail.com>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  grub-core/term/serial.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
> index db80b3b..1a009a9 100644
> --- a/grub-core/term/serial.c
> +++ b/grub-core/term/serial.c
> @@ -191,6 +191,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
>    struct grub_serial_port *port;
>    struct grub_serial_config config;
>    grub_err_t err;
> +  int again;
>
>    if (state[OPTION_UNIT].set)
>      {
> @@ -212,7 +213,26 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
>    if (!name)
>      name = "com0";
>
> -  port = grub_serial_find (name);
> +  again = 0;
> +  while(1)
> +    {
> +      port = grub_serial_find (name);
> +
> +      if (port || again)
> +       break;
> +
> +      if (grub_memcmp (name, "usb", sizeof ("usb") - 1) == 0
> +          && grub_term_poll_usb)
> +       {
> +         grub_term_poll_usb (1);
> +         again = 1;
> +       }
> +      else
> +       {
> +         break;
> +       }
> +    }
> +
>    if (!port)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT,
>                        N_("serial port `%s' isn't found"),
> --
> 2.7.4
>

Re: [PATCH v2] serial: Poll USB devices if usbX serial port is missing

[Joonas Lahtinen]

On pe, 2016-11-18 at 15:21 +0300, Andrei Borzenkov wrote:
> On Fri, Nov 18, 2016 at 1:36 PM, Joonas Lahtinen
> > 
> > v2:
> > - Use grub_term_poll_usb to avoid module dependency (Andrei)
> > 
> 
> Hmm ... note that terminal_input/terminal_output also auto-load
> modules that handle serial ports. So this still will not be
> equivalent, it will require user to manually pre-load USB. Did you
> actually test this version of patch? Do you pre-load USB modules
> somewhere?

Nope, I took your word literally and just changed the function.
Assuming it's a rather direct replacement. Excuse for that.

> 
> The problem is, infrastructure to handle auto-load lists currently
> belongs to normal mode, while serial may sensibly be used before
> normal is loaded. Not sure what we can do here.

I think this is getting more important when we're seeing increasing
amount of devices without even an internal serial port.

Sounds to me like pulling the auto-loading out of just normal mode
would make sense?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

Re: [PATCH v2] serial: Poll USB devices if usbX serial port is missing

[Andrei Borzenkov]

21.11.2016 17:05, Joonas Lahtinen пишет:
> On pe, 2016-11-18 at 15:21 +0300, Andrei Borzenkov wrote:
>> On Fri, Nov 18, 2016 at 1:36 PM, Joonas Lahtinen
>>>
>>> v2:
>>> - Use grub_term_poll_usb to avoid module dependency (Andrei)
>>>
>>
>> Hmm ... note that terminal_input/terminal_output also auto-load
>> modules that handle serial ports. So this still will not be
>> equivalent, it will require user to manually pre-load USB. Did you
>> actually test this version of patch? Do you pre-load USB modules
>> somewhere?
> 
> Nope, I took your word literally and just changed the function.

Looking more closely, only top-level modules are autoloaded anyway, like
"serial" itself; low level drivers are not, at least as far as I can
tell. In your case "serial" is pulled in by using command, so it should
be no change.

> Assuming it's a rather direct replacement. Excuse for that.
> 

Could you confirm that your last version works for you? Could you give
more details about your configuration, in particular, how you load
necessary modules (if you load them)? What USB serial driver you use?

>>
>> The problem is, infrastructure to handle auto-load lists currently
>> belongs to normal mode, while serial may sensibly be used before
>> normal is loaded. Not sure what we can do here.
> 
> I think this is getting more important when we're seeing increasing
> amount of devices without even an internal serial port.
> 
> Sounds to me like pulling the auto-loading out of just normal mode
> would make sense?
> 
> Regards, Joonas
> 

Re: [PATCH 1/2] .gitignore: Add grub-core/build-grub-module-verifier

[Andrei Borzenkov]

10.11.2016 16:45, Joonas Lahtinen пишет:
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  .gitignore | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 18ab8e8..32f634c 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -246,3 +246,5 @@ build-aux/test-driver
>  /garbage-gen
>  /garbage-gen.exe
>  /grub-fs-tester
> +grub-core/build-grub-module-verifier
> +
> 
Applied after removing empty line. Thanks!