lkml.org 
[lkml]   [2004]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH][PPC32] Marvell host bridge support (mv64x60)
"Mark A. Greer" <mgreer@mvista.com> wrote:
>
> This patch adds core support for a line of host bridges from Marvell
> (formerly Galileo). This code has been tested with a GT64260a,
> GT64260b, MV64360, and MV64460. Patches for platforms that use these
> bridges will be sent separately.
>

Shouldn't these guys:


+ u32 cpu2mem_tab[MV64x60_CPU2MEM_WINDOWS][2] = {
+ { MV64x60_CPU2MEM_0_BASE, MV64x60_CPU2MEM_0_SIZE },
+ { MV64x60_CPU2MEM_1_BASE, MV64x60_CPU2MEM_1_SIZE },
+ { MV64x60_CPU2MEM_2_BASE, MV64x60_CPU2MEM_2_SIZE },
+ { MV64x60_CPU2MEM_3_BASE, MV64x60_CPU2MEM_3_SIZE }
+ };
+ u32 com2mem_tab[MV64x60_CPU2MEM_WINDOWS][2] = {
+ { MV64360_MPSC2MEM_0_BASE, MV64360_MPSC2MEM_0_SIZE },
+ { MV64360_MPSC2MEM_1_BASE, MV64360_MPSC2MEM_1_SIZE },
+ { MV64360_MPSC2MEM_2_BASE, MV64360_MPSC2MEM_2_SIZE },
+ { MV64360_MPSC2MEM_3_BASE, MV64360_MPSC2MEM_3_SIZE }
+ };
+ u32 dram_selects[MV64x60_CPU2MEM_WINDOWS] = { 0xe, 0xd, 0xb, 0x7 };
be static, and maybe __devinitdata? Right now, the CPU has to populate
them by hand at runtime.

+wait_for_ownership(int chan)
+{
+ int i;
+
+ for (i=0; i<MAX_TX_WAIT; i++) {
+ if ((MV64x60_REG_READ(sdma_regs[chan].sdcm) &
+ SDMA_SDCM_TXD) == 0)
+ break;
+
+ udelay(1000);
ow, big busywait. Can't use a sleep in here? I guess not :(

+ * arch/ppc/boot/simple/mv64x60_tty.c

hm. Normally we put arch-specific drivers like this into drivers/serial
and do the appropriate Kconfig work. Is there a reason why this serial
driver is buried under arch/ppc?

+#include "../../../../drivers/serial/mpsc_defs.h"

erk.

+struct mv64x60_rx_desc {
+ volatile u16 bufsize;
+ volatile u16 bytecnt;
+ volatile u32 cmd_stat;
+ volatile u32 next_desc_ptr;
+ volatile u32 buffer;
+};
+
+struct mv64x60_tx_desc {
+ volatile u16 bytecnt;
+ volatile u16 shadow;
+ volatile u32 cmd_stat;
+ volatile u32 next_desc_ptr;
+ volatile u32 buffer;
+};
Do these need to be volatile? If so, it indicates that the driver is doing
something wrong.

+gt64260_register_hdlrs(void)
+{
+ /* Register CPU interface error interrupt handler */
+ request_irq(MV64x60_IRQ_CPU_ERR, gt64260_cpu_error_int_handler,
+ SA_INTERRUPT, CPU_INTR_STR, 0);
request_irq() can fail.

+int
+mv64360_get_irq(struct pt_regs *regs)
+{
+ int irq;
+ int irq_gpp;
+
+#ifdef CONFIG_SMP
+ /*
+ * Second CPU gets only doorbell (message) interrupts.
+ * The doorbell interrupt is BIT28 in the main interrupt low cause reg.
+ */
+ int cpu_nr = smp_processor_id();
This function has no callers, so I am unable to determine whether it is
called from non-preemptible code. If it is called from preemptible code
then that smp_processor_id() is buggy, because you can switch CPUs at any
time.


+static struct platform_device mpsc_shared_device = { /* Shared device */
+ .name = MPSC_SHARED_NAME,
+ .id = 0,
+ .num_resources = ARRAY_SIZE(mv64x60_mpsc_shared_resources),
+ .resource = mv64x60_mpsc_shared_resources,
+ .dev = {
+ .driver_data = (void *)&mv64x60_mpsc_shared_pd_dd,
+ },
+};
The cast to void* is unnecessary.

+ (void)mv64x60_setup_for_chip(&bh);

how come you always stick that (void) in there?

+mv64x60_config_cpu2mem_windows(struct mv64x60_handle *bh,
+ struct mv64x60_setup_info *si,
+ u32 mem_windows[MV64x60_CPU2MEM_WINDOWS][2])
+{
+ u32 i, win;
+ u32 prot_tab[] = {
+ MV64x60_CPU_PROT_0_WIN, MV64x60_CPU_PROT_1_WIN,
+ MV64x60_CPU_PROT_2_WIN, MV64x60_CPU_PROT_3_WIN
+ };
+ u32 cpu_snoop_tab[] = {
+ MV64x60_CPU_SNOOP_0_WIN, MV64x60_CPU_SNOOP_1_WIN,
+ MV64x60_CPU_SNOOP_2_WIN, MV64x60_CPU_SNOOP_3_WIN
+ };
static initialisation?

+mv64x60_config_cpu2pci_windows(struct mv64x60_handle *bh,
+ struct mv64x60_pci_info *pi, u32 bus)
+{
+ int i;
+ u32 win_tab[2][4] = {
+ { MV64x60_CPU2PCI0_IO_WIN, MV64x60_CPU2PCI0_MEM_0_WIN,
+ MV64x60_CPU2PCI0_MEM_1_WIN,
+ MV64x60_CPU2PCI0_MEM_2_WIN },
+ { MV64x60_CPU2PCI1_IO_WIN, MV64x60_CPU2PCI1_MEM_0_WIN,
+ MV64x60_CPU2PCI1_MEM_1_WIN,
+ MV64x60_CPU2PCI1_MEM_2_WIN },
+ };
+ u32 remap_tab[2][4] = {
+ { MV64x60_CPU2PCI0_IO_REMAP_WIN,
+ MV64x60_CPU2PCI0_MEM_0_REMAP_WIN,
+ MV64x60_CPU2PCI0_MEM_1_REMAP_WIN,
+ MV64x60_CPU2PCI0_MEM_2_REMAP_WIN },
+ { MV64x60_CPU2PCI1_IO_REMAP_WIN,
+ MV64x60_CPU2PCI1_MEM_0_REMAP_WIN,
+ MV64x60_CPU2PCI1_MEM_1_REMAP_WIN,
+ MV64x60_CPU2PCI1_MEM_2_REMAP_WIN }
+ };
+
ditto

+mv64x60_config_pci2mem_windows(struct mv64x60_handle *bh,

and here

+mv64360_set_pci2mem_window(struct pci_controller *hose, u32 bus, u32 window,

and here

+mv64360_config_io2mem_windows(struct mv64x60_handle *bh,

and here


Anyway, I'll stick this as-is in -mm. Feel free to send an incremental
patch, or a replacement.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2011 Jasper Spaans