[PATCH 1/2] x86: add a user configurable Kconfig option for the NS16550 UART

[PATCH 1/2] x86: add a user configurable Kconfig option for the NS16550 UART

[Derek Straka]

Allows for the conditional inclusion of NS16550 UART driver on the x86 platform
rather than having it always enabled.

The default configuration for the HAS_NS16550 option remains 'y' on x86, so the
behavior out of the box remains unchanged.  The addition of the option allows
advanced users to enable/disable the inclusion of the NS16550 UART driver.

Signed-off-by: Derek Straka <derek@asterius.io>
---
 xen/arch/x86/Kconfig     | 1 -
 xen/drivers/char/Kconfig | 4 ++--
 xen/include/xen/serial.h | 7 ++++++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 265fd79..8a122df 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -15,7 +15,6 @@ config X86
 	select HAS_MEM_ACCESS
 	select HAS_MEM_PAGING
 	select HAS_MEM_SHARING
-	select HAS_NS16550
 	select HAS_PASSTHROUGH
 	select HAS_PCI
 	select HAS_PDX
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index 51343d0..c87e018 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -1,6 +1,6 @@
 config HAS_NS16550
-	bool
-	default y
+	bool "NS16550 UART" if EXPERT = "y"
+	default y if X86
 	help
 	  This selects the 16550-series UART support. For most systems, say Y.
 
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 1212a12..343779c 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -167,9 +167,14 @@ struct ns16550_defaults {
     int irq;       /* default irq */
     unsigned long io_base; /* default io_base address */
 };
+
+#ifdef CONFIG_HAS_NS16550
 void ns16550_init(int index, struct ns16550_defaults *defaults);
-void ehci_dbgp_init(void);
+#else
+static inline void ns16550_init(int index, struct ns16550_defaults *defaults) {}
+#endif
 
+void ehci_dbgp_init(void);
 void arm_uart_init(void);
 
 struct physdev_dbgp_op;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/2] x86: add a user configurable Kconfig option for the EHCI UART

[Derek Straka]

Allows for the conditional inclusion of EHCI UART driver on the x86 platform
rather than having it always enabled.

The default configuration for the HAS_EHCI option remains 'y' on x86, so the
behavior out of the box remains unchanged.  The addition of the option allows
advanced users to enable/disable the inclusion of the EHCI UART driver.

Signed-off-by: Derek Straka <derek@asterius.io>
---
 xen/arch/x86/Kconfig     |  1 -
 xen/drivers/char/Kconfig |  3 ++-
 xen/include/xen/serial.h | 12 +++++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 8a122df..2119c93 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -8,7 +8,6 @@ config X86
 	select COMPAT
 	select CORE_PARKING
 	select HAS_CPUFREQ
-	select HAS_EHCI
 	select HAS_GDBSX
 	select HAS_IOPORTS
 	select HAS_KEXEC
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index c87e018..08a60e0 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -45,7 +45,8 @@ config HAS_SCIF
 	  say Y.
 
 config HAS_EHCI
-	bool
+	bool "EHCI UART" if EXPERT = "y"
+	default y if X86
 	help
 	  This selects the USB based EHCI debug port to be used as a UART. If
 	  you have an x86 based system with USB, say Y.
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 343779c..8f87897 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -174,11 +174,21 @@ void ns16550_init(int index, struct ns16550_defaults *defaults);
 static inline void ns16550_init(int index, struct ns16550_defaults *defaults) {}
 #endif
 
+#ifdef CONFIG_HAS_EHCI
 void ehci_dbgp_init(void);
-void arm_uart_init(void);
+#else
+static inline void ehci_dbgp_init(void) {}
+#endif
 
+void arm_uart_init(void);
+ 
 struct physdev_dbgp_op;
+
+#ifdef CONFIG_HAS_EHCI
 int dbgp_op(const struct physdev_dbgp_op *);
+#else
+static inline int dbgp_op(const struct physdev_dbgp_op *op) { return 0; }
+#endif
 
 /* Baud rate was pre-configured before invoking the UART driver. */
 #define BAUD_AUTO (-1)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] x86: add a user configurable Kconfig option for the NS16550 UART

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 507 bytes --]

On 13/09/16 18:35, Derek Straka wrote:
> Allows for the conditional inclusion of NS16550 UART driver on the x86 platform
> rather than having it always enabled.
>
> The default configuration for the HAS_NS16550 option remains 'y' on x86, so the
> behavior out of the box remains unchanged.  The addition of the option allows
> advanced users to enable/disable the inclusion of the NS16550 UART driver.
>
> Signed-off-by: Derek Straka <derek@asterius.io>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

