[RFC/PATCH 0/3] Attempt at making 32bit BAT assignment more intelligent

[RFC/PATCH 0/3] Attempt at making 32bit BAT assignment more intelligent

[Grant Likely]

Here is my attempt to make BAT allocations dynamic instead of hard coded.
The first patch changes setbat() to dynamically assign BATs instead of requiring
the caller to select a BAT on its own.

A primary user of setbat is mmu_mapin_ram() which used to hard code BATs
2 and 3 for mapping as much of RAM as can fit in 2 BATs.  The first patch
changes the routine to try to use as many BATs as it needs to map all of RAM.
(Note: I've still got BAT0 reserved for mapping RAM, so even if lots of other
users of setbat() appear, RAM is guaranteed to be allocated at least 1 BAT).

The first patch also adds an ioremap_bat() function which works like
ioremap(), but uses BATs instead of page tables and can be called really
early (before MMU_init()).  ioremap_bat() mappings persist after MMU_init
is called too so it can be used to map all of an SoC's IMMR region, or
any other IO device for that matter.  I've seen about a 2.5% performance
improvement by using this on a simple network workload with SoC registers
BAT mapped.

The second patch is just a utility function required by the third patch.

The third patch is a new user of ioremap_bat() to implement early debug
support for the mpc5200 platform.

The first patch is the one I really want feedback on.  It shouldn't break
any 32 bit platforms, but I need testing to make sure.  Also, this is my
first attempt at messing with MMU code, so please let me know if I'm doing
anything foolish or dangerous.

Kumar, Josh; I'd appreciate testing to make sure patch 1 doesn't break stuff.

Thanks,
g.

--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.

[RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

ioremap_bat() is useful for things like mapping SoC internally memory mapped
register and early text because it allows mappings to devices to be setup
early in the boot process where they are needed, and the mappings persist
after the MMU is configured.

Without ioremap_bat(), setting up the MMU would cause the early text
mappings to get lost and mostly likely result in a kernel panic on the next
attempt at output.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/kernel/setup_32.c   |    9 ++
 arch/powerpc/mm/init_32.c        |    7 --
 arch/powerpc/mm/mmu_decl.h       |    4 +
 arch/powerpc/mm/pgtable_32.c     |    2 -
 arch/powerpc/mm/ppc_mmu_32.c     |  148 ++++++++++++++++++++++++++++++++------
 arch/powerpc/sysdev/cpm_common.c |    2 -
 include/asm-powerpc/pgalloc-32.h |    2 +
 7 files changed, 140 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 066e65c..7b25b57 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -113,6 +113,15 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
  */
 notrace void __init machine_init(unsigned long dt_ptr, unsigned long phys)
 {
+	/* Do the bare minimum to start allocting from the IO region so
+	 * that ioremap_bat() works */
+#ifdef CONFIG_HIGHMEM
+	ioremap_base = PKMAP_BASE;
+#else
+	ioremap_base = 0xfe000000UL;	/* for now, could be 0xfffff000 */
+#endif /* CONFIG_HIGHMEM */
+	ioremap_bot = ioremap_base;
+
 	/* Enable early debugging if any specified (see udbg.h) */
 	udbg_early_init();
 
diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 388ceda..a3d9b4e 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -169,13 +169,6 @@ void __init MMU_init(void)
 		ppc_md.progress("MMU:mapin", 0x301);
 	mapin_ram();
 
-#ifdef CONFIG_HIGHMEM
-	ioremap_base = PKMAP_BASE;
-#else
-	ioremap_base = 0xfe000000UL;	/* for now, could be 0xfffff000 */
-#endif /* CONFIG_HIGHMEM */
-	ioremap_bot = ioremap_base;
-
 	/* Map in I/O resources */
 	if (ppc_md.progress)
 		ppc_md.progress("MMU:setio", 0x302);
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index fab3cfa..5027736 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -29,8 +29,8 @@ extern void hash_preload(struct mm_struct *mm, unsigned long ea,
 #ifdef CONFIG_PPC32
 extern void mapin_ram(void);
 extern int map_page(unsigned long va, phys_addr_t pa, int flags);
-extern void setbat(int index, unsigned long virt, phys_addr_t phys,
-		   unsigned int size, int flags);
+extern int setbat(unsigned long virt, phys_addr_t phys,
+		  unsigned int size, int flags);
 extern void settlbcam(int index, unsigned long virt, phys_addr_t phys,
 		      unsigned int size, int flags, unsigned int pid);
 extern void invalidate_tlbcam_entry(int index);
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 2001abd..e96f745 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -55,8 +55,6 @@ extern void hash_page_sync(void);
 #ifdef HAVE_BATS
 extern phys_addr_t v_mapped_by_bats(unsigned long va);
 extern unsigned long p_mapped_by_bats(phys_addr_t pa);
-void setbat(int index, unsigned long virt, phys_addr_t phys,
-	    unsigned int size, int flags);
 
 #else /* !HAVE_BATS */
 #define v_mapped_by_bats(x)	(0UL)
diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index c53145f..62c4603 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -72,41 +72,44 @@ unsigned long p_mapped_by_bats(phys_addr_t pa)
 	return 0;
 }
 
+/**
+ * mmu_mapin_ram - Map as much of RAM as possible into kernel space using BATs
+ */
 unsigned long __init mmu_mapin_ram(void)
 {
 #ifdef CONFIG_POWER4
 	return 0;
 #else
 	unsigned long tot, bl, done;
-	unsigned long max_size = (256<<20);
+	int rc;
 
 	if (__map_without_bats) {
 		printk(KERN_DEBUG "RAM mapped without BATs\n");
 		return 0;
 	}
 
-	/* Set up BAT2 and if necessary BAT3 to cover RAM. */
-
-	/* Make sure we don't map a block larger than the
-	   smallest alignment of the physical address. */
+	/* Set up BATs to cover RAM. */
 	tot = total_lowmem;
-	for (bl = 128<<10; bl < max_size; bl <<= 1) {
-		if (bl * 2 > tot)
+	done = 0;
+	while (done < tot) {
+		/* determine the smallest block size need to map the region.
+		 * Don't use a BAT mapping if the remaining region is less
+		 * that 128k */
+		if (tot - done <= 128<<10)
 			break;
-	}
-
-	setbat(2, KERNELBASE, 0, bl, _PAGE_RAM);
-	done = (unsigned long)bat_addrs[2].limit - KERNELBASE + 1;
-	if ((done < tot) && !bat_addrs[3].limit) {
-		/* use BAT3 to cover a bit more */
-		tot -= done;
-		for (bl = 128<<10; bl < max_size; bl <<= 1)
-			if (bl * 2 > tot)
+		for (bl = 128<<10; bl < (256<<20); bl <<= 1)
+			if ((bl * 2) > (tot - done))
 				break;
-		setbat(3, KERNELBASE+done, done, bl, _PAGE_RAM);
-		done = (unsigned long)bat_addrs[3].limit - KERNELBASE + 1;
+
+		/* Allocate the BAT and recalculate amount of RAM mapped */
+		rc = setbat(KERNELBASE+done, done, bl, _PAGE_RAM);
+		if (rc < 0)
+			break;
+		done = (unsigned long)bat_addrs[rc].limit - KERNELBASE + 1;
 	}
 
+	if (done == 0)
+		printk(KERN_CRIT "Weird; No BATs available for RAM.\n");
 	return done;
 #endif
 }
@@ -116,12 +119,29 @@ unsigned long __init mmu_mapin_ram(void)
  * The parameters are not checked; in particular size must be a power
  * of 2 between 128k and 256M.
  */
-void __init setbat(int index, unsigned long virt, phys_addr_t phys,
-		   unsigned int size, int flags)
+int __init setbat(unsigned long virt, phys_addr_t phys,
+		  unsigned int size, int flags)
 {
 	unsigned int bl;
-	int wimgxpp;
-	struct ppc_bat *bat = BATS[index];
+	int wimgxpp, index, nr_bats;
+	struct ppc_bat *bat;
+
+	/* Find a free BAT
+	 *
+	 * Special case; Keep the first entry in reserve for mapping RAM.
+	 * Otherwise the too many other users can prevent RAM from getting
+	 * mapped at all with a BAT.
+	 */
+	index = (flags == _PAGE_RAM) ? 0 : 1;
+	nr_bats = cpu_has_feature(CPU_FTR_HAS_HIGH_BATS) ? 8 : 4;
+	for (; index < nr_bats; index++) {
+		if ((BATS[index][0].batu == 0) && (BATS[index][1].batu == 0))
+			break;
+	}
+	if (index == nr_bats)
+		return -1;
+
+	bat = BATS[index];
 
 	if (((flags & _PAGE_NO_CACHE) == 0) &&
 	    cpu_has_feature(CPU_FTR_NEED_COHERENT))
@@ -162,6 +182,90 @@ void __init setbat(int index, unsigned long virt, phys_addr_t phys,
 	bat_addrs[index].start = virt;
 	bat_addrs[index].limit = virt + ((bl + 1) << 17) - 1;
 	bat_addrs[index].phys = phys;
+	return index;
+}
+
+/**
+ * ioremap_bat - Allow IO regions to be mapped using BAT registers
+ * @addr: physical address of region
+ * @size: size of region
+ *
+ * This routine uses setbat() to set up IO ranges before the MMU is
+ * fully configured.  Regions allocated with this function will
+ * automatically be converted into page table entries once the MMU is able
+ * to accept them.
+ *
+ * This routine can be called really early, before MMU_init() is called.  It
+ * is useful for setting up early debug output consoles and frequently
+ * accessed IO regions, like the internally memory mapped registers (IMMR)
+ * in an SoC.
+ *
+ * Just like in setbat, size must be a power of 2 between 128k and 256M.
+ * It is also assumed that callers are somewhat sane and will not be trying
+ * to call this multiple times on the same region.
+ */
+void __iomem * __init
+ioremap_bat(phys_addr_t addr, unsigned long size)
+{
+	struct ppc_bat *bat;
+	unsigned long v;
+	int i;
+
+	/* BAT mappings must be 128k aligned */
+	if (addr != _ALIGN_DOWN(addr, 128 << 10))
+		return NULL;
+
+	/* Carve out a chunk of the ioremap virtual address region
+	 * Also must be 128k aligned */
+	v = ioremap_bot = _ALIGN_DOWN(ioremap_bot - size, 128 << 10);
+
+	/* Allocate a BAT for this IO region */
+	i = setbat(v, addr, size, _PAGE_IO);
+	if (i < 0)
+		return NULL;
+	bat = BATS[i];
+
+	/*
+	 * IO BAT setting can be loaded immediately.
+	 * This only sets the DBATs.  IBATs are irrelevant for IO ranges
+	 *
+	 * Note: Don't disturb BAT 0; it is dedicated for mapping RAM,
+	 * especially in early boot.  Kernel will break if it gets changed
+	 * here.  (actually, setbat should never return index 0 for IO BAT
+	 * mappings).
+	 */
+	switch(i) {
+	case 1:
+		mtspr(SPRN_DBAT1U, bat[1].batu);
+		mtspr(SPRN_DBAT1L, bat[1].batl);
+		break;
+	case 2:
+		mtspr(SPRN_DBAT2U, bat[1].batu);
+		mtspr(SPRN_DBAT2L, bat[1].batl);
+		break;
+	case 3:
+		mtspr(SPRN_DBAT3U, bat[1].batu);
+		mtspr(SPRN_DBAT3L, bat[1].batl);
+		break;
+	case 4:
+		mtspr(SPRN_DBAT4U, bat[1].batu);
+		mtspr(SPRN_DBAT4L, bat[1].batl);
+		break;
+	case 5:
+		mtspr(SPRN_DBAT5U, bat[1].batu);
+		mtspr(SPRN_DBAT5L, bat[1].batl);
+		break;
+	case 6:
+		mtspr(SPRN_DBAT6U, bat[1].batu);
+		mtspr(SPRN_DBAT6L, bat[1].batl);
+		break;
+	case 7:
+		mtspr(SPRN_DBAT7U, bat[1].batu);
+		mtspr(SPRN_DBAT7L, bat[1].batl);
+		break;
+	}
+
+	return (void __iomem *)v;
 }
 
 /*
diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index 53da8a0..b3b4f8c 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -56,7 +56,7 @@ void __init udbg_init_cpm(void)
 {
 	if (cpm_udbg_txdesc) {
 #ifdef CONFIG_CPM2
-		setbat(1, 0xf0000000, 0xf0000000, 1024*1024, _PAGE_IO);
+		setbat(0xf0000000, 0xf0000000, 1024*1024, _PAGE_IO);
 #endif
 		udbg_putc = udbg_putc_cpm;
 	}
diff --git a/include/asm-powerpc/pgalloc-32.h b/include/asm-powerpc/pgalloc-32.h
index 58c0714..ea8b23d 100644
--- a/include/asm-powerpc/pgalloc-32.h
+++ b/include/asm-powerpc/pgalloc-32.h
@@ -40,4 +40,6 @@ extern void pte_free(struct mm_struct *mm, pgtable_t pte);
 
 #define check_pgt_cache()	do { } while (0)
 
+extern void __iomem *ioremap_bat(phys_addr_t addr, unsigned long size);
+
 #endif /* _ASM_POWERPC_PGALLOC_32_H */

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

On Aug 6, 2008, at 1:02 AM, Grant Likely wrote:

> From: Grant Likely <grant.likely@secretlab.ca>
>
> ioremap_bat() is useful for things like mapping SoC internally  
> memory mapped
> register and early text because it allows mappings to devices to be  
> setup
> early in the boot process where they are needed, and the mappings  
> persist
> after the MMU is configured.
>
> Without ioremap_bat(), setting up the MMU would cause the early text
> mappings to get lost and mostly likely result in a kernel panic on  
> the next
> attempt at output.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---

why can't we just do this in ioremap itself?

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

On Wed, Aug 6, 2008 at 8:07 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Aug 6, 2008, at 1:02 AM, Grant Likely wrote:
>
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> ioremap_bat() is useful for things like mapping SoC internally memory
>> mapped
>> register and early text because it allows mappings to devices to be setup
>> early in the boot process where they are needed, and the mappings persist
>> after the MMU is configured.
>>
>> Without ioremap_bat(), setting up the MMU would cause the early text
>> mappings to get lost and mostly likely result in a kernel panic on the
>> next
>> attempt at output.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> why can't we just do this in ioremap itself?

I suppose we could; but the usecase is somewhat different and I wanted
to keep it simple.  Using a separate API also helps reenforce that the
caller really needs to know what they are doing because BATs are a
limited resource.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

On Aug 6, 2008, at 4:54 PM, Grant Likely wrote:

> On Wed, Aug 6, 2008 at 8:07 AM, Kumar Gala  
> <galak@kernel.crashing.org> wrote:
>>
>> On Aug 6, 2008, at 1:02 AM, Grant Likely wrote:
>>
>>> From: Grant Likely <grant.likely@secretlab.ca>
>>>
>>> ioremap_bat() is useful for things like mapping SoC internally  
>>> memory
>>> mapped
>>> register and early text because it allows mappings to devices to  
>>> be setup
>>> early in the boot process where they are needed, and the mappings  
>>> persist
>>> after the MMU is configured.
>>>
>>> Without ioremap_bat(), setting up the MMU would cause the early text
>>> mappings to get lost and mostly likely result in a kernel panic on  
>>> the
>>> next
>>> attempt at output.
>>>
>>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>
>> why can't we just do this in ioremap itself?
>
> I suppose we could; but the usecase is somewhat different and I wanted
> to keep it simple.  Using a separate API also helps reenforce that the
> caller really needs to know what they are doing because BATs are a
> limited resource.

there is a bunch of error checking and difference in semantics that  
you need to fix.  I think introduce a new API for this is silly,  
especially since we expect there to only be one actual invocation of  
the API for serial console access.

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Benjamin Herrenschmidt]

> there is a bunch of error checking and difference in semantics that  
> you need to fix.  I think introduce a new API for this is silly,  
> especially since we expect there to only be one actual invocation of  
> the API for serial console access.

Not necessarily....

There's another aspect to BAT mappings here. First, they should be
permanent (ie, not unmappable). That way, we have ioremap just use
an existing BAT mapping when asked for a device that is covered
by a BAT. This allows to have platform code do something like setup
a BAT over a bunch of SOC registers or over a device, to automagically
get drivers doing ioremap to that area benefit from it.

Ben.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Brad Boyer]

On Thu, Aug 07, 2008 at 08:28:29AM +1000, Benjamin Herrenschmidt wrote:
> 
> > there is a bunch of error checking and difference in semantics that  
> > you need to fix.  I think introduce a new API for this is silly,  
> > especially since we expect there to only be one actual invocation of  
> > the API for serial console access.
> 
> Not necessarily....
> 
> There's another aspect to BAT mappings here. First, they should be
> permanent (ie, not unmappable). That way, we have ioremap just use
> an existing BAT mapping when asked for a device that is covered
> by a BAT. This allows to have platform code do something like setup
> a BAT over a bunch of SOC registers or over a device, to automagically
> get drivers doing ioremap to that area benefit from it.

The m68k arch code does something similar with TT registers and ioremap,
but it's a little hackish currently. If there was a more generic way
to handle it, that would make things a little cleaner. In the m68k
implementation, there are just exceptions in the ioremap code that are
hard wired to know about the ranges that are normally set earlier. Since
the ranges end up being in multiple files, it's more fragile to change
what is mapped in these blocks.

	Brad Boyer
	flar@allandria.com

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

On Wed, Aug 6, 2008 at 4:28 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
>> there is a bunch of error checking and difference in semantics that
>> you need to fix.  I think introduce a new API for this is silly,
>> especially since we expect there to only be one actual invocation of
>> the API for serial console access.
>
> Not necessarily....
>
> There's another aspect to BAT mappings here. First, they should be
> permanent (ie, not unmappable). That way, we have ioremap just use
> an existing BAT mapping when asked for a device that is covered
> by a BAT. This allows to have platform code do something like setup
> a BAT over a bunch of SOC registers or over a device, to automagically
> get drivers doing ioremap to that area benefit from it.

Actually, that is exactly what I am in the process of doing right now
for all the 5200 platforms.  It is a performance win with no apparent
downside.

Next I want to investigate if it makes sense to do it for PCI IO regions.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

On Aug 6, 2008, at 5:28 PM, Benjamin Herrenschmidt wrote:

>
>> there is a bunch of error checking and difference in semantics that
>> you need to fix.  I think introduce a new API for this is silly,
>> especially since we expect there to only be one actual invocation of
>> the API for serial console access.
>
> Not necessarily....
>
> There's another aspect to BAT mappings here. First, they should be
> permanent (ie, not unmappable). That way, we have ioremap just use
> an existing BAT mapping when asked for a device that is covered
> by a BAT. This allows to have platform code do something like setup
> a BAT over a bunch of SOC registers or over a device, to automagically
> get drivers doing ioremap to that area benefit from it.

why should they be permanent.. We could implement reference counting  
around the regions and free BATs if the count = 0.

I'm more concerned about this being implemented around the existing  
ioremap core in __ioremap().  We can easily use a flag bit to say use  
"large mappings" or the fact that mem_init_done == 0.

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Benjamin Herrenschmidt]

On Wed, 2008-08-06 at 20:49 -0500, Kumar Gala wrote:
> On Aug 6, 2008, at 5:28 PM, Benjamin Herrenschmidt wrote:
> 
> >
> >> there is a bunch of error checking and difference in semantics that
> >> you need to fix.  I think introduce a new API for this is silly,
> >> especially since we expect there to only be one actual invocation of
> >> the API for serial console access.
> >
> > Not necessarily....
> >
> > There's another aspect to BAT mappings here. First, they should be
> > permanent (ie, not unmappable). That way, we have ioremap just use
> > an existing BAT mapping when asked for a device that is covered
> > by a BAT. This allows to have platform code do something like setup
> > a BAT over a bunch of SOC registers or over a device, to automagically
> > get drivers doing ioremap to that area benefit from it.
> 
> why should they be permanent.. We could implement reference counting  
> around the regions and free BATs if the count = 0.

Do we care ?

> I'm more concerned about this being implemented around the existing  
> ioremap core in __ioremap().  We can easily use a flag bit to say use  
> "large mappings" or the fact that mem_init_done == 0.

mem_init_done isn't a good indication. We can do page tables when it's
0, we would have to use a separate mem_preinit_done or something :-)

