LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] powerpc/legacy_serial: Use early_ioremap()
@ 2020-02-05 12:03 Christophe Leroy
  2020-03-23 21:54 ` Chris Packham
  0 siblings, 1 reply; 4+ messages in thread
From: Christophe Leroy @ 2020-02-05 12:03 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

[    0.000000] ioremap() called early from find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead

find_legacy_serial_ports() is called early from setup_arch(), before
paging_init(). vmalloc is not available yet, ioremap shouldn't be
used that early.

Use early_ioremap() and switch to a regular ioremap() later.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
index f061e06e9f51..8b2c1a8553a0 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -15,6 +15,7 @@
 #include <asm/udbg.h>
 #include <asm/pci-bridge.h>
 #include <asm/ppc-pci.h>
+#include <asm/early_ioremap.h>
 
 #undef DEBUG
 
@@ -34,6 +35,7 @@ static struct legacy_serial_info {
 	unsigned int			clock;
 	int				irq_check_parent;
 	phys_addr_t			taddr;
+	void __iomem			*early_addr;
 } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS];
 
 static const struct of_device_id legacy_serial_parents[] __initconst = {
@@ -325,17 +327,16 @@ static void __init setup_legacy_serial_console(int console)
 {
 	struct legacy_serial_info *info = &legacy_serial_infos[console];
 	struct plat_serial8250_port *port = &legacy_serial_ports[console];
-	void __iomem *addr;
 	unsigned int stride;
 
 	stride = 1 << port->regshift;
 
 	/* Check if a translated MMIO address has been found */
 	if (info->taddr) {
-		addr = ioremap(info->taddr, 0x1000);
-		if (addr == NULL)
+		info->early_addr = early_ioremap(info->taddr, 0x1000);
+		if (info->early_addr == NULL)
 			return;
-		udbg_uart_init_mmio(addr, stride);
+		udbg_uart_init_mmio(info->early_addr, stride);
 	} else {
 		/* Check if it's PIO and we support untranslated PIO */
 		if (port->iotype == UPIO_PORT && isa_io_special)
@@ -353,6 +354,30 @@ static void __init setup_legacy_serial_console(int console)
 	udbg_uart_setup(info->speed, info->clock);
 }
 
+static int __init ioremap_legacy_serial_console(void)
+{
+	struct legacy_serial_info *info = &legacy_serial_infos[legacy_serial_console];
+	struct plat_serial8250_port *port = &legacy_serial_ports[legacy_serial_console];
+	void __iomem *vaddr;
+
+	if (legacy_serial_console < 0)
+		return 0;
+
+	if (!info->early_addr)
+		return 0;
+
+	vaddr = ioremap(info->taddr, 0x1000);
+	if (WARN_ON(!vaddr))
+		return -ENOMEM;
+
+	udbg_uart_init_mmio(vaddr, 1 << port->regshift);
+	early_iounmap(info->early_addr, 0x1000);
+	info->early_addr = NULL;
+
+	return 0;
+}
+early_initcall(ioremap_legacy_serial_console);
+
 /*
  * This is called very early, as part of setup_system() or eventually
  * setup_arch(), basically before anything else in this file. This function
-- 
2.25.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/legacy_serial: Use early_ioremap()
  2020-02-05 12:03 [PATCH] powerpc/legacy_serial: Use early_ioremap() Christophe Leroy
@ 2020-03-23 21:54 ` Chris Packham
  2020-08-10  2:01   ` Chris Packham
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Packham @ 2020-03-23 21:54 UTC (permalink / raw)
  To: christophe.leroy, paulus, mpe, benh
  Cc: Hamish Martin, linuxppc-dev, linux-kernel

Hi Christophe,

On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote:
> [    0.000000] ioremap() called early from
> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead
> 

I was just about to dig into this error message and found you patch. I
applied it to a v5.5 base.

> find_legacy_serial_ports() is called early from setup_arch(), before
> paging_init(). vmalloc is not available yet, ioremap shouldn't be
> used that early.
> 
> Use early_ioremap() and switch to a regular ioremap() later.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

On my system (Freescale T2080 SOC) this seems to cause a crash/hang in
early boot. Unfortunately because this is affecting the boot console I
don't get any earlyprintk output.

> ---
>  arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++
> ----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/legacy_serial.c
> b/arch/powerpc/kernel/legacy_serial.c
> index f061e06e9f51..8b2c1a8553a0 100644
> --- a/arch/powerpc/kernel/legacy_serial.c
> +++ b/arch/powerpc/kernel/legacy_serial.c
> @@ -15,6 +15,7 @@
>  #include <asm/udbg.h>
>  #include <asm/pci-bridge.h>
>  #include <asm/ppc-pci.h>
> +#include <asm/early_ioremap.h>
>  
>  #undef DEBUG
>  
> @@ -34,6 +35,7 @@ static struct legacy_serial_info {
>  	unsigned int			clock;
>  	int				irq_check_parent;
>  	phys_addr_t			taddr;
> +	void __iomem			*early_addr;
>  } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS];
>  
>  static const struct of_device_id legacy_serial_parents[] __initconst
> = {
> @@ -325,17 +327,16 @@ static void __init
> setup_legacy_serial_console(int console)
>  {
>  	struct legacy_serial_info *info =
> &legacy_serial_infos[console];
>  	struct plat_serial8250_port *port =
> &legacy_serial_ports[console];
> -	void __iomem *addr;
>  	unsigned int stride;
>  
>  	stride = 1 << port->regshift;
>  
>  	/* Check if a translated MMIO address has been found */
>  	if (info->taddr) {
> -		addr = ioremap(info->taddr, 0x1000);
> -		if (addr == NULL)
> +		info->early_addr = early_ioremap(info->taddr, 0x1000);
> +		if (info->early_addr == NULL)
>  			return;
> -		udbg_uart_init_mmio(addr, stride);
> +		udbg_uart_init_mmio(info->early_addr, stride);
>  	} else {
>  		/* Check if it's PIO and we support untranslated PIO */
>  		if (port->iotype == UPIO_PORT && isa_io_special)
> @@ -353,6 +354,30 @@ static void __init
> setup_legacy_serial_console(int console)
>  	udbg_uart_setup(info->speed, info->clock);
>  }
>  
> +static int __init ioremap_legacy_serial_console(void)
> +{
> +	struct legacy_serial_info *info =
> &legacy_serial_infos[legacy_serial_console];
> +	struct plat_serial8250_port *port =
> &legacy_serial_ports[legacy_serial_console];
> +	void __iomem *vaddr;
> +
> +	if (legacy_serial_console < 0)
> +		return 0;
> +
> +	if (!info->early_addr)
> +		return 0;
> +
> +	vaddr = ioremap(info->taddr, 0x1000);
> +	if (WARN_ON(!vaddr))
> +		return -ENOMEM;
> +
> +	udbg_uart_init_mmio(vaddr, 1 << port->regshift);
> +	early_iounmap(info->early_addr, 0x1000);
> +	info->early_addr = NULL;
> +
> +	return 0;
> +}
> +early_initcall(ioremap_legacy_serial_console);
> +
>  /*
>   * This is called very early, as part of setup_system() or
> eventually
>   * setup_arch(), basically before anything else in this file. This
> function

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/legacy_serial: Use early_ioremap()
  2020-03-23 21:54 ` Chris Packham