[-- Attachment #1.2: Type: text/html, Size: 1056 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] x86: add a user configurable Kconfig option for the EHCI UART

[Andrew Cooper]

On 13/09/16 18:35, Derek Straka wrote:
> Allows for the conditional inclusion of EHCI UART driver on the x86 platform
> rather than having it always enabled.
>
> The default configuration for the HAS_EHCI option remains 'y' on x86, so the
> behavior out of the box remains unchanged.  The addition of the option allows
> advanced users to enable/disable the inclusion of the EHCI UART driver.
>
> Signed-off-by: Derek Straka <derek@asterius.io>
> ---
>  xen/arch/x86/Kconfig     |  1 -
>  xen/drivers/char/Kconfig |  3 ++-
>  xen/include/xen/serial.h | 12 +++++++++++-
>  3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 8a122df..2119c93 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -8,7 +8,6 @@ config X86
>  	select COMPAT
>  	select CORE_PARKING
>  	select HAS_CPUFREQ
> -	select HAS_EHCI
>  	select HAS_GDBSX
>  	select HAS_IOPORTS
>  	select HAS_KEXEC
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index c87e018..08a60e0 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -45,7 +45,8 @@ config HAS_SCIF
>  	  say Y.
>  
>  config HAS_EHCI
> -	bool
> +	bool "EHCI UART" if EXPERT = "y"
> +	default y if X86
>  	help
>  	  This selects the USB based EHCI debug port to be used as a UART. If
>  	  you have an x86 based system with USB, say Y.
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 343779c..8f87897 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -174,11 +174,21 @@ void ns16550_init(int index, struct ns16550_defaults *defaults);
>  static inline void ns16550_init(int index, struct ns16550_defaults *defaults) {}
>  #endif
>  
> +#ifdef CONFIG_HAS_EHCI
>  void ehci_dbgp_init(void);
> -void arm_uart_init(void);
> +#else
> +static inline void ehci_dbgp_init(void) {}
> +#endif

It would be cleaner to have ehci_dbgp_init() and dbgp_op() beside each
other in a single #ifdef CONFIG_HAS_EHCI

With this, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

>  
> +void arm_uart_init(void);
> + 
>  struct physdev_dbgp_op;
> +
> +#ifdef CONFIG_HAS_EHCI
>  int dbgp_op(const struct physdev_dbgp_op *);
> +#else
> +static inline int dbgp_op(const struct physdev_dbgp_op *op) { return 0; }
> +#endif
>  
>  /* Baud rate was pre-configured before invoking the UART driver. */
>  #define BAUD_AUTO (-1)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] x86: add a user configurable Kconfig option for the NS16550 UART

[Julien Grall]

Hello Derek,

On 13/09/16 18:35, Derek Straka wrote:
> Allows for the conditional inclusion of NS16550 UART driver on the x86 platform
> rather than having it always enabled.
>
> The default configuration for the HAS_NS16550 option remains 'y' on x86, so the
> behavior out of the box remains unchanged.  The addition of the option allows
> advanced users to enable/disable the inclusion of the NS16550 UART driver.
>
> Signed-off-by: Derek Straka <derek@asterius.io>
> ---
>  xen/arch/x86/Kconfig     | 1 -
>  xen/drivers/char/Kconfig | 4 ++--
>  xen/include/xen/serial.h | 7 ++++++-
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 265fd79..8a122df 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -15,7 +15,6 @@ config X86
>  	select HAS_MEM_ACCESS
>  	select HAS_MEM_PAGING
>  	select HAS_MEM_SHARING
> -	select HAS_NS16550
>  	select HAS_PASSTHROUGH
>  	select HAS_PCI
>  	select HAS_PDX
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index 51343d0..c87e018 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -1,6 +1,6 @@
>  config HAS_NS16550
> -	bool
> -	default y
> +	bool "NS16550 UART" if EXPERT = "y"
> +	default y if X86
>  	help
>  	  This selects the 16550-series UART support. For most systems, say Y.

With this change, NS166550 is now disabled by default on ARM. This was 
not the case before.

Please make sure to retain the previous behavior for ARM platforms.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] x86: add a user configurable Kconfig option for the EHCI UART

[Julien Grall]

Hello Derek,

On 13/09/16 18:35, Derek Straka wrote:
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index c87e018..08a60e0 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -45,7 +45,8 @@ config HAS_SCIF
>  	  say Y.
>
>  config HAS_EHCI
> -	bool
> +	bool "EHCI UART" if EXPERT = "y"
> +	default y if X86

HAS_EHCI should not be available for ARM. Otherwise you may break 
randomconfig build.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] x86: add a user configurable Kconfig option for the NS16550 UART

[Jan Beulich]

>>> On 13.09.16 at 19:35, <derek@asterius.io> wrote:
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -1,6 +1,6 @@
>  config HAS_NS16550
> -	bool
> -	default y
> +	bool "NS16550 UART" if EXPERT = "y"

This need more adjustment: HAS_* options should never have
prompts. I.e. you need to rename the config variable at once.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] x86: add a user configurable Kconfig option for the EHCI UART

[Jan Beulich]

>>> On 13.09.16 at 19:35, <derek@asterius.io> wrote:
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -45,7 +45,8 @@ config HAS_SCIF
>  	  say Y.
>  
>  config HAS_EHCI
> -	bool
> +	bool "EHCI UART" if EXPERT = "y"

Please make the prompt say e.g "EHCI debug port" - UART is not a
term commonly used with this hardware. And the variable renaming
comment from patch 1 applies here too, of course.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel