Prev: PROBLEM: SIS7019 stops recording after 42 min
Next: [PATCH v3] alsa: ASoC: Add JZ4740 codec driver
From: Lars-Peter Clausen on 1 Jul 2010 11:50 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Andrew Morton wrote: > On Mon, 28 Jun 2010 03:20:41 +0200 > Lars-Peter Clausen <lars(a)metafoo.de> wrote: > >> This patch adds support for the mmc controller on JZ4740 SoCs. >> >> Signed-off-by: Lars-Peter Clausen <lars(a)metafoo.de> >> Cc: Andrew Morton <akpm(a)linux-foundation.org> >> Cc: Matt Fleming <matt(a)console-pimps.org> >> Cc: linux-mmc(a)vger.kernel.org >> >> ... >> >> +#define JZ4740_MMC_MAX_TIMEOUT 10000000 > > That was a really big timeout. How long do 1e7 readw's take? Oh well. > Hm, right. It doesn't hurt though. I'll do some tests and to try to come up with a more realistic value. >> ... >> >> +static void jz4740_mmc_clock_disable(struct jz4740_mmc_host *host) >> +{ >> + uint32_t status; >> + >> + writew(JZ_MMC_STRPCL_CLOCK_STOP, host->base + JZ_REG_MMC_STRPCL); >> + do { >> + status = readl(host->base + JZ_REG_MMC_STATUS); >> + } while (status & JZ_MMC_STATUS_CLK_EN); >> +} >> + >> +static void jz4740_mmc_reset(struct jz4740_mmc_host *host) >> +{ >> + uint32_t status; >> + >> + writew(JZ_MMC_STRPCL_RESET, host->base + JZ_REG_MMC_STRPCL); >> + udelay(10); >> + do { >> + status = readl(host->base + JZ_REG_MMC_STATUS); >> + } while (status & JZ_MMC_STATUS_IS_RESETTING); >> +} > > Maybe these should have a timeout too? Its very unlikely that these will ever timeout. But right, to be on the safe, there should be a timeout as well. > >> ... >> >> +static inline unsigned int jz4740_mmc_wait_irq(struct jz4740_mmc_host *host, >> + unsigned int irq) >> +{ >> + unsigned int timeout = JZ4740_MMC_MAX_TIMEOUT; >> + uint16_t status; >> + >> + do { >> + status = readw(host->base + JZ_REG_MMC_IREG); >> + } while (!(status & irq) && --timeout); >> + >> + return timeout; >> +} > > This guy's too big to inline. Recent gcc's know that and they tend to > uninline such things behind your back anwyay. Actually, even without the inline attribute and compiling with -Os gcc will inline this function by itself. > >> ... >> >> +struct jz4740_mmc_platform_data { >> + int gpio_power; >> + int gpio_card_detect; >> + int gpio_read_only; >> + unsigned card_detect_active_low:1; >> + unsigned read_only_active_low:1; >> + unsigned power_active_low:1; >> + >> + unsigned data_1bit:1; >> +}; > > The bitfields will all share the same word, so modification of one > field can race against modification of another field. Hence some form > of locking which covers *all* the bitfields is needed. > > Is that a problem in this driver? > They are all read-only in the driver. Thanks for reviewing the patch - - Lars -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkwsuDQACgkQBX4mSR26RiOiMACeMMNj4koCYFAUnxyM0LBr+wOZ x6oAnizk5CaAvZjQ2doXrD6ZYgDeNjSr =92D2 -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Randy Dunlap on 12 Jul 2010 17:50 Lars-Peter Clausen wrote: > This patch adds support for the mmc controller on JZ4740 SoCs. > > Signed-off-by: Lars-Peter Clausen <lars(a)metafoo.de> > Acked-by: Matt Fleming <matt(a)console-pimps.org> > Cc: Andrew Morton <akpm(a)linux-foundation.org> > Cc: Matt Fleming <matt(a)console-pimps.org> > Cc: linux-mmc(a)vger.kernel.org > > --- > arch/mips/include/asm/mach-jz4740/jz4740_mmc.h | 15 + > drivers/mmc/host/Kconfig | 8 + > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/jz4740_mmc.c | 1024 ++++++++++++++++++++++++ > 4 files changed, 1048 insertions(+), 0 deletions(-) > create mode 100644 arch/mips/include/asm/mach-jz4740/jz4740_mmc.h > create mode 100644 drivers/mmc/host/jz4740_mmc.c > > diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h > new file mode 100644 > index 0000000..8543f43 > --- /dev/null > +++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h > @@ -0,0 +1,15 @@ > +#ifndef __LINUX_MMC_JZ4740_MMC > +#define __LINUX_MMC_JZ4740_MMC > + > +struct jz4740_mmc_platform_data { > + int gpio_power; > + int gpio_card_detect; > + int gpio_read_only; > + unsigned card_detect_active_low:1; > + unsigned read_only_active_low:1; > + unsigned power_active_low:1; > + > + unsigned data_1bit:1; > +}; > + > +#endif > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index f06d06e..546fc49 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -81,6 +81,14 @@ config MMC_RICOH_MMC > > If unsure, say Y. > > +config MMC_JZ4740 > + tristate "JZ4740 SD/Multimedia Card Interface support" > + depends on MACH_JZ4740 What tree has the kconfig symbol MACH_JZ4740 in it? I can't seem to find it... Should the depends also say anything about GPIO? I only ask because the header file above mentions gpio. > + help > + This selects the Ingenic Z4740 SD/Multimedia card Interface. > + If you have an ngenic platform with a Multimedia Card slot, Ingenic ? > + say Y or M here. > + > config MMC_SDHCI_OF > tristate "SDHCI support on OpenFirmware platforms" > depends on MMC_SDHCI && PPC_OF -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lars-Peter Clausen on 12 Jul 2010 18:10 Hi Randy Dunlap wrote: > Lars-Peter Clausen wrote: >> This patch adds support for the mmc controller on JZ4740 SoCs. >> >> Signed-off-by: Lars-Peter Clausen <lars(a)metafoo.de> >> Acked-by: Matt Fleming <matt(a)console-pimps.org> >> Cc: Andrew Morton <akpm(a)linux-foundation.org> >> Cc: Matt Fleming <matt(a)console-pimps.org> >> Cc: linux-mmc(a)vger.kernel.org >> >> --- >> arch/mips/include/asm/mach-jz4740/jz4740_mmc.h | 15 + >> drivers/mmc/host/Kconfig | 8 + >> drivers/mmc/host/Makefile | 1 + >> drivers/mmc/host/jz4740_mmc.c | 1024 ++++++++++++++++++++++++ >> 4 files changed, 1048 insertions(+), 0 deletions(-) >> create mode 100644 arch/mips/include/asm/mach-jz4740/jz4740_mmc.h >> create mode 100644 drivers/mmc/host/jz4740_mmc.c >> >> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h >> new file mode 100644 >> index 0000000..8543f43 >> --- /dev/null >> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h >> @@ -0,0 +1,15 @@ >> +#ifndef __LINUX_MMC_JZ4740_MMC >> +#define __LINUX_MMC_JZ4740_MMC >> + >> +struct jz4740_mmc_platform_data { >> + int gpio_power; >> + int gpio_card_detect; >> + int gpio_read_only; >> + unsigned card_detect_active_low:1; >> + unsigned read_only_active_low:1; >> + unsigned power_active_low:1; >> + >> + unsigned data_1bit:1; >> +}; >> + >> +#endif >> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig >> index f06d06e..546fc49 100644 >> --- a/drivers/mmc/host/Kconfig >> +++ b/drivers/mmc/host/Kconfig >> @@ -81,6 +81,14 @@ config MMC_RICOH_MMC >> >> If unsure, say Y. >> >> +config MMC_JZ4740 >> + tristate "JZ4740 SD/Multimedia Card Interface support" >> + depends on MACH_JZ4740 > > What tree has the kconfig symbol MACH_JZ4740 in it? > I can't seem to find it... > > Should the depends also say anything about GPIO? > I only ask because the header file above mentions gpio. > None yet, mips hopefully soon. Version 1 of this patch was part of a series adding support for the JZ4740. Since the jz4740 platform code provides the gpio functions I don't think it is necessary to add an additional depends on GENERIC_GPIO. >> + help >> + This selects the Ingenic Z4740 SD/Multimedia card Interface. >> + If you have an ngenic platform with a Multimedia Card slot, > > Ingenic ? > Woops, yes. >> + say Y or M here. >> + >> config MMC_SDHCI_OF >> tristate "SDHCI support on OpenFirmware platforms" >> depends on MMC_SDHCI && PPC_OF > > Thanks for reviewing - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 12 Jul 2010 18:50 On Tue, 2010-07-13 at 00:20 +0200, Lars-Peter Clausen wrote: > This patch adds support for the mmc controller on JZ4740 SoCs. > +static bool jz4740_mmc_write_data(struct jz4740_mmc_host *host, > + struct mmc_data *data) > +{ > + struct sg_mapping_iter *miter = &host->miter; > + uint32_t *buf; > + bool timeout; > + size_t i, j; > + > + while (sg_miter_next(miter)) { > + buf = miter->addr; > + i = miter->length / 4; > + j = i / 8; > + i = i & 0x7; > + while (j) { > + timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_TXFIFO_WR_REQ); > + if (unlikely(timeout)) > + goto poll_timeout; > + > + writel(buf[0], host->base + JZ_REG_MMC_TXFIFO); Perhaps it'd be better to use a temporary for host->base + JZ_REG_MMC_TXFIFO > + writel(buf[1], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[2], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[3], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[4], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[5], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[6], host->base + JZ_REG_MMC_TXFIFO); > + writel(buf[7], host->base + JZ_REG_MMC_TXFIFO); > + buf += 8; > + --j; > + } > + if (unlikely(i)) { > + timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_TXFIFO_WR_REQ); > + if (unlikely(timeout)) > + goto poll_timeout; > + > + while (i) { > + writel(*buf, host->base + JZ_REG_MMC_TXFIFO); > + ++buf; > + --i; > + } > + } > + data->bytes_xfered += miter->length; > + } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lars-Peter Clausen on 12 Jul 2010 19:50 Hi Joe Perches wrote: > On Tue, 2010-07-13 at 00:20 +0200, Lars-Peter Clausen wrote: >> This patch adds support for the mmc controller on JZ4740 SoCs. >> +static bool jz4740_mmc_write_data(struct jz4740_mmc_host *host, >> + struct mmc_data *data) >> +{ >> + struct sg_mapping_iter *miter = &host->miter; >> + uint32_t *buf; >> + bool timeout; >> + size_t i, j; >> + >> + while (sg_miter_next(miter)) { >> + buf = miter->addr; >> + i = miter->length / 4; >> + j = i / 8; >> + i = i & 0x7; >> + while (j) { >> + timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_TXFIFO_WR_REQ); >> + if (unlikely(timeout)) >> + goto poll_timeout; >> + >> + writel(buf[0], host->base + JZ_REG_MMC_TXFIFO); > > Perhaps it'd be better to use a temporary for > host->base + JZ_REG_MMC_TXFIFO Hm, indeed host->addr is reloaded before each write. > >> + writel(buf[1], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[2], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[3], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[4], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[5], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[6], host->base + JZ_REG_MMC_TXFIFO); >> + writel(buf[7], host->base + JZ_REG_MMC_TXFIFO); >> + buf += 8; >> + --j; >> + } >> + if (unlikely(i)) { >> + timeout = jz4740_mmc_poll_irq(host, JZ_MMC_IRQ_TXFIFO_WR_REQ); >> + if (unlikely(timeout)) >> + goto poll_timeout; >> + >> + while (i) { >> + writel(*buf, host->base + JZ_REG_MMC_TXFIFO); >> + ++buf; >> + --i; >> + } >> + } >> + data->bytes_xfered += miter->length; >> + } > > Thanks for reviewing - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
First
|
Prev
|
Next
|
Last
Pages: 1 2 3 Prev: PROBLEM: SIS7019 stops recording after 42 min Next: [PATCH v3] alsa: ASoC: Add JZ4740 codec driver |