Re: [Xen-devel] [PATCH v2 2/2] x86: add a user configurable Kconfig option for the EHCI debug port

["Jan Beulich" <JBeulich@xxxxxxxx>]

> On 19/09/2016 17:37, Derek Straka wrote:
>> On Mon, Sep 19, 2016 at 10:56 AM, Julien Grall <julien.grall@xxxxxxx 
>> <mailto:julien.grall@xxxxxxx>> wrote:
>>     On 19/09/2016 16:51, Derek Straka wrote:
>>
>>         Allows for the conditional inclusion of EHCI debug port driver
>>         on the x86
>>         platform rather than having it always enabled.
>>
>>         The default configuration for the CONFIG_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 debug
>>         port driver.
>>
>>         Signed-off-by: Derek Straka <derek@xxxxxxxxxxx 
>>         <mailto:derek@xxxxxxxxxxx>>
>>         ---
>>          xen/drivers/char/Kconfig  |  5 +++++
>>          xen/drivers/char/Makefile |  2 +-
>>          xen/include/xen/serial.h  | 12 +++++++++---
>>          3 files changed, 15 insertions(+), 4 deletions(-)
>>
>>         diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
>>         index 1d894a7..1c5400f 100644
>>         --- a/xen/drivers/char/Kconfig
>>         +++ b/xen/drivers/char/Kconfig
>>         @@ -51,6 +51,11 @@ config HAS_SCIF
>>
>>          config HAS_EHCI
>>                 bool
>>         +
>>         +config EHCI
>>         +       bool "EHCI debug port" if EXPERT = "y"
>>         +       default y
>>         +       depends on HAS_EHCI
>>                 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/drivers/char/Makefile b/xen/drivers/char/Makefile
>>         index 0afadaf..40c193b 100644
>>         --- a/xen/drivers/char/Makefile
>>         +++ b/xen/drivers/char/Makefile
>>         @@ -5,6 +5,6 @@ obj-$(CONFIG_HAS_PL011) += pl011.o
>>          obj-$(CONFIG_HAS_EXYNOS4210) += exynos4210-uart.o
>>          obj-$(CONFIG_HAS_OMAP) += omap-uart.o
>>          obj-$(CONFIG_HAS_SCIF) += scif-uart.o
>>         -obj-$(CONFIG_HAS_EHCI) += ehci-dbgp.o
>>         +obj-$(CONFIG_EHCI) += ehci-dbgp.o
>>          obj-$(CONFIG_ARM) += arm-uart.o
>>          obj-y += serial.o
>>         diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
>>         index 46edff8..5c6cbe9 100644
>>         --- a/xen/include/xen/serial.h
>>         +++ b/xen/include/xen/serial.h
>>         @@ -174,11 +174,17 @@ void ns16550_init(int index, struct
>>         ns16550_defaults *defaults);
>>          static inline void ns16550_init(int index, struct
>>         ns16550_defaults *defaults) {}
>>          #endif
>>
>>         -void ehci_dbgp_init(void);
>>         -void arm_uart_init(void);
>>
>>
>>     Why did you move arm_uart_init? It does not seem related to this
>>     patch...
>>
>> I moved the EHCI code above the arm_uart_init since ehci_dbgp_init
>> preceded the arm code originally, but I can certainly move the ECHI
>> declarations after if you'd prefer.
> 
> I don't think we ever ordered code in the header files based on the 
> addition date. In general, we try to minimize the changes to make the 
> patch simpler and avoid been distract by meaningless things.
> 
> Anyway, I am not the maintainers of this code. So I will let Andrew and 
> Jan deciding.

Having looked a second time, I actually prefer the movement: I
consider it sub-optimal for arm_uart_init() to originally have got
placed in the middle between the two dbgp declarations.

Jan


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