@ 2020-08-10  2:01   ` Chris Packham
  2021-04-20  8:18     ` Christophe Leroy
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Packham @ 2020-08-10  2:01 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: Hamish Martin, linuxppc-dev, linux-kernel


On 24/03/20 10:54 am, Chris Packham wrote:
> Hi Christophe,
>
> On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote:
>> [    0.000000] ioremap() called early from
>> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead
>>
> I was just about to dig into this error message and found you patch. I
> applied it to a v5.5 base.
>
>> find_legacy_serial_ports() is called early from setup_arch(), before
>> paging_init(). vmalloc is not available yet, ioremap shouldn't be
>> used that early.
>>
>> Use early_ioremap() and switch to a regular ioremap() later.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> On my system (Freescale T2080 SOC) this seems to cause a crash/hang in
> early boot. Unfortunately because this is affecting the boot console I
> don't get any earlyprintk output.

I've been doing a bit more digging into why Christophe's patch didn't 
work for me. I noticed the powerpc specific early_ioremap_range() 
returns addresses around ioremap_bot. Yet the generic early_ioremap() 
uses addresses around FIXADDR_TOP. If I try the following hack I can 
make Christophe's patch work

diff --git a/arch/powerpc/include/asm/fixmap.h 
b/arch/powerpc/include/asm/fixmap.h
index 2ef155a3c821..7bc2f3f73c8b 100644
--- a/arch/powerpc/include/asm/fixmap.h
+++ b/arch/powerpc/include/asm/fixmap.h
@@ -27,7 +27,7 @@
  #include <asm/kasan.h>
  #define FIXADDR_TOP    (KASAN_SHADOW_START - PAGE_SIZE)
  #else
