Patchwork [U-Boot,RFC,06/10] MIPS: qemu-malta: add PCI support

login
register
mail settings
Submitter Gabor Juhos
Date Jan. 19, 2013, 3:19 p.m.
Message ID <1358608777-7270-7-git-send-email-juhosg@openwrt.org>
Download mbox | patch
Permalink /patch/213870/
State Superseded
Delegated to: Daniel Schwierzeck
Headers show

Comments

Gabor Juhos - Jan. 19, 2013, 3:19 p.m.
Qemu emulates the Galileo GT64120 System Controller
which provides a CPU bus to PCI bus bridge.

The patch adds driver for this bridge and enables
PCI support for the emulated Malta board.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
Cc: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
---
 board/qemu-malta/Makefile    |    5 +-
 board/qemu-malta/pci.c       |  173 ++++++++++++++++++++++++++++++++++++++++++
 include/configs/qemu-malta.h |    6 ++
 3 files changed, 183 insertions(+), 1 deletion(-)
 create mode 100644 board/qemu-malta/pci.c
Wolfgang Denk - Jan. 21, 2013, 12:56 p.m.
Dear Gabor Juhos,

In message <1358608777-7270-7-git-send-email-juhosg@openwrt.org> you wrote:
> Qemu emulates the Galileo GT64120 System Controller
> which provides a CPU bus to PCI bus bridge.
> 
> The patch adds driver for this bridge and enables
> PCI support for the emulated Malta board.
> 
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> ---
>  board/qemu-malta/Makefile    |    5 +-
>  board/qemu-malta/pci.c       |  173 ++++++++++++++++++++++++++++++++++++++++++
>  include/configs/qemu-malta.h |    6 ++
>  3 files changed, 183 insertions(+), 1 deletion(-)
>  create mode 100644 board/qemu-malta/pci.c
> 
> diff --git a/board/qemu-malta/Makefile b/board/qemu-malta/Makefile
> index 6251bb8..59c1b1d 100644
> --- a/board/qemu-malta/Makefile
> +++ b/board/qemu-malta/Makefile
> @@ -25,7 +25,10 @@ include $(TOPDIR)/config.mk
>  
>  LIB	= $(obj)lib$(BOARD).o
>  
> -COBJS	= $(BOARD).o
> +COBJS-y			+= $(BOARD).o
> +COBJS-$(CONFIG_PCI)	+= pci.o
> +
> +COBJS   := $(COBJS-y)
>  SOBJS	= lowlevel_init.o
>  
>  SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
> diff --git a/board/qemu-malta/pci.c b/board/qemu-malta/pci.c
> new file mode 100644
> index 0000000..823ae9b
> --- /dev/null
> +++ b/board/qemu-malta/pci.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright (C) 2013 Gabor Juhos <juhosg@openwrt.org>
> + *
> + * Based on the Linux implementation.
> + *   Copyright (C) 1999, 2000, 2004  MIPS Technologies, Inc.
> + *   Authors: Carsten Langgaard <carstenl@mips.com>
> + *            Maciej W. Rozycki <macro@mips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include <common.h>
> +#include <asm/addrspace.h>
> +#include <asm/gt64120.h>
> +#include <asm/malta.h>
> +#include <asm/io.h>
> +#include <pci.h>
> +
> +#define PCI_ACCESS_READ  0
> +#define PCI_ACCESS_WRITE 1
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * PCI controller "hose" value
> + */
> +
> +static struct pci_controller hose;
> +static void __iomem *gt_base = (void __iomem *)CKSEG1ADDR(MALTA_GT_BASE);
> +
> +static inline u32 gt_read_host_reg(unsigned reg)
> +{
> +	return __raw_readl(gt_base + reg);
> +}
> +
> +static inline void gt_write_host_reg(unsigned reg, u32 val)
> +{
> +	__raw_writel(val, gt_base + reg);
> +}
> +
> +static inline u32 gt_read_reg(unsigned reg)
> +{
> +	return le32_to_cpu(gt_read_host_reg(reg));
> +}
> +
> +static inline gt_write_reg(unsigned reg, u32 val)
> +{
> +	gt_write_host_reg(reg, cpu_to_le32(val));
> +}

I dislike that you introduce new I/O accessors here, and additionally
in a way which is explicitly discouraged in U-Boot.

We don't allow to access device registers through a base address plus
offset notation; instead, we use C structs to describe the register
layout.

Also, on real hardware your accessors areprobably lacking sufficient
memory barriers etc.

Is there any specific reason for not using the usual standard
accessors as provided by <asm/io.h> ?

Best regards,

Wolfgang Denk
Gabor Juhos - Jan. 22, 2013, 6:03 a.m.
Dear Wolfgang,

> I dislike that you introduce new I/O accessors here, and additionally
> in a way which is explicitly discouraged in U-Boot.
> 
> We don't allow to access device registers through a base address plus
> offset notation; instead, we use C structs to describe the register
> layout.

Sorry, I was not aware of these requirements.

> Also, on real hardware your accessors areprobably lacking sufficient
> memory barriers etc.

The lack of memory barriers should not cause any problems on real hardware.
Although I don't have a Malta board, but the original Linux code also does not
use memory barriers.

> Is there any specific reason for not using the usual standard
> accessors as provided by <asm/io.h> ?

No specific reason. The original Linux code uses custom macros for register
access, and I have converted those into inline functions.

Thank you for the review. I will fix the issues in the next version of the patch.

-Gabor

Patch

diff --git a/board/qemu-malta/Makefile b/board/qemu-malta/Makefile
index 6251bb8..59c1b1d 100644
--- a/board/qemu-malta/Makefile
+++ b/board/qemu-malta/Makefile
@@ -25,7 +25,10 @@  include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(BOARD).o
 
