Re: [PATCH 00/12] MIPS: ath79: AR724X PCI fixes and AR71XX PCI support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


2011/11/24 Gabor Juhos <juhosg@xxxxxxxxxxx>:
> Hi René,
>
>> Sorry Gabor for the following patch, but it seems your patchset was
>> against a other tree?
>
> Both of my patch sets was based on the 'mips-for-linux-next' branch of Ralf's
> 'upstream-sfr' tree:
>
> git://git.linux-mips.org/pub/scm/ralf/upstream-sfr.git
>
>> Because of the many failures I rebase'ed against 09521577ca7718b6c of the
>> linus tree and written anything from scratch.
>
> That was waste of time. The ath79 platform got a pile of changes recently, and
> those changes are not yet available in Linus' tree. If you were unsure about the
> tree, you should have asked earlier.
>

Alright. I don't play with all trees now.
I wait for the next merge.

>> - The ar724x pci build only then SOC_AR724X and AR724X_PCI is set.
>> (new symbol AR724X_PCI)
>> - We have a shared PCI header file for both controllers. (Only AR724x
>> is included atm)
>> - We have a default irq map if the board pass no other map with
>> ar724x_pci_add_data.
>> - I added the fix for the 7240 controller bug, hopefully right.
>
>> Gabor, can you please test the patch?
>
> I can tell without testing that this is not working, see below.
>
>>
>> René
>>
>> diff --git a/arch/mips/ath79/Kconfig b/arch/mips/ath79/Kconfig
>> index 4770741..e763661 100644
>> --- a/arch/mips/ath79/Kconfig
>> +++ b/arch/mips/ath79/Kconfig
>> @@ -23,6 +23,16 @@ config ATH79_MACH_PB44
>>         Say 'Y' here if you want your kernel to support the
>>         Atheros PB44 reference board.
>>
>> +config ATH79_MACH_UBNT_XM
>> +     bool "Ubiquiti Networks XM (rev 1.0) board"
>> +     select SOC_AR724X
>> +     select ATH79_DEV_GPIO_BUTTONS
>> +     select ATH79_DEV_LEDS_GPIO
>> +     select ATH79_DEV_SPI
>> +     help
>> +       Say 'Y' here if you want your kernel to support the
>> +       Ubiquiti Networks XM (rev 1.0) board.
>> +
>>  endmenu
>>
>>  config SOC_AR71XX
>> @@ -33,6 +43,7 @@ config SOC_AR71XX
>>  config SOC_AR724X
>>       select USB_ARCH_HAS_EHCI
>>       select USB_ARCH_HAS_OHCI
>> +     select HW_HAS_PCI
>>       def_bool n
>>
>>  config SOC_AR913X
>> @@ -52,4 +63,8 @@ config ATH79_DEV_LEDS_GPIO
>>  config ATH79_DEV_SPI
>>       def_bool n
>>
>> +config AR724X_PCI
>> +     depends on PCI
>> +     def_bool y
>> +
>>  endif
>
> Why would we have to add yet another Kconfig symbol? The AR724X specific code is
> only build when both PCI and SOC_AR724X is selected (in Ralf's tree, and in
> linux-next).
>
>> diff --git a/arch/mips/ath79/Makefile b/arch/mips/ath79/Makefile
>> index c33d465..ac9f375 100644
>> --- a/arch/mips/ath79/Makefile
>> +++ b/arch/mips/ath79/Makefile
>> @@ -26,3 +26,4 @@ obj-$(CONFIG_ATH79_DEV_SPI)         += dev-spi.o
>>  #
>>  obj-$(CONFIG_ATH79_MACH_AP81)                += mach-ap81.o
>>  obj-$(CONFIG_ATH79_MACH_PB44)                += mach-pb44.o
>> +obj-$(CONFIG_ATH79_MACH_UBNT_XM)     += mach-ubnt-xm.o
>> diff --git a/arch/mips/ath79/mach-ubnt-xm.c b/arch/mips/ath79/mach-ubnt-xm.c
>> new file mode 100644
>> index 0000000..6fadd0b
>> --- /dev/null
>> +++ b/arch/mips/ath79/mach-ubnt-xm.c
>> @@ -0,0 +1,121 @@
>> +/*
>> + *  Ubiquiti Networks XM (rev 1.0) board support
>> + *
>> + *  Copyright (C) 2011 René Bolldorf <xsecute@xxxxxxxxxxxxxx>
>> + *
>> + *  Derived from: mach-pb44.c
>> + *
>> + *  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 <linux/init.h>
>> +#include <linux/pci.h>
>> +
>> +#ifdef CONFIG_PCI
>> +#include <linux/ath9k_platform.h>
>> +#include <asm/mach-ath79/pci.h>
>> +#include <asm/mach-ath79/irq.h>
>> +#endif /* CONFIG_PCI */
>> +
>> +#include "machtypes.h"
>> +#include "dev-gpio-buttons.h"
>> +#include "dev-leds-gpio.h"
>> +#include "dev-spi.h"
>> +
>> +#define UBNT_XM_GPIO_LED_L1          0
>> +#define UBNT_XM_GPIO_LED_L2          1
>> +#define UBNT_XM_GPIO_LED_L3          11
>> +#define UBNT_XM_GPIO_LED_L4          7
>> +
>> +#define UBNT_XM_GPIO_BTN_RESET               12
>> +
>> +#define UBNT_XM_KEYS_POLL_INTERVAL   20
>> +#define UBNT_XM_KEYS_DEBOUNCE_INTERVAL       (3 * UBNT_XM_KEYS_POLL_INTERVAL)
>> +
>> +#define UBNT_XM_EEPROM_ADDR          (u8 *) KSEG1ADDR(0x1fff1000)
>> +
>> +static struct gpio_led ubnt_xm_leds_gpio[] __initdata = {
>> +     {
>> +             .name           = "ubnt-xm:red:link1",
>> +             .gpio           = UBNT_XM_GPIO_LED_L1,
>> +             .active_low     = 0,
>> +     }, {
>> +             .name           = "ubnt-xm:orange:link2",
>> +             .gpio           = UBNT_XM_GPIO_LED_L2,
>> +             .active_low     = 0,
>> +     }, {
>> +             .name           = "ubnt-xm:green:link3",
>> +             .gpio           = UBNT_XM_GPIO_LED_L3,
>> +             .active_low     = 0,
>> +     }, {
>> +             .name           = "ubnt-xm:green:link4",
>> +             .gpio           = UBNT_XM_GPIO_LED_L4,
>> +             .active_low     = 0,
>> +     },
>> +};
>> +
>> +static struct gpio_keys_button ubnt_xm_gpio_keys[] __initdata = {
>> +     {
>> +             .desc                   = "reset",
>> +             .type                   = EV_KEY,
>> +             .code                   = KEY_RESTART,
>> +             .debounce_interval      = UBNT_XM_KEYS_DEBOUNCE_INTERVAL,
>> +             .gpio                   = UBNT_XM_GPIO_BTN_RESET,
>> +             .active_low             = 1,
>> +     }
>> +};
>> +
>> +static struct spi_board_info ubnt_xm_spi_info[] = {
>> +     {
>> +             .bus_num        = 0,
>> +             .chip_select    = 0,
>> +             .max_speed_hz   = 25000000,
>> +             .modalias       = "mx25l6405d",
>> +     }
>> +};
>> +
>> +static struct ath79_spi_platform_data ubnt_xm_spi_data = {
>> +     .bus_num                = 0,
>> +     .num_chipselect         = 1,
>> +};
>> +
>> +#ifdef CONFIG_PCI
>> +static struct ath9k_platform_data ubnt_xm_eeprom_data;
>> +
>> +static struct ath79_pci_data ubnt_xm_pci_data[] = {
>> +     {
>> +             .slot   = 0,
>> +             .pin    = 1,
>> +             .irq    = ATH79_PCI_IRQ(0),
>> +             .pdata  = &ubnt_xm_eeprom_data,
>> +     },
>> +};
>> +#endif /* CONFIG_PCI */
>> +
>> +static void __init ubnt_xm_init(void)
>> +{
>> +     ath79_register_leds_gpio(-1, ARRAY_SIZE(ubnt_xm_leds_gpio),
>> +                              ubnt_xm_leds_gpio);
>> +
>> +     ath79_register_gpio_keys_polled(-1, UBNT_XM_KEYS_POLL_INTERVAL,
>> +                                     ARRAY_SIZE(ubnt_xm_gpio_keys),
>> +                                     ubnt_xm_gpio_keys);
>> +
>> +     ath79_register_spi(&ubnt_xm_spi_data, ubnt_xm_spi_info,
>> +                        ARRAY_SIZE(ubnt_xm_spi_info));
>> +
>> +#ifdef CONFIG_PCI
>> +     memcpy(ubnt_xm_eeprom_data.eeprom_data, UBNT_XM_EEPROM_ADDR,
>> +            sizeof(ubnt_xm_eeprom_data.eeprom_data));
>> +
>> +     ar724x_pci_add_data(ubnt_xm_pci_data, ARRAY_SIZE(ubnt_xm_pci_data));
>> +#endif /* CONFIG_PCI */
>> +
>> +}
>> +
>> +MIPS_MACHINE(ATH79_MACH_UBNT_XM,
>> +          "UBNT-XM",
>> +          "Ubiquiti Networks XM (rev 1.0) board",
>> +          ubnt_xm_init);
>> diff --git a/arch/mips/ath79/machtypes.h b/arch/mips/ath79/machtypes.h
>> index 3940fe4..35d5d5c 100644
>> --- a/arch/mips/ath79/machtypes.h
>> +++ b/arch/mips/ath79/machtypes.h
>> @@ -18,6 +18,7 @@ enum ath79_mach_type {
>>       ATH79_MACH_GENERIC = 0,
>>       ATH79_MACH_AP81,                /* Atheros AP81 reference board */
>>       ATH79_MACH_PB44,                /* Atheros PB44 reference board */
>> +     ATH79_MACH_UBNT_XM,             /* Ubiquiti Networks XM board rev 1.0 */
>>  };
>>
>>  #endif /* _ATH79_MACHTYPE_H */
>> diff --git a/arch/mips/include/asm/mach-ath79/irq.h
>> b/arch/mips/include/asm/mach-ath79/irq.h
>> index 189bc6e..eb68e79 100644
>> --- a/arch/mips/include/asm/mach-ath79/irq.h
>> +++ b/arch/mips/include/asm/mach-ath79/irq.h
>> @@ -10,11 +10,15 @@
>>  #define __ASM_MACH_ATH79_IRQ_H
>>
>>  #define MIPS_CPU_IRQ_BASE    0
>> -#define NR_IRQS                      16
>> +#define NR_IRQS                      22
>>
>
> This will conflict with other changes already in linux-next.
>
>>  #define ATH79_MISC_IRQ_BASE  8
>>  #define ATH79_MISC_IRQ_COUNT 8
>>
>> +#define ATH79_PCI_IRQ_BASE   (ATH79_MISC_IRQ_BASE + ATH79_MISC_IRQ_COUNT)
>> +#define ATH79_PCI_IRQ_COUNT  6
>> +#define ATH79_PCI_IRQ(_x)    (ATH79_PCI_IRQ_BASE + (_x))
>> +
>>  #define ATH79_CPU_IRQ_IP2    (MIPS_CPU_IRQ_BASE + 2)
>>  #define ATH79_CPU_IRQ_USB    (MIPS_CPU_IRQ_BASE + 3)
>>  #define ATH79_CPU_IRQ_GE0    (MIPS_CPU_IRQ_BASE + 4)
>> diff --git a/arch/mips/include/asm/mach-ath79/pci.h
>> b/arch/mips/include/asm/mach-ath79/pci.h
>> new file mode 100644
>> index 0000000..f671174
>> --- /dev/null
>> +++ b/arch/mips/include/asm/mach-ath79/pci.h
>> @@ -0,0 +1,24 @@
>> +/*
>> + *  Atheros 71xx/724x PCI support
>> + *
>> + *  Copyright (C) 2011 René Bolldorf <xsecute@xxxxxxxxxxxxxx>
>> + *  Copyright (C) 2009-2011 Gabor Juhos <juhosg@xxxxxxxxxxx>
>> + *
>> + *  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.
>> + */
>> +
>> +#ifndef __ASM_MACH_ATH79_PCI_H
>> +#define __ASM_MACH_ATH79_PCI_H
>> +
>> +struct ath79_pci_data {
>> +     uint8_t slot;
>> +     uint8_t pin;
>> +     int irq;
>> +     void *pdata;
>> +};
>> +
>> +void ar724x_pci_add_data(struct ath79_pci_data *data, int size);
>> +
>> +#endif /* __ASM_MACH_ATH79_PCI_H */
>> diff --git a/arch/mips/pci/Makefile b/arch/mips/pci/Makefile
>> index bb82cbd..6603594 100644
>> --- a/arch/mips/pci/Makefile
>> +++ b/arch/mips/pci/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_BCM47XX)               += pci-bcm47xx.o
>>  obj-$(CONFIG_BCM63XX)                += pci-bcm63xx.o fixup-bcm63xx.o \
>>                                       ops-bcm63xx.o
>>  obj-$(CONFIG_MIPS_ALCHEMY)   += pci-alchemy.o
>> +obj-$(CONFIG_AR724X_PCI)     += pci-ar724x.o
>>
>>  #
>>  # These are still pretty much in the old state, watch, go blind.
>> diff --git a/arch/mips/pci/pci-ar724x.c b/arch/mips/pci/pci-ar724x.c
>> new file mode 100644
>> index 0000000..66974fa
>> --- /dev/null
>> +++ b/arch/mips/pci/pci-ar724x.c
>> @@ -0,0 +1,284 @@
>> +/*
>> + *  Atheros 724x PCI support
>> + *
>> + *  Copyright (C) 2011 René Bolldorf <xsecute@xxxxxxxxxxxxxx>
>> + *  Copyright (C) 2009-2011 Gabor Juhos <juhosg@xxxxxxxxxxx>
>> + *
>> + *  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 <linux/pci.h>
>> +#include <linux/irq.h>
>> +#include <asm/mach-ath79/ath79.h>
>> +#include <asm/mach-ath79/pci.h>
>> +#include <asm/mach-ath79/irq.h>
>> +#include <asm/mach-ath79/ar71xx_regs.h>
>> +
>> +#define AR724X_PCI_CFG_BASE  0x14000000
>> +#define AR724X_PCI_CFG_SIZE  0x1000
>> +#define AR724X_PCI_CTRL_BASE (AR71XX_APB_BASE + 0x000f0000)
>> +#define AR724X_PCI_CTRL_SIZE 0x100
>> +
>> +#define AR724X_PCI_MEM_BASE  0x10000000
>> +#define AR724X_PCI_MEM_SIZE  0x08000000
>> +
>> +#define AR724X_PCI_REG_INT_STATUS    0x4c
>> +#define AR724X_PCI_REG_INT_MASK              0x50
>> +#define AR724X_PCI_INT_DEV0          BIT(14)
>> +
>> +#define AR7240_BAR0_WAR_VALUE        0xffff
>> +
>> +static DEFINE_SPINLOCK(ar724x_pci_lock);
>> +
>> +static void __iomem *ar724x_pci_devcfg_base;
>> +static void __iomem *ar724x_pci_ctrl_base;
>> +
>> +static struct ath79_pci_data *pci_data;
>> +static int pci_data_size = -1;
>> +
>> +static u32 ar724x_pci_bar0_value;
>> +static bool ar724x_pci_bar0_is_cached;
>> +
>> +static int ar724x_pci_read(struct pci_bus *bus, unsigned int devfn, int where,
>> +                         int size, uint32_t *value)
>> +{
>> +     unsigned long flags;
>> +     void __iomem *base;
>> +
>> +     if (devfn)
>> +             return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> +     base = ar724x_pci_devcfg_base;
>> +
>> +     spin_lock_irqsave(&ar724x_pci_lock, flags);
>> +
>> +     switch (size) {
>> +     case 1:
>> +             *value = (__raw_readl(base + (where & ~3)) & 0xff);
>
> This is wrong. This will always return the least significant byte, instead of
> the right one. And the outermost parens are not needed.
>
>> +             break;
>> +     case 2:
>> +             *value = (__raw_readl(base + (where & ~3)) & 0xffff);
>
> This is wrong as well.
>
>> +             break;
>> +     case 4:
>> +             if (soc_is_ar7240() && where == PCI_BASE_ADDRESS_0 &&
>> +                 ar724x_pci_bar0_is_cached)
>> +                     /* use the cached value */
>> +                     *value = ar724x_pci_bar0_value;
>> +             else
>> +                     *value = __raw_readl(base + where);
>> +             break;
>> +     default:
>> +             spin_unlock_irqrestore(&ar724x_pci_lock, flags);
>> +
>> +             return PCIBIOS_BAD_REGISTER_NUMBER;
>> +     }
>> +
>> +     spin_unlock_irqrestore(&ar724x_pci_lock, flags);
>> +
>> +     return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int ar724x_pci_write(struct pci_bus *bus, unsigned int devfn, int where,
>> +                          int size, uint32_t value)
>> +{
>> +     unsigned long flags;
>> +     void __iomem *base;
>> +
>> +     if (devfn)
>> +             return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> +     if (soc_is_ar7240() && where == PCI_BASE_ADDRESS_0 && size == 4) {
>> +             if (value != 0xffffffff) {
>> +                     /*
>> +                      * WAR for a hw issue. If the BAR0 register of the
>> +                      * device is set to the proper base address, the
>> +                      * memory space of the device is not accessible.
>> +                      *
>> +                      * Cache the intended value so it can be read back,
>> +                      * and write a SoC specific constant value to the
>> +                      * BAR0 register in order to make the device memory
>> +                      * accessible.
>> +                      */
>> +                     ar724x_pci_bar0_is_cached = true;
>> +                     ar724x_pci_bar0_value = value;
>> +                     value = AR7240_BAR0_WAR_VALUE;
>> +             } else {
>> +                     ar724x_pci_bar0_is_cached = false;
>> +             }
>> +     }
>> +
>> +     base = ar724x_pci_devcfg_base;
>> +
>> +     spin_lock_irqsave(&ar724x_pci_lock, flags);
>> +
>> +     switch (size) {
>> +     case 1:
>> +             value = (__raw_readl(base + (where & ~3)) & 0xff);
>> +             __raw_writel(value, base + (where & ~3));
>
> Wrong. This reads the register, and masks out everything but the least
> significant byte, and writes the results back to the same register. It must
> modify the right byte intead.
>
>> +             break;
>> +     case 2:
>> +             value = (__raw_readl(base + (where & ~3)) & 0xffff);
>> +             __raw_writel(value, base + (where & ~3));
>
> Also wrong (with words instead of bytes).
>
>> +             break;
>> +     case 4:
>> +             __raw_writel(value, (base + where));
>> +             break;
>> +     default:
>> +             spin_unlock_irqrestore(&ar724x_pci_lock, flags);
>> +
>> +             return PCIBIOS_BAD_REGISTER_NUMBER;
>> +     }
>> +
>> +     spin_unlock_irqrestore(&ar724x_pci_lock, flags);
>> +
>> +     return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static struct pci_ops ar724x_pci_ops = {
>> +     .read   = ar724x_pci_read,
>> +     .write  = ar724x_pci_write,
>> +};
>> +
>> +static struct resource ar724x_io_resource = {
>> +     .name   = "PCI IO space",
>> +     .start  = 0,
>> +     .end    = 0,
>> +     .flags  = IORESOURCE_IO,
>> +};
>> +
>> +static struct resource ar724x_mem_resource = {
>> +     .name   = "PCI memory space",
>> +     .start  = AR724X_PCI_MEM_BASE,
>> +     .end    = AR724X_PCI_MEM_BASE + AR724X_PCI_MEM_SIZE - 1,
>> +     .flags  = IORESOURCE_MEM,
>> +};
>> +
>> +static struct pci_controller ar724x_pci_controller = {
>> +     .pci_ops        = &ar724x_pci_ops,
>> +     .io_resource    = &ar724x_io_resource,
>> +     .mem_resource   = &ar724x_mem_resource,
>> +};
>> +
>> +static void ar724x_pci_irq_mask(struct irq_data *data)
>> +{
>> +     void __iomem *base;
>> +     u32 t;
>> +
>> +     base = ar724x_pci_ctrl_base;
>> +
>> +     switch (data->irq) {
>> +     case ATH79_PCI_IRQ(0):
>> +             t = __raw_readl(base + AR724X_PCI_REG_INT_MASK);
>> +             __raw_writel(t & ~AR724X_PCI_INT_DEV0,
>> +                          base + AR724X_PCI_REG_INT_MASK);
>
> A __raw_readl is missing.
>
>> +
>> +             t = __raw_readl(base + AR724X_PCI_REG_INT_STATUS);
>> +             __raw_writel(t | AR724X_PCI_INT_DEV0,
>> +                          base + AR724X_PCI_REG_INT_STATUS);
>
> Here too.
>
>> +     }
>> +}
>> +
>> +static void ar724x_pci_irq_unmask(struct irq_data *data)
>> +{
>> +     void __iomem *base;
>> +     u32 t;
>> +
>> +     base = ar724x_pci_ctrl_base;
>> +
>> +     switch (data->irq) {
>> +     case ATH79_PCI_IRQ(0):
>> +             t = __raw_readl(base + AR724X_PCI_REG_INT_MASK);
>> +             __raw_writel(t | AR724X_PCI_INT_DEV0,
>> +                          base + AR724X_PCI_REG_INT_MASK);
>
> And here also.
>
>> +     }
>> +}
>> +
>> +static struct irq_chip ar724x_pci_irq_chip = {
>> +     .name           = "AR724X PCI",
>> +     .irq_mask       = ar724x_pci_irq_mask,
>> +     .irq_unmask     = ar724x_pci_irq_unmask,
>> +     .irq_mask_ack   = ar724x_pci_irq_mask,
>> +};
>> +
>> +static __initconst struct ath79_pci_data ar724x_default_pci_data[] = {
>
> __initconst must be placed after [].
>
>> +     {
>> +             .slot = 0,
>> +             .pin  = 1,
>> +             .irq  = ATH79_PCI_IRQ(0),
>> +     },
>> +};
>> +
>> +void ar724x_pci_add_data(struct ath79_pci_data *data, int size)
>> +{
>> +     pci_data        = data;
>> +     pci_data_size   = size;
>> +}
>> +
>> +int __init pcibios_map_irq(const struct pci_dev *dev, uint8_t slot,
>> uint8_t pin)
>> +{
>> +     int irq = -1;
>> +     int i;
>> +
>> +     if (pci_data_size == -1)
>> +             return irq;
>> +
>> +     for (i = 0; i < pci_data_size; i++) {
>> +             if ((pci_data[i].slot == slot) && (pci_data[i].pin == pin)) {
>> +                     if (pci_data[i].irq != 0)
>> +                             irq = pci_data[i].irq;
>> +             break;
>
> Wrong indentation for the break statement.
>
>> +             }
>> +     }
>> +
>> +     return irq;
>> +}
>> +
>> +int pcibios_plat_dev_init(struct pci_dev *dev)
>> +{
>> +     int i;
>> +
>> +     if (pci_data_size == -1)
>> +             return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> +     for (i = 0; i < pci_data_size; i++) {
>> +             if (pci_data[i].slot == PCI_SLOT(dev->devfn)) {
>> +                     if (pci_data[i].pdata != NULL)
>> +                             dev->dev.platform_data = pci_data[i].pdata;
>> +             break;
>
> Ditto.
>
>> +             }
>> +     }
>> +
>> +     return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int __init ar724x_pcibios_init(void)
>> +{
>> +     int i;
>> +
>> +     ar724x_pci_devcfg_base = ioremap_nocache(AR724X_PCI_CFG_BASE,
>> +                                              AR724X_PCI_CFG_SIZE);
>
> ioremap can be used instead of ioremap_nocache.
>
>> +     if (ar724x_pci_devcfg_base == NULL)
>> +             return -ENOMEM;
>> +
>> +     ar724x_pci_ctrl_base = ioremap_nocache(AR724X_PCI_CTRL_BASE,
>> +                                               AR724X_PCI_CTRL_SIZE);
>> +     if (ar724x_pci_ctrl_base == NULL)
>> +             return -ENOMEM;
>
> ar724x_pci_devcfg_base must be unmapped if the second iomap call fails.
>
>> +
>> +     if (pci_data == NULL)
>> +             pci_data = ar724x_default_pci_data;
>> +             pci_data_size = ARRAY_SIZE(ar724x_default_pci_data);
>
> Braces are missing from this if statement.
>
> The AR724X_PCI_IRQ_REG_INT_{MASK,STATUS} registers must be cleared here.
>
>> +
>> +     for (i = ATH79_PCI_IRQ_BASE;
>> +          i < ATH79_PCI_IRQ_BASE + ATH79_PCI_IRQ_COUNT; i++)
>> +             irq_set_chip_and_handler(i, &ar724x_pci_irq_chip,
>> +                                      handle_level_irq);
>
> The 'irq_set_chained_handler' call is missing here. And the
> ar724x_pci_irq_handler function is completely missing from the patch.
>
>> +
>> +     register_pci_controller(&ar724x_pci_controller);
>> +
>> +     return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +arch_initcall(ar724x_pcibios_init);
>
> Regards,
> Gabor
>



[Linux MIPS Home]     [LKML Archive]     [Linux ARM]     [Linux]     [Git]     [Photo]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

Add to Google Powered by Linux