I initially also though about a flag to ioremap_prot to be honest. But
it does obfuscate the normal ioremap code path and if there's a flag,
that means that callers know the difference and thus may as well call
a separate function, don't you think ?

Ben.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

On Aug 7, 2008, at 5:13 PM, Benjamin Herrenschmidt wrote:

> On Wed, 2008-08-06 at 20:49 -0500, Kumar Gala wrote:
>> On Aug 6, 2008, at 5:28 PM, Benjamin Herrenschmidt wrote:
>>
>>>
>>>> there is a bunch of error checking and difference in semantics that
>>>> you need to fix.  I think introduce a new API for this is silly,
>>>> especially since we expect there to only be one actual invocation  
>>>> of
>>>> the API for serial console access.
>>>
>>> Not necessarily....
>>>
>>> There's another aspect to BAT mappings here. First, they should be
>>> permanent (ie, not unmappable). That way, we have ioremap just use
>>> an existing BAT mapping when asked for a device that is covered
>>> by a BAT. This allows to have platform code do something like setup
>>> a BAT over a bunch of SOC registers or over a device, to  
>>> automagically
>>> get drivers doing ioremap to that area benefit from it.
>>
>> why should they be permanent.. We could implement reference counting
>> around the regions and free BATs if the count = 0.
>
> Do we care ?

probably not for BATs but for other things we might.

>> I'm more concerned about this being implemented around the existing
>> ioremap core in __ioremap().  We can easily use a flag bit to say use
>> "large mappings" or the fact that mem_init_done == 0.
>
> mem_init_done isn't a good indication. We can do page tables when it's
> 0, we would have to use a separate mem_preinit_done or something :-)
>
> I initially also though about a flag to ioremap_prot to be honest. But
> it does obfuscate the normal ioremap code path and if there's a flag,
> that means that callers know the difference and thus may as well call
> a separate function, don't you think ?

I'm ok with exposing a separate function as far as the API goes.. I'm  
not ok with duplicating the logic of __ioremap().

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

On Thu, Aug 07, 2008 at 07:04:04PM -0500, Kumar Gala wrote:
>> mem_init_done isn't a good indication. We can do page tables when it's
>> 0, we would have to use a separate mem_preinit_done or something :-)
>>
>> I initially also though about a flag to ioremap_prot to be honest. But
>> it does obfuscate the normal ioremap code path and if there's a flag,
>> that means that callers know the difference and thus may as well call
>> a separate function, don't you think ?
>
> I'm ok with exposing a separate function as far as the API goes.. I'm  
> not ok with duplicating the logic of __ioremap().

Turns out there is very little actual duplication of code with
__ioremap().  The checks for p_mapped_by_* are the same, but all the
alignment checks are different because different boundaries are used.

I attempted to break things down to a common function, but there is not
a lot there.  But I will add a function to manage modification of
ioremap_bot.

g.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Scott Wood]

Kumar Gala wrote:
> On Aug 6, 2008, at 4:54 PM, Grant Likely wrote:
>> I suppose we could; but the usecase is somewhat different and I
>> wanted to keep it simple.  Using a separate API also helps
>> reenforce that the caller really needs to know what they are doing
>> because BATs are a limited resource.
> 
> there is a bunch of error checking and difference in semantics that
> you need to fix.  I think introduce a new API for this is silly,

Why is it silly?  It seems silly to me to complicate ioremap() with 
knowing when to use BATs and when not to use them.

> especially since we expect there to only be one actual invocation of
> the API for serial console access.

I could also see a BAT (or equivalent) being used for IMMR-space, or
other frequently accessed registers, to save TLB entries.

-Scott

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

On Wed, Aug 6, 2008 at 4:11 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Aug 6, 2008, at 4:54 PM, Grant Likely wrote:
>
>> On Wed, Aug 6, 2008 at 8:07 AM, Kumar Gala <galak@kernel.crashing.org>
>> wrote:
>>>
>>> On Aug 6, 2008, at 1:02 AM, Grant Likely wrote:
>>>
>>>> From: Grant Likely <grant.likely@secretlab.ca>
>>>>
>>>> ioremap_bat() is useful for things like mapping SoC internally memory
>>>> mapped
>>>> register and early text because it allows mappings to devices to be
>>>> setup
>>>> early in the boot process where they are needed, and the mappings
>>>> persist
>>>> after the MMU is configured.
>>>>
>>>> Without ioremap_bat(), setting up the MMU would cause the early text
>>>> mappings to get lost and mostly likely result in a kernel panic on the
>>>> next
>>>> attempt at output.
>>>>
>>>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>>>> ---
>>>
>>> why can't we just do this in ioremap itself?
>>
>> I suppose we could; but the usecase is somewhat different and I wanted
>> to keep it simple.  Using a separate API also helps reenforce that the
>> caller really needs to know what they are doing because BATs are a
>> limited resource.
>
> there is a bunch of error checking and difference in semantics that you need
> to fix.

details please?  I agree that it can be tightened up in some areas,
but I'd like to know if you've seen something I haven't.

>  I think introduce a new API for this is silly,

I don't think so.  I actually prefer it being an different API.  It
forces the caller to understand what the function does.  It forces the
programmer to think and to understand the effect of mapping large io
blocks.

> especially since we
> expect there to only be one actual invocation of the API for serial console
> access.

Actually, this is not only for early serial.  Early serial just
happens to be the first user.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

>>>> why can't we just do this in ioremap itself?
>>>
>>> I suppose we could; but the usecase is somewhat different and I  
>>> wanted
>>> to keep it simple.  Using a separate API also helps reenforce that  
>>> the
>>> caller really needs to know what they are doing because BATs are a
>>> limited resource.
>>
>> there is a bunch of error checking and difference in semantics that  
>> you need
>> to fix.
>
> details please?  I agree that it can be tightened up in some areas,
> but I'd like to know if you've seen something I haven't.

The alignment checks for one?  Why does the caller need to ensure  
alignment.. we just force alignment in __ioremap().. we should be  
doing the same thing here.

>> I think introduce a new API for this is silly,
>
> I don't think so.  I actually prefer it being an different API.  It
> forces the caller to understand what the function does.  It forces the
> programmer to think and to understand the effect of mapping large io
> blocks.

Ok.. My feeling is the same core logic should be used so I don't have  
to fix 2 places in the future.

>> especially since we
>> expect there to only be one actual invocation of the API for serial  
>> console
>> access.
>
> Actually, this is not only for early serial.  Early serial just
> happens to be the first user.

Than this really should be done as part of __ioremap() since you have  
to deal with doing different things depending on how mem_init_done is  
set.

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Benjamin Herrenschmidt]

On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> ioremap_bat() is useful for things like mapping SoC internally memory mapped
> register and early text because it allows mappings to devices to be setup
> early in the boot process where they are needed, and the mappings persist
> after the MMU is configured.
> 
> Without ioremap_bat(), setting up the MMU would cause the early text
> mappings to get lost and mostly likely result in a kernel panic on the next
> attempt at output.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---

First comment, make the name more generic. This facility could be
implemented on processors without BATs using different techniques.

Maybe ioremap_block() or ioremap_early() or something like that.

> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 066e65c..7b25b57 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -113,6 +113,15 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
>   */
>  notrace void __init machine_init(unsigned long dt_ptr, unsigned long phys)
>  {
> +	/* Do the bare minimum to start allocting from the IO region so
> +	 * that ioremap_bat() works */
> +#ifdef CONFIG_HIGHMEM
> +	ioremap_base = PKMAP_BASE;
> +#else
> +	ioremap_base = 0xfe000000UL;	/* for now, could be 0xfffff000 */
> +#endif /* CONFIG_HIGHMEM */
> +	ioremap_bot = ioremap_base;
> +

Can't these be statically initialized ? If not, then do a function call
to mm/ please. Something like mm_init_early().

> +	/* BAT mappings must be 128k aligned */
> +	if (addr != _ALIGN_DOWN(addr, 128 << 10))
> +		return NULL;

Mustn't they be naturally aligned on their size ?

Cheers,
Ben.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Grant Likely]

On Wed, Aug 6, 2008 at 4:26 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> ioremap_bat() is useful for things like mapping SoC internally memory mapped
>> register and early text because it allows mappings to devices to be setup
>> early in the boot process where they are needed, and the mappings persist
>> after the MMU is configured.
>>
>> Without ioremap_bat(), setting up the MMU would cause the early text
>> mappings to get lost and mostly likely result in a kernel panic on the next
>> attempt at output.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>
> First comment, make the name more generic. This facility could be
> implemented on processors without BATs using different techniques.
>
> Maybe ioremap_block() or ioremap_early() or something like that.

okay

>> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
>> index 066e65c..7b25b57 100644
>> --- a/arch/powerpc/kernel/setup_32.c
>> +++ b/arch/powerpc/kernel/setup_32.c
>> @@ -113,6 +113,15 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
>>   */
>>  notrace void __init machine_init(unsigned long dt_ptr, unsigned long phys)
>>  {
>> +     /* Do the bare minimum to start allocting from the IO region so
>> +      * that ioremap_bat() works */
>> +#ifdef CONFIG_HIGHMEM
>> +     ioremap_base = PKMAP_BASE;
>> +#else
>> +     ioremap_base = 0xfe000000UL;    /* for now, could be 0xfffff000 */
>> +#endif /* CONFIG_HIGHMEM */
>> +     ioremap_bot = ioremap_base;
>> +
>
> Can't these be statically initialized ? If not, then do a function call
> to mm/ please. Something like mm_init_early().

heh, I had just moved this code block without thinking about it.  I'll
investigate and see what I can do.

>> +     /* BAT mappings must be 128k aligned */
>> +     if (addr != _ALIGN_DOWN(addr, 128 << 10))
>> +             return NULL;
>
> Mustn't they be naturally aligned on their size ?

I'm not sure what you're asking.  Are you asking about this particular
test, or are you asking why I don't also test the size?

I do this particular test to make absolute sure that the caller
absolutely understands the limitations of the block mapping.  If they
call this with something that isn't 128k aligned, then I make it fail
immediately so the coder is forced to go and understand what they are
allowed to do.  Basically, I'm reinforcing the fact that this is not
the same as ioremap().

I haven't decided yet if I should test size in the same way.  Thoughts?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Scott Wood]

On Wed, Aug 06, 2008 at 05:11:08PM -0600, Grant Likely wrote:
> I do this particular test to make absolute sure that the caller
> absolutely understands the limitations of the block mapping.  If they
> call this with something that isn't 128k aligned, then I make it fail
> immediately so the coder is forced to go and understand what they are
> allowed to do.  Basically, I'm reinforcing the fact that this is not
> the same as ioremap().
> 
> I haven't decided yet if I should test size in the same way.  Thoughts?

I'd prefer it round up the size as needed to cover the requested mapping
and needed alignment.

The minimum size is going to be different on Book E, for example, and I
can think of at least one user that will be shared between Book E and
classic (CPM2 early console).

-Scott

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Kumar Gala]

On Aug 7, 2008, at 11:45 AM, Scott Wood wrote:

> On Wed, Aug 06, 2008 at 05:11:08PM -0600, Grant Likely wrote:
>> I do this particular test to make absolute sure that the caller
>> absolutely understands the limitations of the block mapping.  If they
>> call this with something that isn't 128k aligned, then I make it fail
>> immediately so the coder is forced to go and understand what they are
>> allowed to do.  Basically, I'm reinforcing the fact that this is not
>> the same as ioremap().
>>
>> I haven't decided yet if I should test size in the same way.   
>> Thoughts?
>
> I'd prefer it round up the size as needed to cover the requested  
> mapping
> and needed alignment.
>
> The minimum size is going to be different on Book E, for example,  
> and I
> can think of at least one user that will be shared between Book E and
> classic (CPM2 early console).

Which is how ioremap works today.  We ask for a size of 3 bytes and it  
rounds up to a 4k page.  The same should be true of the new interface  
(and round up to whatever the smallest size the HW we are using can  
handle).

- k

Re: [RFC/PATCH 1/3] powerpc: add ioremap_bat() function for setting up BAT translated IO regions.

[Benjamin Herrenschmidt]

On Wed, 2008-08-06 at 17:11 -0600, Grant Likely wrote:
> I'm not sure what you're asking.  Are you asking about this particular
> test, or are you asking why I don't also test the size?

Badly worded. 

I meant BAT sizes are masks of bits. IE, they are power of 2 and the
BAT address must be aligned to that power of 2 (ie, the BAT matching
uses the size as a bit mask of relevant bits to compare).

Unless I misread, your code doesn't provide the necessary tests/rounding
of size and alignment of the virtual address.. does it ?
 
> I do this particular test to make absolute sure that the caller
> absolutely understands the limitations of the block mapping.  If they
> call this with something that isn't 128k aligned, then I make it fail
> immediately so the coder is forced to go and understand what they are
> allowed to do.  Basically, I'm reinforcing the fact that this is not
> the same as ioremap().
> 
> I haven't decided yet if I should test size in the same way.
> Thoughts?

Ben.

[RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

of_lookup_stdout() is useful for figuring out what device to use as output
for early boot progress messages.  It returns the node pointed to by the
linux,stdout-path property in the chosen node.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/of/base.c  |   24 ++++++++++++++++++++++++
 include/linux/of.h |    1 +
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ad8ac1a..10c6a46 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -473,3 +473,27 @@ int of_modalias_node(struct device_node *node, char *modalias, int len)
 }
 EXPORT_SYMBOL_GPL(of_modalias_node);
 
+/**
+ * of_lookup_stdout - find node pointed to by chosen linux,stdout-path property
+ *
+ * This routine returns a pointer to the stdout node or NULL on failure
+ */
+struct device_node __init *of_lookup_stdout(void)
+{
+	struct device_node *chosen, *stdout_node;
+	const char *stdout_path;
+
+	chosen = of_find_node_by_path("/chosen");
+	if (!chosen)
+		return NULL;
+
+	stdout_path = of_get_property(chosen, "linux,stdout-path", NULL);
+	if (!stdout_path) {
+		of_node_put(chosen);
+		return NULL;
+	}
+
+	stdout_node = of_find_node_by_path(stdout_path);
+	of_node_put(chosen);
+	return stdout_node;
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index 79886ad..e8b215b 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -71,5 +71,6 @@ extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+struct device_node __init *of_lookup_stdout(void);
 
 #endif /* _LINUX_OF_H */

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> of_lookup_stdout() is useful for figuring out what device to use as output
> for early boot progress messages.  It returns the node pointed to by the
> linux,stdout-path property in the chosen node.

Nice. You don't feel like converting all the places that currently do it
by hand to use this do you :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Grant Likely]

On Wed, Aug 6, 2008 at 12:14 AM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> of_lookup_stdout() is useful for figuring out what device to use as output
>> for early boot progress messages.  It returns the node pointed to by the
>> linux,stdout-path property in the chosen node.
>
> Nice. You don't feel like converting all the places that currently do it
> by hand to use this do you :)

Yep, I'll do this.  :-)

g.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

Hi Grant,

On Wed, 6 Aug 2008 00:34:04 -0600 "Grant Likely" <grant.likely@secretlab.ca> wrote:
>
> On Wed, Aug 6, 2008 at 12:14 AM, Michael Ellerman
> <michael@ellerman.id.au> wrote:
> > On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
> >> From: Grant Likely <grant.likely@secretlab.ca>
> >>
> >> of_lookup_stdout() is useful for figuring out what device to use as output
> >> for early boot progress messages.  It returns the node pointed to by the
> >> linux,stdout-path property in the chosen node.
> >
> > Nice. You don't feel like converting all the places that currently do it
> > by hand to use this do you :)
> 
> Yep, I'll do this.  :-)

Could you also send an email to Dave Miller to see if he has any objections
to this function being generic (since the Sparc guys share this code).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 6 Aug 2008 17:42:33 +1000

> Hi Grant,
> 
> On Wed, 6 Aug 2008 00:34:04 -0600 "Grant Likely" <grant.likely@secretlab.ca> wrote:
> >
> > On Wed, Aug 6, 2008 at 12:14 AM, Michael Ellerman
> > <michael@ellerman.id.au> wrote:
> > > On Wed, 2008-08-06 at 00:02 -0600, Grant Likely wrote:
> > >> From: Grant Likely <grant.likely@secretlab.ca>
> > >>
> > >> of_lookup_stdout() is useful for figuring out what device to use as output
> > >> for early boot progress messages.  It returns the node pointed to by the
> > >> linux,stdout-path property in the chosen node.
> > >
> > > Nice. You don't feel like converting all the places that currently do it
> > > by hand to use this do you :)
> > 
> > Yep, I'll do this.  :-)
> 
> Could you also send an email to Dave Miller to see if he has any objections
> to this function being generic (since the Sparc guys share this code).

I already voiced my concerns.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On Wed, 6 Aug 2008 17:42:33 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Could you also send an email to Dave Miller to see if he has any objections
> to this function being generic (since the Sparc guys share this code).

So I should read ahead :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Miller]

From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 06 Aug 2008 00:02:44 -0600

> of_lookup_stdout() is useful for figuring out what device to use as output
> for early boot progress messages.  It returns the node pointed to by the
> linux,stdout-path property in the chosen node.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

On sparc platforms this is obtained differently.  We obtain the 32-bit
instance value of "/chosen/stdout" and convert that into a prom device
node path using "instance-to-path".

If this of_lookup_stdout() is going into generic OF code, it should be
done in a way that works on all platforms.  All of these "linux,*"
properties and node names are powerpc platform specific.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Grant Likely]

On Wed, Aug 6, 2008 at 12:32 AM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Wed, 06 Aug 2008 00:02:44 -0600
>
>> of_lookup_stdout() is useful for figuring out what device to use as output
>> for early boot progress messages.  It returns the node pointed to by the
>> linux,stdout-path property in the chosen node.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>
> On sparc platforms this is obtained differently.  We obtain the 32-bit
> instance value of "/chosen/stdout" and convert that into a prom device
> node path using "instance-to-path".

How about if I modify it to try both methods?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Miller]

From: "Grant Likely" <grant.likely@secretlab.ca>
Date: Wed, 6 Aug 2008 00:35:24 -0600

> On Wed, Aug 6, 2008 at 12:32 AM, David Miller <davem@davemloft.net> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> > Date: Wed, 06 Aug 2008 00:02:44 -0600
> >
> >> of_lookup_stdout() is useful for figuring out what device to use as output
> >> for early boot progress messages.  It returns the node pointed to by the
> >> linux,stdout-path property in the chosen node.
> >>
> >> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > On sparc platforms this is obtained differently.  We obtain the 32-bit
> > instance value of "/chosen/stdout" and convert that into a prom device
> > node path using "instance-to-path".
> 
> How about if I modify it to try both methods?

Sure, that would be nice.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Benjamin Herrenschmidt]

On Wed, 2008-08-06 at 00:35 -0600, Grant Likely wrote:
> On Wed, Aug 6, 2008 at 12:32 AM, David Miller <davem@davemloft.net> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> > Date: Wed, 06 Aug 2008 00:02:44 -0600
> >
> >> of_lookup_stdout() is useful for figuring out what device to use as output
> >> for early boot progress messages.  It returns the node pointed to by the
> >> linux,stdout-path property in the chosen node.
> >>
> >> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > On sparc platforms this is obtained differently.  We obtain the 32-bit
> > instance value of "/chosen/stdout" and convert that into a prom device
> > node path using "instance-to-path".
> 
> How about if I modify it to try both methods?

The sparc method however cannot be used on powerpc because it requires
a call to OF to do the "live" conversion (ie, instance-to-path). In
fact, our /chosen/linux,stdout-path is just a cached result of that
conversion done during boot.

Ben.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Paul Mackerras]

David Miller writes:

> On sparc platforms this is obtained differently.  We obtain the 32-bit
> instance value of "/chosen/stdout" and convert that into a prom device
> node path using "instance-to-path".

That's actually exactly what we do too, the linux,stdout-path property
is just a cache of the result of that process.  The difference is that
we have to do it early on while we still have OF around, while you can
do it later.

Paul.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Wed, 6 Aug 2008 20:21:04 +1000

> David Miller writes:
> 
> > On sparc platforms this is obtained differently.  We obtain the 32-bit
> > instance value of "/chosen/stdout" and convert that into a prom device
> > node path using "instance-to-path".
> 
> That's actually exactly what we do too, the linux,stdout-path property
> is just a cache of the result of that process.  The difference is that
> we have to do it early on while we still have OF around, while you can
> do it later.

I do it right when I build the in-kernel OBP tree.  Then I cache
these results in:

extern struct device_node *of_console_device;
extern char *of_console_path;
extern char *of_console_options;

which are declared in asm/prom.h

I could compute it even earlier and just store a phandle instead
of a device_node pointer.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Grant Likely]

On Wed, Aug 6, 2008 at 4:21 AM, Paul Mackerras <paulus@samba.org> wrote:
> David Miller writes:
>
>> On sparc platforms this is obtained differently.  We obtain the 32-bit
>> instance value of "/chosen/stdout" and convert that into a prom device
>> node path using "instance-to-path".
>
> That's actually exactly what we do too, the linux,stdout-path property
> is just a cache of the result of that process.  The difference is that
> we have to do it early on while we still have OF around, while you can
> do it later.

It's not what we do with flattened device trees blobs though.  In the
flattened tree we're not using a /chosen/stdout property, just the
linux,stdout-path one.

The question that remains is; should there be?  Should the dt blobs
use /chosen/stdout also?  (I'm not familiar enough with real OF to
know the answer.  I'm assuming that an instance value is not the same
as a phandle).

g.

>
> Paul.
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Segher Boessenkool]

> It's not what we do with flattened device trees blobs though.  In the
> flattened tree we're not using a /chosen/stdout property, just the
> linux,stdout-path one.
>
> The question that remains is; should there be?  Should the dt blobs
> use /chosen/stdout also?  (I'm not familiar enough with real OF to
> know the answer.  I'm assuming that an instance value is not the same
> as a phandle).

ihandles and phandles are not the same thing in OF.  Since in the
"flat world" we cannot have instances, we should use phandles instead
of ihandles for the things in /chosen.  I thought we agreed on that
already, perhaps I am wrong?


Segher

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Mitch Bradley]

Segher Boessenkool wrote:
>> It's not what we do with flattened device trees blobs though.  In the
>> flattened tree we're not using a /chosen/stdout property, just the
>> linux,stdout-path one.
>>
>> The question that remains is; should there be?  Should the dt blobs
>> use /chosen/stdout also?  (I'm not familiar enough with real OF to
>> know the answer.  I'm assuming that an instance value is not the same
>> as a phandle).

The difference between a phandle and an ihandle is similar to the 
difference between (the inode of) an executable files on disk and (the 
process id of) a running process.  A phandle refers to the static 
information that describes a device, while an ihandle refers to a 
particular (out of potentially several) active instantiation of the OFW 
driver for that devices.  An "instance value" is a data item that can 
have a different value for each of the running instances of a given 
driver.  In the analogy, an instance value is like a data segment variable.

Given an ihandle, you can get the corresponding phandle with 
ihandle>phandle.  You can't go from phandle to ihandle, because that 
direction is one-to-many.

Why you you ever need more than one running instance of a given driver?  
For leaf devices, it is pretty rare to have multiple instances.  
Multiple instances are more often used for intermediate nodes.  The same 
intermediate node - for example a usb node - is often the parent of 
several leaf nodes that are active simultaneously.  When you open a 
device, its device tree parents up to the root are implicitly opened.  
Each such instance can hold dynamic state on behalf of its children.

>
> ihandles and phandles are not the same thing in OF.  Since in the
> "flat world" we cannot have instances, we should use phandles instead
> of ihandles for the things in /chosen.  I thought we agreed on that
> already, perhaps I am wrong?
>
>
> Segher
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@ozlabs.org
> https://ozlabs.org/mailman/listinfo/devicetree-discuss
>

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Gibson]

On Wed, Aug 06, 2008 at 06:25:48PM +0200, Segher Boessenkool wrote:
>> It's not what we do with flattened device trees blobs though.  In the
>> flattened tree we're not using a /chosen/stdout property, just the
>> linux,stdout-path one.
>>
>> The question that remains is; should there be?  Should the dt blobs
>> use /chosen/stdout also?  (I'm not familiar enough with real OF to
>> know the answer.  I'm assuming that an instance value is not the same
>> as a phandle).
>
> ihandles and phandles are not the same thing in OF.  Since in the
> "flat world" we cannot have instances, we should use phandles instead
> of ihandles for the things in /chosen.  I thought we agreed on that
> already, perhaps I am wrong?

Not that I recall.  In general we've been avoiding using anything that
traditionally contains an ihandle; hence the direct use of
linux,stdout-path when using a flat tree.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Benjamin Herrenschmidt]

> It's not what we do with flattened device trees blobs though.  In the
> flattened tree we're not using a /chosen/stdout property, just the
> linux,stdout-path one.
> 
> The question that remains is; should there be?  Should the dt blobs
> use /chosen/stdout also?  (I'm not familiar enough with real OF to
> know the answer.  I'm assuming that an instance value is not the same
> as a phandle).

Yup, there are two issues there:

 - The instance value would have to be converted to a phandle while
OF is still alive. I initially did that and added a stdout-node or so
property, but that still hit the next issue.

 - IBM machines has this weird distinction between the real phandle
and the ibm,phandle, the later being the same except for things
that get hotplugged ... and some of the vdevices. We got really
confused trying to sort that out with the output device.

So in the end, I decided to just convert the ihandle to a path and
stick that path in the device-tree.

Cheers,
Ben.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Timur Tabi]

On Wed, Aug 6, 2008 at 1:02 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> of_lookup_stdout() is useful for figuring out what device to use as output
> for early boot progress messages.  It returns the node pointed to by the
> linux,stdout-path property in the chosen node.

I thought linux,stdout-path is deprecated are we're supposed to be
using the aliases node instead?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[David Gibson]

On Wed, Aug 06, 2008 at 11:46:47AM -0500, Timur Tabi wrote:
> On Wed, Aug 6, 2008 at 1:02 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > of_lookup_stdout() is useful for figuring out what device to use as output
> > for early boot progress messages.  It returns the node pointed to by the
> > linux,stdout-path property in the chosen node.
> 
> I thought linux,stdout-path is deprecated are we're supposed to be
> using the aliases node instead?

During the ePAPR process this idea came up - standardising a 'stdout'
alias that would replace linux,stdout-path in chosen.  However that
was done in ignorance of the history of the linux,stdout-path property
and its connection to the stdout ihandle in chosen.  In any case, the
proposed 'stdout' alias didn't make the final cut for ePAPR, so how to
address this for flat-tree systems is still an open question.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

RE: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Yoder Stuart]

=20

> -----Original Message-----
> From: linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org=20
> [mailto:linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org]=20
> On Behalf Of David Gibson
> Sent: Thursday, August 07, 2008 1:13 AM
> To: Tabi Timur
> Cc: linuxppc-dev@ozlabs.org; paulus@samba.org; miltonm@bga.com
> Subject: Re: [RFC/PATCH 2/3] of: add of_lookup_stdout()=20
> utility function
>=20
> On Wed, Aug 06, 2008 at 11:46:47AM -0500, Timur Tabi wrote:
> > On Wed, Aug 6, 2008 at 1:02 AM, Grant Likely=20
> <grant.likely@secretlab.ca> wrote:
> > > From: Grant Likely <grant.likely@secretlab.ca>
> > >
> > > of_lookup_stdout() is useful for figuring out what device=20
> to use as output
> > > for early boot progress messages.  It returns the node=20
> pointed to by the
> > > linux,stdout-path property in the chosen node.
> >=20
> > I thought linux,stdout-path is deprecated are we're supposed to be
> > using the aliases node instead?
>=20
> During the ePAPR process this idea came up - standardising a 'stdout'
> alias that would replace linux,stdout-path in chosen.  However that
> was done in ignorance of the history of the linux,stdout-path property
> and its connection to the stdout ihandle in chosen.  In any case, the
> proposed 'stdout' alias didn't make the final cut for ePAPR, so how to
> address this for flat-tree systems is still an open question.

In the ePAPR discussion I think we generally agreed that
an alias was better than a property under /chosen.  There
were 2 things that caused us to delete the /aliases/stdout.

One was discussions around whether we needed to support
multiple consoles somehow.

The second was the idea that we may need a /aliases/stdin as well.
You could conceptually have stdout be a monitor and a stdin
be a keyboard.

Stuart

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Timur Tabi]

Yoder Stuart wrote:

> The second was the idea that we may need a /aliases/stdin as well.
> You could conceptually have stdout be a monitor and a stdin
> be a keyboard.

I don't think the hypervisor console subsystem supports this.  I don't see any
way of registering separate clients for input vs. output.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Grant Likely]

On Thu, Aug 07, 2008 at 04:12:54PM +1000, David Gibson wrote:
> On Wed, Aug 06, 2008 at 11:46:47AM -0500, Timur Tabi wrote:
> > On Wed, Aug 6, 2008 at 1:02 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > From: Grant Likely <grant.likely@secretlab.ca>
> > >
> > > of_lookup_stdout() is useful for figuring out what device to use as output
> > > for early boot progress messages.  It returns the node pointed to by the
> > > linux,stdout-path property in the chosen node.
> > 
> > I thought linux,stdout-path is deprecated are we're supposed to be
> > using the aliases node instead?
> 
> During the ePAPR process this idea came up - standardising a 'stdout'
> alias that would replace linux,stdout-path in chosen.  However that
> was done in ignorance of the history of the linux,stdout-path property
> and its connection to the stdout ihandle in chosen.  In any case, the
> proposed 'stdout' alias didn't make the final cut for ePAPR, so how to
> address this for flat-tree systems is still an open question.

So, seeing as settling on a way to determine stdout still up in the air,
it probably makes sense to condense that code down to a single
authoritative function so that changes in this area are contained in one
place.  For now, I'll stick with decoding linux,stdout-path and on Sparc
decoding the ihandle with the expectation that there will be further
refinements to be made.

g.

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Timur Tabi]

Grant Likely wrote:

> So, seeing as settling on a way to determine stdout still up in the air,
> it probably makes sense to condense that code down to a single
> authoritative function so that changes in this area are contained in one
> place.  For now, I'll stick with decoding linux,stdout-path and on Sparc
> decoding the ihandle with the expectation that there will be further
> refinements to be made.

Ack.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Benjamin Herrenschmidt]

On Wed, 2008-08-06 at 11:46 -0500, Timur Tabi wrote:
> On Wed, Aug 6, 2008 at 1:02 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > of_lookup_stdout() is useful for figuring out what device to use as output
> > for early boot progress messages.  It returns the node pointed to by the
> > linux,stdout-path property in the chosen node.
> 
> I thought linux,stdout-path is deprecated are we're supposed to be
> using the aliases node instead?

You are mixing two completely different things afaik.

Ben.

[RFC/PATCH 3/3] powerpc/52xx: add udbg and early debug support for PSC serial console

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/Kconfig.debug                   |   11 +++
 arch/powerpc/kernel/udbg.c                   |    2 
 arch/powerpc/platforms/52xx/lite5200.c       |    6 +
 arch/powerpc/platforms/52xx/mpc5200_simple.c |    3 +
 arch/powerpc/platforms/52xx/mpc52xx_common.c |  109 ++++++++++++++++++++++++++
 include/asm-powerpc/mpc52xx.h                |    1 
 include/asm-powerpc/udbg.h                   |    1 
 7 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 8c8aadb..26e12d6 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -210,6 +210,12 @@ config PPC_EARLY_DEBUG_40x
 	  inbuilt serial port. This works on chips with a 16550 compatible
 	  UART. Xilinx chips with uartlite cannot use this option.
 
+config PPC_EARLY_DEBUG_5200
+	bool "MPC5200 PSC serial port"
+	depends on PPC_MPC52xx
+	help
+	  Select this to enable early debugging for the Freescale MPC5200 SoC.
+
 config PPC_EARLY_DEBUG_CPM
 	bool "Early serial debugging for Freescale CPM-based serial ports"
 	depends on SERIAL_CPM
@@ -239,6 +245,11 @@ config PPC_EARLY_DEBUG_40x_PHYSADDR
 	depends on PPC_EARLY_DEBUG_40x
 	default "0xef600300"
 
+config PPC_EARLY_DEBUG_5200_PHYSADDR
+	hex "Early debug PSC UART physical address"
+	depends on PPC_EARLY_DEBUG_5200
+	default "0xf0002000"
+
 config PPC_EARLY_DEBUG_CPM_ADDR
 	hex "CPM UART early debug transmit descriptor address"
 	depends on PPC_EARLY_DEBUG_CPM
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index 7d6c9bb..a3a0c13 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -57,6 +57,8 @@ void __init udbg_early_init(void)
 #elif defined(CONFIG_PPC_EARLY_DEBUG_40x)
 	/* PPC40x debug */
 	udbg_init_40x_realmode();
+#elif defined(CONFIG_PPC_EARLY_DEBUG_5200)
+	udbg_init_mpc5200();
 #elif defined(CONFIG_PPC_EARLY_DEBUG_CPM)
 	udbg_init_cpm();
 #endif
diff --git a/arch/powerpc/platforms/52xx/lite5200.c b/arch/powerpc/platforms/52xx/lite5200.c
index 6d584f4..0685c2c 100644
--- a/arch/powerpc/platforms/52xx/lite5200.c
+++ b/arch/powerpc/platforms/52xx/lite5200.c
@@ -25,6 +25,7 @@
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/mpc52xx.h>
+#include <asm/udbg.h>
 
 /* ************************************************************************
  *
@@ -182,7 +183,8 @@ static int __init lite5200_probe(void)
 	if (!of_flat_dt_is_compatible(node, "fsl,lite5200") &&
 	    !of_flat_dt_is_compatible(node, "fsl,lite5200b"))
 		return 0;
-	pr_debug("%s board found\n", model ? model : "unknown");
+
+	udbg_printf("%s board found\n", model ? model : "unknown");
 
 	return 1;
 }
@@ -191,9 +193,11 @@ define_machine(lite5200) {
 	.name 		= "lite5200",
 	.probe 		= lite5200_probe,
 	.setup_arch 	= lite5200_setup_arch,
+	.init_early	= mpc52xx_udbg_init,
 	.init		= mpc52xx_declare_of_platform_devices,
 	.init_IRQ 	= mpc52xx_init_irq,
 	.get_irq 	= mpc52xx_get_irq,
 	.restart	= mpc52xx_restart,
 	.calibrate_decr	= generic_calibrate_decr,
+	.progress	= udbg_progress,
 };
diff --git a/arch/powerpc/platforms/52xx/mpc5200_simple.c b/arch/powerpc/platforms/52xx/mpc5200_simple.c
index a3bda0b..16daf9d 100644
--- a/arch/powerpc/platforms/52xx/mpc5200_simple.c
+++ b/arch/powerpc/platforms/52xx/mpc5200_simple.c
@@ -30,6 +30,7 @@
 #include <asm/prom.h>
 #include <asm/machdep.h>
 #include <asm/mpc52xx.h>
+#include <asm/udbg.h>
 
 /*
  * Setup the architecture
@@ -78,9 +79,11 @@ define_machine(mpc5200_simple_platform) {
 	.name		= "mpc5200-simple-platform",
 	.probe		= mpc5200_simple_probe,
 	.setup_arch	= mpc5200_simple_setup_arch,
+	.init_early	= mpc52xx_udbg_init,
 	.init		= mpc52xx_declare_of_platform_devices,
 	.init_IRQ	= mpc52xx_init_irq,
 	.get_irq	= mpc52xx_get_irq,
 	.restart	= mpc52xx_restart,
 	.calibrate_decr	= generic_calibrate_decr,
+	.progress	= udbg_progress,
 };
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
index 4d5fd1d..dcbeb06 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -10,7 +10,7 @@
  *
  */
 
