Re: [PATCH 4/7] ns8250: Add configuration parameter when adding ports

[Daniel Kiper]

On Fri, Dec 02, 2022 at 10:05:23AM +1100, Benjamin Herrenschmidt wrote:
> From: Benjamin Herrenschmidt <benh@amazon.com>
>
> This will allow ports to be added with a pre-set configuration
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  grub-core/term/ns8250.c | 25 +++++++++++++++++++------
>  grub-core/term/serial.c |  2 +-
>  include/grub/serial.h   |  4 ++--
>  3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
> index b640508d0..08dfb99da 100644
> --- a/grub-core/term/ns8250.c
> +++ b/grub-core/term/ns8250.c
> @@ -303,7 +303,7 @@ grub_ns8250_hw_get_port (const unsigned int unit)
>  }
>
>  char *
> -grub_serial_ns8250_add_port (grub_port_t port)
> +grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config 
> *config)
>  {
>    struct grub_serial_port *p;
>    unsigned i;
> @@ -312,6 +312,9 @@ grub_serial_ns8250_add_port (grub_port_t port)
>        {
>       if (dead_ports & (1 << i))
>         return NULL;
> +     /* give the opportunity for SPCR to configure a default com port */
> +     if (config)
> +       grub_serial_port_configure (&com_ports[i], config);
>       return com_names[i];
>        }
>
> @@ -333,22 +336,29 @@ grub_serial_ns8250_add_port (grub_port_t port)
>        return NULL;
>      }
>    p->driver = &grub_ns8250_driver;
> -  grub_serial_config_defaults (p);
>    p->mmio = 0;
>    p->port = port;
> +  if (config)
> +    grub_serial_port_configure (p, config);
> +  else
> +    grub_serial_config_defaults (p);
>    grub_serial_register (p);
>
>    return p->name;
>  }
>
>  char *
> -grub_serial_ns8250_add_mmio(grub_addr_t addr)
> +grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config 
> *config)

If you touch the code and its coding style is broken please fix it. Here
a space is missing before "(".

>  {
>    struct grub_serial_port *p;
>    unsigned i;
>    for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
> -    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
> -     return com_names[i];
> +    if (com_ports[i].mmio && com_ports[i].mmio_base == addr)
> +      {
> +        if (config)
> +          grub_serial_port_configure (&com_ports[i], config);
> +        return com_names[i];
> +      }
>
>    p = grub_malloc (sizeof (*p));
>    if (!p)
> @@ -360,9 +370,12 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr)
>        return NULL;
>      }
>    p->driver = &grub_ns8250_driver;
> -  grub_serial_config_defaults (p);
>    p->mmio = 1;
>    p->mmio_base = addr;
> +  if (config)
> +    grub_serial_port_configure (p, config);
> +  else
> +    grub_serial_config_defaults (p);
>    grub_serial_register (p);
>
>    return p->name;
> diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
> index c8f5f02db..a1707d2f7 100644
> --- a/grub-core/term/serial.c
> +++ b/grub-core/term/serial.c
> @@ -156,7 +156,7 @@ grub_serial_find (const char *name)
>        && grub_isxdigit (name [sizeof ("port") - 1]))
>      {
>        name = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof 
> ("port") - 1],
> -                                                     0, 16));
> +                                                     0, 16), NULL);
>        if (!name)
>       return NULL;
>
> diff --git a/include/grub/serial.h b/include/grub/serial.h
> index ccf464d41..7efc956b0 100644
> --- a/include/grub/serial.h
> +++ b/include/grub/serial.h
> @@ -185,8 +185,8 @@ grub_serial_config_defaults (struct grub_serial_port 
> *port)
>
>  #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
>  void grub_ns8250_init (void);
> -char *grub_serial_ns8250_add_port (grub_port_t port);
> -char *grub_serial_ns8250_add_mmio(grub_addr_t addr);
> +char *grub_serial_ns8250_add_port (grub_port_t port, struct 
> grub_serial_config *config);
> +char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct 
> grub_serial_config *config);

Again, please do not break coding style...

Daniel