Re: [PATCH v2 1/2] serial_core: add pci uart early console support

[One Thousand Gnomes]

> +/* x86 uses "earlyprintk=xxx", so we keep the compatibility here */
> +#ifdef CONFIG_X86
> +early_param("earlyprintk", param_setup_earlycon);
> +#else
>  early_param("earlycon", param_setup_earlycon);
> +#endif

Is there any reason for not just allowing both ? That way you don't need
an ifdef and the same command line (either) works everywhere ?

>  
>  int __init of_setup_earlycon(unsigned long addr,
>  			     int (*setup)(struct earlycon_device *, const char *))
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 0b7bb12..221143c 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -34,10 +34,16 @@
>  #include <linux/serial_core.h>
>  #include <linux/delay.h>
>  #include <linux/mutex.h>
> +#include <linux/pci_regs.h>
>  
>  #include <asm/irq.h>
>  #include <asm/uaccess.h>
>  
> +/* Only x86 has early pci access APIs */
> +#if defined(CONFIG_PCI) && defined(CONFIG_X86)
> +#include <asm/pci-direct.h>
> +#endif

We should probably make that a define or Kconfig value so that any other
port wanting early PCI console can add the needed direct functions
without the defines here getting longer and longer

It would then be a single

#if defined(CONFIG_EARLY_PCI)

that could be put anywhere


> +static int parse_pci_options(char *options, unsigned long *phys)
> +{
> +	u8 bus, dev, func;

...

> +failed:
> +	pr_err("Invalid earlycon pci parameters\n");
> +	return -EINVAL;
> +}

#else
static int parse_pci_options(char *options, unsigned long *phys)
{
	pr_err("earlycon pci not available\n");
	return -EINVAL;
}
> +#endif
> +


The all the ifdef mess below goes away

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/