Re: [PATCH] added io_map_base to pci_controller on Lemote 2e box

Re: [PATCH] added io_map_base to pci_controller on Lemote 2e box

[Zhang Le]

Sorry, I should've added -n and -s option to git format-patch.
If the patches themselves are ok, I will post them again, of course with
signed-off-by.

Zhang, Le

Re: [PATCH] defined a macro for lemote 2e box IO base

[Thiemo Seufer]

Zhang Le wrote:
> ---
>  arch/mips/include/asm/lemote/pci.h |   31 +++++++++++++++++++++++++++++++
>  arch/mips/lemote/lm2e/setup.c      |    9 ++-------
>  2 files changed, 33 insertions(+), 7 deletions(-)
>  create mode 100644 arch/mips/include/asm/lemote/pci.h
> 
> diff --git a/arch/mips/include/asm/lemote/pci.h b/arch/mips/include/asm/lemote/pci.h
> new file mode 100644
> index 0000000..8e5c9c3
> --- /dev/null
> +++ b/arch/mips/include/asm/lemote/pci.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (c) 2008 Zhang Le <r0bertz@gentoo.org>
> + *
> + *     This program is free software; you can redistribute it
> + *     and/or modify it under the terms of the GNU General
> + *     Public License as published by the Free Software
> + *     Foundation; either version 2 of the License, or (at your
> + *     option) any later version.
> + *
> + *     This program is distributed in the hope that it will be
> + *     useful, but WITHOUT ANY WARRANTY; without even the implied
> + *     warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
> + *     PURPOSE.  See the GNU General Public License for more
> + *     details.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this program; if not, write to the Free
> + *     Software Foundation, Inc., 675 Mass Ave, Cambridge, MA
> + *     02139, USA.
> + */
> +
> +#ifndef _LEMOTE_PCI_H_
> +#define _LEMOTE_PCI_H_
> +
> +#ifdef CONFIG_64BIT
> +#define LEMOTE_IO_PORT_BASE 0xffffffffbfd00000
> +#else
> +#define LEMOTE_IO_PORT_BASE 0xbfd00000
> +#endif

Why not "((const long)0xbfd00000)" instead? AFAICS it is never used in
assembly code.


Thiemo

Re: [PATCH] defined a macro for lemote 2e box IO base

[Ralf Baechle]

On Thu, Oct 23, 2008 at 12:48:58AM +0000, Zhang Le wrote:

> +#ifdef CONFIG_64BIT
> +#define LEMOTE_IO_PORT_BASE 0xffffffffbfd00000
> +#else
> +#define LEMOTE_IO_PORT_BASE 0xbfd00000
> +#endif

This sort of #ifdefery is one of the reasons why it's better to define
physical addresses of devices, not virtual addresses in header files.

  Ralf

[PATCH] defined a macro for lemote 2e box IO base

[Zhang Le]

---
 arch/mips/include/asm/lemote/pci.h |   31 +++++++++++++++++++++++++++++++
 arch/mips/lemote/lm2e/setup.c      |    9 ++-------
 2 files changed, 33 insertions(+), 7 deletions(-)
 create mode 100644 arch/mips/include/asm/lemote/pci.h

diff --git a/arch/mips/include/asm/lemote/pci.h b/arch/mips/include/asm/lemote/pci.h
new file mode 100644
index 0000000..8e5c9c3
--- /dev/null
+++ b/arch/mips/include/asm/lemote/pci.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2008 Zhang Le <r0bertz@gentoo.org>
+ *
+ *     This program is free software; you can redistribute it
+ *     and/or modify it under the terms of the GNU General
+ *     Public License as published by the Free Software
+ *     Foundation; either version 2 of the License, or (at your
+ *     option) any later version.
+ *
+ *     This program is distributed in the hope that it will be
+ *     useful, but WITHOUT ANY WARRANTY; without even the implied
+ *     warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ *     PURPOSE.  See the GNU General Public License for more
+ *     details.
+ *
+ *     You should have received a copy of the GNU General Public
+ *     License along with this program; if not, write to the Free
+ *     Software Foundation, Inc., 675 Mass Ave, Cambridge, MA
+ *     02139, USA.
+ */
+
+#ifndef _LEMOTE_PCI_H_
+#define _LEMOTE_PCI_H_
+
+#ifdef CONFIG_64BIT
+#define LEMOTE_IO_PORT_BASE 0xffffffffbfd00000
+#else
+#define LEMOTE_IO_PORT_BASE 0xbfd00000
+#endif
+
+#endif /* !_LEMOTE_PCI_H_ */
diff --git a/arch/mips/lemote/lm2e/setup.c b/arch/mips/lemote/lm2e/setup.c
index 2cc6745..6f7076d 100644
--- a/arch/mips/lemote/lm2e/setup.c
+++ b/arch/mips/lemote/lm2e/setup.c
@@ -34,6 +34,7 @@
 #include <asm/mc146818-time.h>
 #include <asm/time.h>
 #include <asm/wbflush.h>
+#include <asm/lemote/pci.h>
 
 #ifdef CONFIG_VT
 #include <linux/console.h>
@@ -42,12 +43,6 @@
 
 extern void mips_reboot_setup(void);
 
-#ifdef CONFIG_64BIT
-#define PTR_PAD(p) ((0xffffffff00000000)|((unsigned long long)(p)))
-#else
-#define PTR_PAD(p) (p)
-#endif
-
 unsigned long cpu_clock_freq;
 unsigned long bus_clock;
 unsigned int memsize;
@@ -80,7 +75,7 @@ static void wbflush_loongson2e(void)
 
 void __init plat_mem_setup(void)
 {
-	set_io_port_base(PTR_PAD(0xbfd00000));
+	set_io_port_base(LEMOTE_IO_PORT_BASE);
 
 	mips_reboot_setup();
 
-- 
1.6.0.2

[PATCH] added io_map_base to pci_controller on Lemote 2e box

[Zhang Le]

---
 arch/mips/lemote/lm2e/pci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/mips/lemote/lm2e/pci.c b/arch/mips/lemote/lm2e/pci.c
index c1e41f1..dd04957 100644
--- a/arch/mips/lemote/lm2e/pci.c
+++ b/arch/mips/lemote/lm2e/pci.c
@@ -30,6 +30,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <asm/mips-boards/bonito64.h>
+#include <asm/lemote/pci.h>
 
 extern struct pci_ops bonito64_pci_ops;
 
@@ -53,6 +54,7 @@ static struct pci_controller  loongson2e_pci_controller = {
 	.mem_resource   = &loongson2e_pci_mem_resource,
 	.mem_offset     = 0x00000000UL,
 	.io_offset      = 0x00000000UL,
+	.io_map_base    = LEMOTE_IO_PORT_BASE,
 };
 
 static void __init ict_pcimap(void)
-- 
1.6.0.2

Re: [PATCH] defined a macro for lemote 2e box IO base

[Zhang Le]

On 21:28 Wed 22 Oct     , Ralf Baechle wrote:
> On Thu, Oct 23, 2008 at 12:48:58AM +0000, Zhang Le wrote:
> 
> > +#ifdef CONFIG_64BIT
> > +#define LEMOTE_IO_PORT_BASE 0xffffffffbfd00000
> > +#else
> > +#define LEMOTE_IO_PORT_BASE 0xbfd00000
> > +#endif
> 
> This sort of #ifdefery is one of the reasons why it's better to define
> physical addresses of devices, not virtual addresses in header files.

Thanks for the comment.
I have checked how other platforms handle this problem.
Many have used CKSEG1ADDR.
So I have posted another set of patches here:
http://www.linux-mips.org/archives/linux-mips/2008-10/msg00189.html
http://www.linux-mips.org/archives/linux-mips/2008-10/msg00190.html

Is it OK?

Zhang, Le

Re: [PATCH] defined a macro for lemote 2e box IO base

[Maciej W. Rozycki]

On Fri, 24 Oct 2008, Zhang Le wrote:

> Thanks for the comment.
> I have checked how other platforms handle this problem.
> Many have used CKSEG1ADDR.

 Please note long-term we want CKSEG1ADDR() to go away from board/platform 
code and possibly only keep it for some generic use if at all.  Have you 
considered using ioremap()?  With a literal physical address it should get 
optimised to the same code as the use of CKSEG1ADDR() produces, yet keep 
the source portable and in line with the rest of the kernel.  I try to 
remove references to CKSEG1ADDR() as I come across them myself too.

  Maciej

Re: [PATCH] defined a macro for lemote 2e box IO base

[Ralf Baechle]

On Fri, Oct 24, 2008 at 09:24:05PM +0100, Maciej W. Rozycki wrote:

> > Thanks for the comment.
> > I have checked how other platforms handle this problem.
> > Many have used CKSEG1ADDR.
> 
>  Please note long-term we want CKSEG1ADDR() to go away from board/platform 
> code and possibly only keep it for some generic use if at all.  Have you 
> considered using ioremap()?  With a literal physical address it should get 
> optimised to the same code as the use of CKSEG1ADDR() produces, yet keep 
> the source portable and in line with the rest of the kernel.  I try to 
> remove references to CKSEG1ADDR() as I come across them myself too.

We should probably start removing them all over the source so people have
less bad examples to copy from.  The reason why KSEG0ADDR() and all the
other macros exist is that all the other UNIX operating systems before were
using something like it also.  But in Linux it has two disadvantages -
it duplicates the functionality of a generic interface in arch code which
is making for example the live of kernel janitors who don't know about
MIPSisms harder.  And it's not a terribly nice interface for dealing with
32-bit vs. 64-bit addresses.

  Ralf

Re: [PATCH] defined a macro for lemote 2e box IO base

[Zhang Le]

On 21:24 Fri 24 Oct     , Maciej W. Rozycki wrote:
> On Fri, 24 Oct 2008, Zhang Le wrote:
> 
> > Thanks for the comment.
> > I have checked how other platforms handle this problem.
> > Many have used CKSEG1ADDR.
> 
>  Please note long-term we want CKSEG1ADDR() to go away from board/platform 
> code and possibly only keep it for some generic use if at all.  Have you 
> considered using ioremap()?  With a literal physical address it should get 
> optimised to the same code as the use of CKSEG1ADDR() produces, yet keep 
> the source portable and in line with the rest of the kernel.

Thank you!
I will make a new patch.

Zhang, Le