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

2016-09-20 Thread Jan Beulich 
> On 19/09/2016 17:37, Derek Straka wrote:
>> On Mon, Sep 19, 2016 at 10:56 AM, Julien Grall > > 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 > >
>> ---
>>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

2016-09-20 Thread Jan Beulich 
>>> On 19.09.16 at 16:51,  wrote:
> --- 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

While here HAS_EHCI doesn't get left completely useless, I'd still
prefer for it to be dropped, and the select(s) converted to ordinary
dependencies put here. Also please name the variable e.g.
EHCI_DBGP, as there's no full EHCI support in Xen (this was not
really of interest for HAS_EHCI).

Jan


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

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

2016-09-20 Thread Jan Beulich 
>>> On 20.09.16 at 09:42,  wrote:
> Anyway, I am not the maintainers of this code. So I will let Andrew and 
> Jan deciding.

I'm in general against needless moving around of code, but for the
one line in question here I don't think it matters all that much where
exactly it lives.

Jan


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

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

2016-09-20 Thread Julien Grall 



On 19/09/2016 17:37, Derek Straka wrote:

Julien,

On Mon, Sep 19, 2016 at 10:56 AM, Julien Grall > wrote:

Hello,

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 >
---
 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.


Regards,

--
Julien Grall

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

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

2016-09-19 Thread Derek Straka 
Julien,

On Mon, Sep 19, 2016 at 10:56 AM, Julien Grall  wrote:

> Hello,
>
> 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 
>> ---
>>  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.

>
> -
>>  struct physdev_dbgp_op;
>> +
>> +#ifdef CONFIG_EHCI
>>  int dbgp_op(const struct physdev_dbgp_op *);
>> +void ehci_dbgp_init(void);
>> +#else
>> +static inline void ehci_dbgp_init(void) {}
>> +static inline int dbgp_op(const struct physdev_dbgp_op *op) { return 0; }
>> +#endif
>> +
>> +void arm_uart_init(void);
>>
>>  /* Baud rate was pre-configured before invoking the UART driver. */
>>  #define BAUD_AUTO (-1)
>>
>>
> Regards,
>
> --
> Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2016-09-19 Thread Julien Grall 

Hello,

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 
---
 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...


-
 struct physdev_dbgp_op;
+
+#ifdef CONFIG_EHCI
 int dbgp_op(const struct physdev_dbgp_op *);
+void ehci_dbgp_init(void);
+#else
+static inline void ehci_dbgp_init(void) {}
+static inline int dbgp_op(const struct physdev_dbgp_op *op) { return 0; }
+#endif
+
+void arm_uart_init(void);

 /* Baud rate was pre-configured before invoking the UART driver. */
 #define BAUD_AUTO (-1)



Regards,

--
Julien Grall

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

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

2016-09-19 Thread Derek Straka 
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 
---
 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);
-
 struct physdev_dbgp_op;
+
+#ifdef CONFIG_EHCI
 int dbgp_op(const struct physdev_dbgp_op *);
+void ehci_dbgp_init(void);
+#else
+static inline void ehci_dbgp_init(void) {}
+static inline int dbgp_op(const struct physdev_dbgp_op *op) { return 0; }
+#endif
+
+void arm_uart_init(void);
 
 /* 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