Hi Sergei, Does Linux 2.6.14.2 support TI PCI1520-EP (CardBus controller)?
Thanks Kevin -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Sergei Shtylyov Sent: Friday, September 25, 2009 9:38 AM To: [email protected] Cc: [email protected]; Ralf Baechle; Greg Kroah-Hartman; Wolfram Sang; [email protected] Subject: Re: [PATCH] MIPS: BCM63xx: Add PCMCIA & Cardbus support. Hello. Maxime Bizon wrote: >>On Wed, 2009-09-23 at 14:31 +0200, Wolfram Sang wrote: >>>Okay, here is a fast review. If you fix the mentioned points (or give me good >>>reasons why not ;)), then you might add my >>>Reviewed-by: Wolfram Sang <[email protected]> >>>I am fine with Ralf picking this up. >>Agreed on all your points and will fix them. Thanks. >>Ralf, please give me a couple of days to fix this and I will send you an >>updated patch. > Here it is: > Signed-off-by: Maxime Bizon <[email protected]> > Signed-off-by: Ralf Baechle <[email protected]> > Reviewed-by: Wolfram Sang <[email protected]> [...] > diff --git a/arch/mips/bcm63xx/dev-pcmcia.c b/arch/mips/bcm63xx/dev-pcmcia.c > new file mode 100644 > index 0000000..4fb42ac > --- /dev/null > +++ b/arch/mips/bcm63xx/dev-pcmcia.c > @@ -0,0 +1,135 @@ > +/* > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + * > + * Copyright (C) 2008 Maxime Bizon <[email protected]> > + */ > + > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <asm/bootinfo.h> > +#include <linux/platform_device.h> > +#include <bcm63xx_cs.h> > +#include <bcm63xx_cpu.h> > +#include <bcm63xx_dev_pcmcia.h> > +#include <bcm63xx_io.h> > +#include <bcm63xx_regs.h> > + > +static struct resource pcmcia_resources[] = { > + /* pcmcia registers */ > + { > + .start = -1, /* filled at runtime */ > + .end = -1, /* filled at runtime */ Then why initialize it? [...] > + .flags = IORESOURCE_MEM, > + }, > +static const __initdata unsigned int pcmcia_cs[3][3] = { > + /* cs, base address, size */ Shouln't this be array of structures instead? > + { MPI_CS_PCMCIA_COMMON, BCM_PCMCIA_COMMON_BASE_PA, > + BCM_PCMCIA_COMMON_SIZE }, > + > + { MPI_CS_PCMCIA_ATTR, BCM_PCMCIA_ATTR_BASE_PA, > + BCM_PCMCIA_ATTR_SIZE }, > + > + { MPI_CS_PCMCIA_IO, BCM_PCMCIA_IO_BASE_PA, > + BCM_PCMCIA_IO_SIZE }, > +}; > + > +int __init bcm63xx_pcmcia_register(void) > +{ > + int ret, i; > + > + if (!BCMCPU_IS_6348() && !BCMCPU_IS_6358()) > + return 0; > + > + /* use correct pcmcia ready gpio depending on processor */ > + switch (bcm63xx_get_cpu_id()) { > + case BCM6348_CPU_ID: > + pd.ready_gpio = 22; > + break; > + > + case BCM6358_CPU_ID: > + pd.ready_gpio = 18; > + break; Is this GPIO selection really chip- and not board-specific? [...] > diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_pcmcia.h > b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_pcmcia.h > new file mode 100644 > index 0000000..2beb396 > --- /dev/null > +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_pcmcia.h > @@ -0,0 +1,13 @@ > +#ifndef BCM63XX_DEV_PCMCIA_H_ > +#define BCM63XX_DEV_PCMCIA_H_ > + > +/* > + * PCMCIA driver platform data > + */ > +struct bcm63xx_pcmcia_platform_data { > + unsigned int ready_gpio; > +}; > + > +int bcm63xx_pcmcia_register(void); > + > +#endif /* BCM63XX_DEV_PCMCIA_H_ */ > diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig > index fbf965b..17f38a7 100644 > --- a/drivers/pcmcia/Kconfig > +++ b/drivers/pcmcia/Kconfig > @@ -192,6 +192,10 @@ config PCMCIA_AU1X00 > tristate "Au1x00 pcmcia support" > depends on SOC_AU1X00 && PCMCIA > > +config PCMCIA_BCM63XX > + tristate "bcm63xx pcmcia support" > + depends on BCM63XX && PCMCIA > + > config PCMCIA_SA1100 > tristate "SA1100 support" > depends on ARM && ARCH_SA1100 && PCMCIA I don't think having both board code and the driver in a single patch is a good idea. I'd put the driver in its own separate patch... > diff --git a/drivers/pcmcia/bcm63xx_pcmcia.c b/drivers/pcmcia/bcm63xx_pcmcia.c > new file mode 100644 > index 0000000..ef186c6 > --- /dev/null > +++ b/drivers/pcmcia/bcm63xx_pcmcia.c [...] > +static struct pci_device_id bcm63xx_cb_table[] = { > + { > + .vendor = PCI_VENDOR_ID_BROADCOM, > + .device = PCI_ANY_ID, Are you sure you can drive any Broadcom's bridge? > + .subvendor = PCI_VENDOR_ID_BROADCOM, > + .subdevice = PCI_ANY_ID, > + .class = PCI_CLASS_BRIDGE_CARDBUS << 8, > + .class_mask = ~0, > + }, > + > + { }, > +}; > + > +MODULE_DEVICE_TABLE(pci, bcm63xx_cb_table); > + > +static struct pci_driver bcm63xx_cardbus_driver = { > + .name = "yenta_cardbus", Why it's called the same as 'yenta_cardbus_driver' and not "bcm63xx_cardbus"? > + .id_table = bcm63xx_cb_table, > + .probe = bcm63xx_cb_probe, > + .remove = __devexit_p(bcm63xx_cb_exit), > +}; > +#endif [...] > diff --git a/drivers/pcmcia/bcm63xx_pcmcia.h b/drivers/pcmcia/bcm63xx_pcmcia.h > new file mode 100644 > index 0000000..85de866 > --- /dev/null > +++ b/drivers/pcmcia/bcm63xx_pcmcia.h > @@ -0,0 +1,65 @@ > +#ifndef BCM63XX_PCMCIA_H_ > +#define BCM63XX_PCMCIA_H_ > + > +#include <linux/types.h> > +#include <linux/timer.h> > +#include <pcmcia/ss.h> > +#include <bcm63xx_dev_pcmcia.h> > + > +/* socket polling rate in ms */ > +#define BCM63XX_PCMCIA_POLL_RATE 500 > + > +enum { > + CARD_CARDBUS = (1 << 0), > + > + CARD_PCCARD = (1 << 1), > + > + CARD_5V = (1 << 2), > + > + CARD_3V = (1 << 3), > + > + CARD_XV = (1 << 4), > + > + CARD_YV = (1 << 5), Why so many empty lines in between? > +}; WBR, Sergei _______________________________________________ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia _______________________________________________ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