-#undef DEBUG
+#define DEBUG
 
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
@@ -18,6 +18,18 @@
 #include <asm/io.h>
 #include <asm/prom.h>
 #include <asm/mpc52xx.h>
+#include <asm/udbg.h>
+#include <asm/pgalloc.h>
+#include <mm/mmu_decl.h>
+
+/* Programmable Serial Controller (PSC) status register bits */
+#define MPC52xx_PSC_SR		0x04
+#define MPC52xx_PSC_SR_RXRDY		0x0100
+#define MPC52xx_PSC_SR_RXFULL		0x0200
+#define MPC52xx_PSC_SR_TXRDY		0x0400
+#define MPC52xx_PSC_SR_TXEMP		0x0800
+
+#define MPC52xx_PSC_BUFFER	0x0C
 
 /* MPC5200 device tree match tables */
 static struct of_device_id mpc52xx_xlb_ids[] __initdata = {
@@ -36,6 +48,101 @@ static struct of_device_id mpc52xx_bus_ids[] __initdata = {
 	{}
 };
 
+static struct of_device_id mpc52xx_psc_uart_ids[] __initdata = {
+	{ .compatible = "fsl,mpc5200-psc-uart", },
+};
+
+static void __iomem *psc_console;
+
+static void mpc52xx_udbg_putc(char c)
+{
+	while (!(in_be16(psc_console + MPC52xx_PSC_SR) & MPC52xx_PSC_SR_TXRDY))
+		; /* wait for idle */
+	out_8(psc_console + MPC52xx_PSC_BUFFER, c);
+	if (c == '\n')
+		mpc52xx_udbg_putc('\r');
+}
+
+static int mpc52xx_udbg_getc(void)
+{
+	while (!(in_be16(psc_console + MPC52xx_PSC_SR) & MPC52xx_PSC_SR_RXRDY))
+		; /* wait for char */
+	return in_8(psc_console + MPC52xx_PSC_BUFFER);
+}
+
+static int mpc52xx_udbg_getc_poll(void)
+{
+	if (!(in_be16(psc_console + MPC52xx_PSC_SR) & MPC52xx_PSC_SR_RXRDY))
+		return -1;
+
+	return in_8(psc_console + MPC52xx_PSC_BUFFER);
+}
+
+
+#if defined(CONFIG_PPC_EARLY_DEBUG_5200)
+/**
+ * mpc52xx_udbg_init_early - Set up UDBG for early debug
+ *
+ * Called by udbg code if early debug is configured for the MPC5200
+ */
+void __init udbg_init_mpc5200(void)
+{
+	/* At this point, the bootwrapper or u-boot has already set up the
+	 * serial console correctly.  Assume that it is still configured
+	 * for the correct baud rate */
+
+	/* Map the entire IMMR range  (minimum mapping of 128k) */
+	psc_console = ioremap_bat(0xf0000000, 128<<10);
+	if (!psc_console)
+		return;
+
+	/* Adjust upwards for the base address of the PSC. */
+	psc_console += CONFIG_PPC_EARLY_DEBUG_5200_PHYSADDR - 0xf0000000;
+
+	/* See if it works */
+	mpc52xx_udbg_putc('=');
+
+	udbg_putc = mpc52xx_udbg_putc;
+	udbg_getc = mpc52xx_udbg_getc;
+	udbg_getc_poll = mpc52xx_udbg_getc_poll;
+	udbg_printf("psc_console=%p\n", psc_console);
+}
+#endif
+
+/**
+ * Initialize UDBG based on data in the device tree
+ */
+void __init mpc52xx_udbg_init(void)
+{
+	/* Lookup the console node */
+	struct device_node *stdout_node = of_lookup_stdout();
+	if (!stdout_node)
+		return;
+
+	/* Is this a PSC? */
+	if (!of_match_node(mpc52xx_psc_uart_ids, stdout_node)) {
+		of_node_put(stdout_node);
+		return;
+	}
+
+	/* Map the PSC registers */
+	psc_console = of_iomap(stdout_node, 0);
+	of_node_put(stdout_node);
+	if (!psc_console)
+		return;
+
+	/* See if it works */
+	mpc52xx_udbg_putc('+');
+
+	/* At this point, the bootwrapper or u-boot has already set up the
+	 * serial console correctly.  Assume that it is still configured
+	 * for the correct baud rate */
+	udbg_putc = mpc52xx_udbg_putc;
+	udbg_getc = mpc52xx_udbg_getc;
+	udbg_getc_poll = mpc52xx_udbg_getc_poll;
+	udbg_printf("psc_console=%p\n", psc_console);
+}
+
 /*
  * This variable is mapped in mpc52xx_map_wdt() and used in mpc52xx_restart().
  * Permanent mapping is required because mpc52xx_restart() can be called
diff --git a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
index 81ef10b..94e7f22 100644
--- a/include/asm-powerpc/mpc52xx.h
+++ b/include/asm-powerpc/mpc52xx.h
@@ -255,6 +255,7 @@ extern void mpc52xx_declare_of_platform_devices(void);
 extern void mpc52xx_map_common_devices(void);
 extern int mpc52xx_set_psc_clkdiv(int psc_id, int clkdiv);
 extern void mpc52xx_restart(char *cmd);
+extern void __init mpc52xx_udbg_init(void);
 
 /* mpc52xx_pic.c */
 extern void mpc52xx_init_irq(void);
diff --git a/include/asm-powerpc/udbg.h b/include/asm-powerpc/udbg.h
index 6418cee..ea6dd41 100644
--- a/include/asm-powerpc/udbg.h
+++ b/include/asm-powerpc/udbg.h
@@ -49,6 +49,7 @@ extern void __init udbg_init_debug_beat(void);
 extern void __init udbg_init_btext(void);
 extern void __init udbg_init_44x_as1(void);
 extern void __init udbg_init_40x_realmode(void);
+extern void __init udbg_init_mpc5200(void);
 extern void __init udbg_init_cpm(void);
 
 #endif /* __KERNEL__ */

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Milton Miller]

TimurTabi wrote:
> Yoder Stuart wrote:
> 
> > The second was the idea that we may need a
> > /aliases/stdin as well. You could conceptually have
> > stdout be a monitor and a stdin be a keyboard.
> 
> I don't think the hypervisor console subsystem supports
> this.  I don't see any way of registering separate clients
> for input vs. output.

Why should what the hvc driver support have any effect on
what the binding should be?

And anyways, both the boot wrapper and udbg (and hence early
debug console) have totally indepenedent functions for
reading and writing.  

And for that matter, we also support output on monitor and
input on keyboard for linear frame buffer on 6xx via the
btext output (but only adb input).

milton

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Timur Tabi]

Milton Miller wrote:

> Why should what the hvc driver support have any effect on
> what the binding should be?

Because my hypervisor console driver uses the hypervisor console/tty subsystem.
 If the subsystem assumes one stdout/stdin device, then I must abide by that.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function

[Scott Wood]

Timur Tabi wrote:
> Milton Miller wrote:
> 
>> Why should what the hvc driver support have any effect on
>> what the binding should be?
> 
> Because my hypervisor console driver uses the hypervisor console/tty subsystem.
>  If the subsystem assumes one stdout/stdin device, then I must abide by that.

So fix the subsystem, don't use the subsystem, or live with the fact 
that you support a subset of what the device tree can express.  Don't 
cripple the device tree because of that.

-Scott