Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-13 Thread Stefano Stabellini
On Thu, 12 Mar 2020, Julien Grall wrote:
> On 11/03/2020 14:46, Anthony PERARD wrote:
> > On Wed, Mar 11, 2020 at 01:57:37PM +, Julien Grall wrote:
> > > Hi Anthony,
> > > 
> > > On 09/03/2020 17:45, Anthony PERARD wrote:
> > > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > > > index e9d356f05c2b..2b593c5ef99a 100644
> > > > --- a/xen/arch/arm/arm32/head.S
> > > > +++ b/xen/arch/arm/arm32/head.S
> > > > @@ -36,8 +36,8 @@
> > > >#define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
> > > >#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> > > > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > > > -#include EARLY_PRINTK_INC
> > > > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined
> > > > (CONFIG_EARLY_PRINTK_INC))
> > > 
> > > NIT: I would also take the opportunity to clean-up the line by remove the
> > > extra () and the space before (. Something like:
> > > 
> > > #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
> > > 
> > > [...]
> > > 
> > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > > > index e5015f93a2d8..4d45ea3dac3c 100644
> > > > --- a/xen/arch/arm/arm64/head.S
> > > > +++ b/xen/arch/arm/arm64/head.S
> > > > @@ -45,8 +45,8 @@
> > > >#define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
> > > > (__HEAD_FLAG_PHYS_BASE << 3))
> > > > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > > > -#include EARLY_PRINTK_INC
> > > > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined
> > > > (CONFIG_EARLY_PRINTK_INC))
> > > 
> > > Same here.
> > 
> > Those clean-up sounds good.
> 
> I will give a couple of days so Stefano can have an opportunity to comment. If
> I don't hear anything by Monday, I will commit it.

It's fine by me. Also:

Tested-by: Stefano Stabellini 

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

Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-12 Thread Julien Grall

Hi,

On 11/03/2020 14:46, Anthony PERARD wrote:

On Wed, Mar 11, 2020 at 01:57:37PM +, Julien Grall wrote:

Hi Anthony,

On 09/03/2020 17:45, Anthony PERARD wrote:

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index e9d356f05c2b..2b593c5ef99a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -36,8 +36,8 @@
   #define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
   #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


NIT: I would also take the opportunity to clean-up the line by remove the
extra () and the space before (. Something like:

#if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)

[...]


diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index e5015f93a2d8..4d45ea3dac3c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -45,8 +45,8 @@
   #define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
(__HEAD_FLAG_PHYS_BASE << 3))
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


Same here.


Those clean-up sounds good.


I will give a couple of days so Stefano can have an opportunity to 
comment. If I don't hear anything by Monday, I will commit it.





I am happy to fix both cases on commit:

Acked-by: Julien Grall 


Thanks,



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-11 Thread Anthony PERARD
On Wed, Mar 11, 2020 at 01:57:37PM +, Julien Grall wrote:
> Hi Anthony,
> 
> On 09/03/2020 17:45, Anthony PERARD wrote:
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index e9d356f05c2b..2b593c5ef99a 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -36,8 +36,8 @@
> >   #define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
> >   #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
> 
> NIT: I would also take the opportunity to clean-up the line by remove the
> extra () and the space before (. Something like:
> 
> #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
> 
> [...]
> 
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index e5015f93a2d8..4d45ea3dac3c 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -45,8 +45,8 @@
> >   #define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
> >(__HEAD_FLAG_PHYS_BASE << 3))
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
> 
> Same here.

Those clean-up sounds good.

> I am happy to fix both cases on commit:
>
> Acked-by: Julien Grall 

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-11 Thread Julien Grall

Hi Anthony,

On 09/03/2020 17:45, Anthony PERARD wrote:

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index e9d356f05c2b..2b593c5ef99a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -36,8 +36,8 @@
  #define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
  #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
  
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))

-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


NIT: I would also take the opportunity to clean-up the line by remove 
the extra () and the space before (. Something like:


#if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)

[...]


diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index e5015f93a2d8..4d45ea3dac3c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -45,8 +45,8 @@
  #define __HEAD_FLAGS((__HEAD_FLAG_PAGE_SIZE << 1) | \
   (__HEAD_FLAG_PHYS_BASE << 3))
  
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))

-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))


Same here.

I am happy to fix both cases on commit:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

[Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro

2020-03-09 Thread Anthony PERARD
We are going to move the generation of the early printk macro into
Kconfig. This means all macro will be prefix with CONFIG_. We do that
ahead of the change.

We also take the opportunity to better name some variables, which are
used by only one driver and wouldn't make sens for other UART driver.
Thus,
- EARLY_UART_REG_SHIFT became CONFIG_EARLY_UART_8250_REG_SHIFT
- EARLY_PRINTK_VERSION_* became CONFIG_EARLY_UART_SCIF_VERSION_*

The other variables are change to have the prefix CONFIG_EARLY_UART_
when they change a parameter of the driver. So we have now:
- CONFIG_EARLY_UART_BAUD_RATE
- CONFIG_EARLY_UART_BASE_ADDRESS
- CONFIG_EARLY_UART_INIT

Signed-off-by: Anthony PERARD 
---
That's based on early work by Julien
[PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk 
using Kconfig
<20190913103953.8182-1-julien.gr...@arm.com>
---

Notes:
v3:
- Revert the renaming of EARLY_PRINTK to CONFIG_EARLY_PRINTK in the
  makefiles, as this doesn't work well with user provided
  CONFIG_EARLY_PRINTK.
  This is done in the following patch instead.

- rename CONFIG_EARLY_UART_BAUD_RATE to CONFIG_EARLY_UART_PL011_BAUD_RATE

 xen/arch/arm/Rules.mk  | 14 +++---
 xen/arch/arm/arm32/debug-8250.inc  |  2 +-
 xen/arch/arm/arm32/debug-pl011.inc |  4 ++--
 xen/arch/arm/arm32/debug-scif.inc  |  4 ++--
 xen/arch/arm/arm32/debug.S |  4 ++--
 xen/arch/arm/arm32/head.S  | 10 +-
 xen/arch/arm/arm64/debug-8250.inc  |  4 ++--
 xen/arch/arm/arm64/debug-pl011.inc |  4 ++--
 xen/arch/arm/arm64/debug.S |  4 ++--
 xen/arch/arm/arm64/head.S  | 10 +-
 xen/include/asm-arm/early_printk.h |  2 +-
 11 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 022a3a6f82ba..faa09ea111ec 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -66,9 +66,9 @@ endif
 endif
 ifeq ($(EARLY_PRINTK_INC),scif)
 ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-CFLAGS-y += -DEARLY_PRINTK_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
+CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
 else
-CFLAGS-y += -DEARLY_PRINTK_VERSION_NONE
+CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_NONE
 endif
 endif
 
@@ -77,11 +77,11 @@ EARLY_PRINTK := y
 endif
 
 CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
-CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
+CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
+CFLAGS-$(EARLY_PRINTK) += 
-DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
+CFLAGS-$(EARLY_PRINTK) += 
-DCONFIG_EARLY_UART_PL011_BAUD_RATE=$(EARLY_PRINTK_BAUD)
+CFLAGS-$(EARLY_PRINTK) += 
-DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
+CFLAGS-$(EARLY_PRINTK) += 
-DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
 
 else # !CONFIG_DEBUG
 
diff --git a/xen/arch/arm/arm32/debug-8250.inc 
b/xen/arch/arm/arm32/debug-8250.inc
index 0759a27ee157..c47e8be4aaf3 100644
--- a/xen/arch/arm/arm32/debug-8250.inc
+++ b/xen/arch/arm/arm32/debug-8250.inc
@@ -23,7 +23,7 @@
  */
 .macro early_uart_ready rb rc
 1:
-ldr \rc, [\rb, #(UART_LSR << EARLY_UART_REG_SHIFT)] /* Read LSR */
+ldr \rc, [\rb, #(UART_LSR << CONFIG_EARLY_UART_8250_REG_SHIFT)] /* 
Read LSR */
 tst \rc, #UART_LSR_THRE /* Check Xmit holding register flag */
 beq 1b /* Wait for the UART to be ready */
 .endm
diff --git a/xen/arch/arm/arm32/debug-pl011.inc 
b/xen/arch/arm/arm32/debug-pl011.inc
index ec462eabab5c..214f68dc95bd 100644
--- a/xen/arch/arm/arm32/debug-pl011.inc
+++ b/xen/arch/arm/arm32/debug-pl011.inc
@@ -25,9 +25,9 @@
  * rd: scratch register 2 (unused here)
  */
 .macro early_uart_init rb, rc, rd
-mov   \rc, #(7372800 / EARLY_PRINTK_BAUD % 16)
+mov   \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE % 16)
 str   \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) */
-mov   \rc, #(7372800 / EARLY_PRINTK_BAUD / 16)
+mov   \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE / 16)
 str   \rc, [\rb, #IBRD] /* -> UARTIBRD (Baud divisor integer) */
 mov   \rc, #0x60/* 8n1 */
 str   \rc, [\rb, #LCR_H] /* -> UARTLCR_H (Line control) */
diff --git a/xen/arch/arm/arm32/debug-scif.inc 
b/xen/arch/arm/arm32/debug-scif.inc
index 3f01c909c238..b2b82501e792 100644
--- a/xen/arch/arm/arm32/debug-scif.inc
+++ b/xen/arch/arm/arm32/debug-scif.inc
@@ -19,10 +19,10 @@
 
 #include 
 
-#ifdef EARLY_PRINTK_VERSION_NONE
+#ifdef CONFIG_EARLY_UART_SCIF_VERSION_NONE
 #define STATUS_REGSCIF_SCFSR
 #define