-#define FIXADDR_TOP    ((unsigned long)(-PAGE_SIZE))
+#define FIXADDR_TOP    (IOREMAP_END - PAGE_SIZE)
  #endif

  /*

I'll admit to being out of my depth. It seems that the generic 
early_ioremap() is not quite correctly plumbed in for powerpc.

>> ---
>>   arch/powerpc/kernel/legacy_serial.c | 33 +++++++++++++++++++++++++
>> ----
>>   1 file changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/legacy_serial.c
>> b/arch/powerpc/kernel/legacy_serial.c
>> index f061e06e9f51..8b2c1a8553a0 100644
>> --- a/arch/powerpc/kernel/legacy_serial.c
>> +++ b/arch/powerpc/kernel/legacy_serial.c
>> @@ -15,6 +15,7 @@
>>   #include <asm/udbg.h>
>>   #include <asm/pci-bridge.h>
>>   #include <asm/ppc-pci.h>
>> +#include <asm/early_ioremap.h>
>>   
>>   #undef DEBUG
>>   
>> @@ -34,6 +35,7 @@ static struct legacy_serial_info {
>>   	unsigned int			clock;
>>   	int				irq_check_parent;
>>   	phys_addr_t			taddr;
>> +	void __iomem			*early_addr;
>>   } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS];
>>   
>>   static const struct of_device_id legacy_serial_parents[] __initconst
>> = {
>> @@ -325,17 +327,16 @@ static void __init
>> setup_legacy_serial_console(int console)
>>   {
>>   	struct legacy_serial_info *info =
>> &legacy_serial_infos[console];
>>   	struct plat_serial8250_port *port =
>> &legacy_serial_ports[console];
>> -	void __iomem *addr;
>>   	unsigned int stride;
>>   
>>   	stride = 1 << port->regshift;
>>   
>>   	/* Check if a translated MMIO address has been found */
>>   	if (info->taddr) {
>> -		addr = ioremap(info->taddr, 0x1000);
>> -		if (addr == NULL)
>> +		info->early_addr = early_ioremap(info->taddr, 0x1000);
>> +		if (info->early_addr == NULL)
>>   			return;
>> -		udbg_uart_init_mmio(addr, stride);
>> +		udbg_uart_init_mmio(info->early_addr, stride);
>>   	} else {
>>   		/* Check if it's PIO and we support untranslated PIO */
>>   		if (port->iotype == UPIO_PORT && isa_io_special)
>> @@ -353,6 +354,30 @@ static void __init
>> setup_legacy_serial_console(int console)
>>   	udbg_uart_setup(info->speed, info->clock);
>>   }
>>   
>> +static int __init ioremap_legacy_serial_console(void)
>> +{
>> +	struct legacy_serial_info *info =
>> &legacy_serial_infos[legacy_serial_console];
>> +	struct plat_serial8250_port *port =
>> &legacy_serial_ports[legacy_serial_console];
>> +	void __iomem *vaddr;
>> +
>> +	if (legacy_serial_console < 0)
>> +		return 0;
>> +
>> +	if (!info->early_addr)
>> +		return 0;
>> +
>> +	vaddr = ioremap(info->taddr, 0x1000);
>> +	if (WARN_ON(!vaddr))
>> +		return -ENOMEM;
>> +
>> +	udbg_uart_init_mmio(vaddr, 1 << port->regshift);
>> +	early_iounmap(info->early_addr, 0x1000);
>> +	info->early_addr = NULL;
>> +
>> +	return 0;
>> +}
>> +early_initcall(ioremap_legacy_serial_console);
>> +
>>   /*
>>    * This is called very early, as part of setup_system() or
>> eventually
>>    * setup_arch(), basically before anything else in this file. This
>> function

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/legacy_serial: Use early_ioremap()
  2020-08-10  2:01   ` Chris Packham
@ 2021-04-20  8:18     ` Christophe Leroy
  0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2021-04-20  8:18 UTC (permalink / raw)
  To: Chris Packham, Christophe Leroy, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman
  Cc: Hamish Martin, linuxppc-dev, linux-kernel

Hi Chris,

Le 10/08/2020 à 04:01, Chris Packham a écrit :
> 
> On 24/03/20 10:54 am, Chris Packham wrote:
>> Hi Christophe,
>>
>> On Wed, 2020-02-05 at 12:03 +0000, Christophe Leroy wrote:
>>> [    0.000000] ioremap() called early from
>>> find_legacy_serial_ports+0x3cc/0x474. Use early_ioremap() instead
>>>
>> I was just about to dig into this error message and found you patch. I
>> applied it to a v5.5 base.
>>
>>> find_legacy_serial_ports() is called early from setup_arch(), before
>>> paging_init(). vmalloc is not available yet, ioremap shouldn't be
>>> used that early.
>>>
>>> Use early_ioremap() and switch to a regular ioremap() later.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> On my system (Freescale T2080 SOC) this seems to cause a crash/hang in
>> early boot. Unfortunately because this is affecting the boot console I
>> don't get any earlyprintk output.
> 
> I've been doing a bit more digging into why Christophe's patch didn't
> work for me. I noticed the powerpc specific early_ioremap_range()
> returns addresses around ioremap_bot. Yet the generic early_ioremap()
> uses addresses around FIXADDR_TOP. If I try the following hack I can
> make Christophe's patch work
> 
> diff --git a/arch/powerpc/include/asm/fixmap.h
> b/arch/powerpc/include/asm/fixmap.h
> index 2ef155a3c821..7bc2f3f73c8b 100644
> --- a/arch/powerpc/include/asm/fixmap.h
> +++ b/arch/powerpc/include/asm/fixmap.h
> @@ -27,7 +27,7 @@
>    #include <asm/kasan.h>
>    #define FIXADDR_TOP    (KASAN_SHADOW_START - PAGE_SIZE)
>    #else
> -#define FIXADDR_TOP    ((unsigned long)(-PAGE_SIZE))
> +#define FIXADDR_TOP    (IOREMAP_END - PAGE_SIZE)
>    #endif
> 
>    /*
> 
> I'll admit to being out of my depth. It seems that the generic
> early_ioremap() is not quite correctly plumbed in for powerpc.

Yes that's probably true for PPC64.

I see that on PPC32 I had to implement the following changes in order to enable earlier use of 
early_ioremap()

https://github.com/torvalds/linux/commit/925ac141d106b55acbe112a9272f970631a3c082


I have the problem with QEMU with the ppce500 machine. It will allow me to investigate it a bit further.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-05 12:03 [PATCH] powerpc/legacy_serial: Use early_ioremap() Christophe Leroy
2020-03-23 21:54 ` Chris Packham
2020-08-10  2:01   ` Chris Packham
2021-04-20  8:18     ` Christophe Leroy

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git