-COBJS	= $(BOARD).o
+COBJS-y			+= $(BOARD).o
+COBJS-$(CONFIG_PCI)	+= pci.o
+
+COBJS   := $(COBJS-y)
 SOBJS	= lowlevel_init.o
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
diff --git a/board/qemu-malta/pci.c b/board/qemu-malta/pci.c
new file mode 100644
index 0000000..823ae9b
--- /dev/null
+++ b/board/qemu-malta/pci.c
@@ -0,0 +1,173 @@ 
+/*
+ * Copyright (C) 2013 Gabor Juhos <juhosg@openwrt.org>
+ *
+ * Based on the Linux implementation.
+ *   Copyright (C) 1999, 2000, 2004  MIPS Technologies, Inc.
+ *   Authors: Carsten Langgaard <carstenl@mips.com>
+ *            Maciej W. Rozycki <macro@mips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <common.h>
+#include <asm/addrspace.h>
+#include <asm/gt64120.h>
+#include <asm/malta.h>
+#include <asm/io.h>
+#include <pci.h>
+
+#define PCI_ACCESS_READ  0
+#define PCI_ACCESS_WRITE 1
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * PCI controller "hose" value
+ */
+
+static struct pci_controller hose;
+static void __iomem *gt_base = (void __iomem *)CKSEG1ADDR(MALTA_GT_BASE);
+
+static inline u32 gt_read_host_reg(unsigned reg)
+{
+	return __raw_readl(gt_base + reg);
+}
+
+static inline void gt_write_host_reg(unsigned reg, u32 val)
+{
+	__raw_writel(val, gt_base + reg);
+}
+
+static inline u32 gt_read_reg(unsigned reg)
+{
+	return le32_to_cpu(gt_read_host_reg(reg));
+}
+
+static inline gt_write_reg(unsigned reg, u32 val)
+{
+	gt_write_host_reg(reg, cpu_to_le32(val));
+}
+
+#define GT_INTRCAUSE_ABORT_BITS	\
+		(GT_INTRCAUSE_MASABORT0_BIT | GT_INTRCAUSE_TARABORT0_BIT)
+
+static int gt_config_access(unsigned char access_type, pci_dev_t bdf,
+			    int where, u32 *data)
+{
+	unsigned int bus = PCI_BUS(bdf);
+	unsigned int dev = PCI_DEV(bdf);
+	unsigned int devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf);
+	u32 intr;
+	u32 addr;
+
+	if (bus == 0 && dev >= 31) {
+		/* Because of a bug in the galileo (for slot 31). */
+		return -1;
+	}
+
+	if (access_type == PCI_ACCESS_WRITE)
+		debug("PCI WR %02x:%02x.%x reg:%02d data:%08x\n",
+		       PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf),
+		       where, *data);
+
+	/* Clear cause register bits */
+	gt_write_host_reg(GT_INTRCAUSE_OFS, ~GT_INTRCAUSE_ABORT_BITS);
+
+	addr = GT_PCI0_CFGADDR_CONFIGEN_BIT;
+	addr |=	bus << GT_PCI0_CFGADDR_BUSNUM_SHF;
+	addr |=	devfn << GT_PCI0_CFGADDR_FUNCTNUM_SHF;
+	addr |= (where / 4) << GT_PCI0_CFGADDR_REGNUM_SHF;
+
+	/* Setup address */
+	gt_write_host_reg(GT_PCI0_CFGADDR_OFS, addr);
+
+	if (access_type == PCI_ACCESS_WRITE) {
+		if (bus == 0 && dev == 0) {
+			/*
+			 * The Galileo system controller is acting
+			 * differently than other devices.
+			 */
+			gt_write_host_reg(GT_PCI0_CFGDATA_OFS, *data);
+		} else
+			gt_write_reg(GT_PCI0_CFGDATA_OFS, *data);
+	} else {
+		if (bus == 0 && dev == 0) {
+			/*
+			 * The Galileo system controller is acting
+			 * differently than other devices.
+			 */
+			*data = gt_read_host_reg(GT_PCI0_CFGDATA_OFS);
+		} else
+			*data = gt_read_reg(GT_PCI0_CFGDATA_OFS);
+	}
+
+	/* Check for master or target abort */
+	intr = gt_read_host_reg(GT_INTRCAUSE_OFS);
+	if (intr & GT_INTRCAUSE_ABORT_BITS) {
+		/* Error occurred, clear abort bits */
+		gt_write_host_reg(GT_INTRCAUSE_OFS, ~GT_INTRCAUSE_ABORT_BITS);
+		return -1;
+	}
+
+	if (access_type == PCI_ACCESS_READ)
+		debug("PCI RD %02x:%02x.%x reg:%02d data:%08x\n",
+		      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), where, *data);
+
+	return 0;
+}
+
+static int gt_read_config_dword(struct pci_controller *hose, pci_dev_t dev,
+				int where, u32 *value)
+{
+	*value = 0xffffffff;
+	return gt_config_access(PCI_ACCESS_READ, dev, where, value);
+}
+
+static int gt_write_config_dword(struct pci_controller *hose, pci_dev_t dev,
+				 int where, u32 value)
+{
+	u32 data = value;
+
+	return gt_config_access(PCI_ACCESS_WRITE, dev, where, &data);
+}
+
+void pci_init_board(void)
+{
+	set_io_port_base(CKSEG1ADDR(MALTA_IO_PORT_BASE));
+
+	hose.first_busno = 0;
+	hose.last_busno = 0xff;
+
+	/* System memory space */
+	pci_set_region(&hose.regions[0],
+		       0x00000000, 0x00000000,
+		       CONFIG_SYS_MEM_SIZE,
+		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
+
+	/* PCI memory space */
+	pci_set_region(&hose.regions[1],
+		       0x10000000, 0x10000000,
+		       128 * 1024 * 1024,
+		       PCI_REGION_MEM);
+
+	/* PCI I/O space */
+	pci_set_region(&hose.regions[2],
+		       0x0000000, 0x0000000,
+		       0x20000,
+		       PCI_REGION_IO);
+
+	hose.region_count = 3;
+
+	pci_set_ops(&hose,
+		    pci_hose_read_config_byte_via_dword,
+		    pci_hose_read_config_word_via_dword,
+		    gt_read_config_dword,
+		    pci_hose_write_config_byte_via_dword,
+		    pci_hose_write_config_word_via_dword,
+		    gt_write_config_dword);
+
+	pci_register_hose(&hose);
+	hose.last_busno = pci_hose_scan(&hose);
+}
diff --git a/include/configs/qemu-malta.h b/include/configs/qemu-malta.h
index 881c15d..36b584a 100644
--- a/include/configs/qemu-malta.h
+++ b/include/configs/qemu-malta.h
@@ -17,6 +17,9 @@ 
  */
 #define CONFIG_QEMU_MALTA
 
+#define CONFIG_PCI
+#define CONFIG_PCI_PNP
+
 /*
  * CPU Configuration
  */
@@ -30,6 +33,7 @@ 
 
 #define CONFIG_SWAP_IO_SPACE
 
+
 /*
  * Memory map
  */
@@ -108,6 +112,8 @@ 
 #undef CONFIG_CMD_NET
 #undef CONFIG_CMD_NFS
 
+#define CONFIG_CMD_PCI
+
 #define CONFIG_SYS_LONGHELP		/* verbose help, undef to save memory */
 
 #endif /* _QEMU_MALTA_CONFIG_H */