| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20071124180023.20c831b8@poseidon.drzeus.cx>
Date: Sat, 24 Nov 2007 18:00:23 +0100
From: Pierre Ossman <drzeus-list@...eus.cx>
To: Haavard Skinnemoen <hskinnemoen@...el.com>
Cc: linux-kernel@...r.kernel.org,
Shannon Nelson <shannon.nelson@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
David Brownell <david-b@...bell.net>, kernel@...32linux.org,
linux-arm-kernel@...ts.arm.linux.org.uk,
Haavard Skinnemoen <hskinnemoen@...el.com>
Subject: Re: [RFC 4/4] Atmel MCI: Driver for Atmel on-chip MMC controllers
On Fri, 23 Nov 2007 13:20:13 +0100
Haavard Skinnemoen <hskinnemoen@...el.com> wrote:
> This is a driver for the MMC controller on the AP7000 chips from
> Atmel. It should in theory work on AT91 systems too with some
> tweaking, but since the DMA interface is quite different, it's not
> entirely clear if it's worth it.
>
> This driver has been around for a while in BSPs and kernel sources
> provided by Atmel, but this particular version uses the generic DMA
> Engine framework (with the slave extensions) instead of an
> avr32-only DMA controller framework.
>
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@...el.com>
Why didn't I get a cc? Don't you love me any more? :'(
> ---
> arch/avr32/boards/atngw100/setup.c | 6 +
> arch/avr32/boards/atstk1000/atstk1002.c | 3 +
> arch/avr32/mach-at32ap/at32ap7000.c | 31 +-
> drivers/mmc/host/Kconfig | 10 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/atmel-mci.c | 1170 +++++++++++++++++++++++++++++++
> drivers/mmc/host/atmel-mci.h | 192 +++++
> include/asm-avr32/arch-at32ap/board.h | 10 +-
> 8 files changed, 1417 insertions(+), 6 deletions(-)
> create mode 100644 drivers/mmc/host/atmel-mci.c
> create mode 100644 drivers/mmc/host/atmel-mci.h
>
Could you add a note to MAINTAINERS as well?
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5fef678..687cf8b 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -91,6 +91,16 @@ config MMC_AT91
>
> If unsure, say N.
>
> +config MMC_ATMELMCI
> + tristate "Atmel Multimedia Card Interface support"
> + depends on AVR32 && DMA_ENGINE
> + help
> + This selects the Atmel Multimedia Card Interface. If you have
> + a AT91 (ARM) or AT32 (AVR32) platform with a Multimedia Card
> + slot, say Y or M here.
> +
> + If unsure, say N.
> +
> config MMC_IMX
> tristate "Motorola i.MX Multimedia Card Interface support"
> depends on ARCH_IMX
Now this gets a bit confusing as we'll have two drivers for AT91. Any status report on merging these?
I can accept having two drivers (for a while at least), but the Kconfig help texts should explain the sordid details.
> +
> +/* Those printks take an awful lot of time... */
> +#ifndef DEBUG
> +static unsigned int fmax = 15000000U;
> +#else
> +static unsigned int fmax = 1000000U;
> +#endif
> +module_param(fmax, uint, 0444);
> +MODULE_PARM_DESC(fmax, "Max frequency in Hz of the MMC bus clock");
> +
Why is this needed and is it perhaps something that can be moved to the MMC core?
> +
> +static int req_dbg_open(struct inode *inode, struct file *file)
> +{
This also looks like something that can be made general.
> +
> + if (mmc->ios.bus_mode == MMC_BUSMODE_OPENDRAIN)
> + cmdr |= MCI_BIT(OPDCMD);
> +
> + dev_dbg(&mmc->class_dev,
> + "cmd: op %02x arg %08x flags %08x, cmdflags %08lx\n",
> + cmd->opcode, cmd->arg, cmd->flags, (unsigned long)cmdr);
> +
The debug output in the core should make this redundant.
> +
> +static void atmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
I seem to recall that atmci couldn't currently handle transfers that weren't a multiple of four. Could you please add a check for this and fail the request with -EINVAL when that happens?
> +
> +static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct atmel_mci *host = mmc_priv(mmc);
> + u32 mr;
> +
> + if (ios->clock) {
> + u32 clkdiv;
> +
> + /* Set clock rate */
> + clkdiv = host->bus_hz / (2 * ios->clock) - 1;
> + if (clkdiv > 255) {
> + dev_warn(&mmc->class_dev,
> + "clock %u too slow; using %lu\n",
> + ios->clock, host->bus_hz / (2 * 256));
> + clkdiv = 255;
> + }
> +
> + mr = mci_readl(host, MR);
> + mr = MCI_BFINS(CLKDIV, clkdiv, mr)
> + | MCI_BIT(WRPROOF) | MCI_BIT(RDPROOF);
> + mci_writel(host, MR, mr);
> +
> + /* Enable the MCI controller */
> + mci_writel(host, CR, MCI_BIT(MCIEN));
> + } else {
> + /* Disable the MCI controller */
> + mci_writel(host, CR, MCI_BIT(MCIDIS));
> + }
> +
I hope "disable" here doesn't power down the card, as that would be incorrect.
> +
> + if (status & MCI_BIT(DCRCE)) {
> + dev_dbg(&mmc->class_dev, "data CRC error\n");
> + data->error = -EILSEQ;
> + } else if (status & MCI_BIT(DTOE)) {
> + dev_dbg(&mmc->class_dev, "data timeout error\n");
> + data->error = -ETIMEDOUT;
> + } else {
> + dev_dbg(&mmc->class_dev, "data FIFO error\n");
> + data->error = -EIO;
> + }
> + dev_dbg(&mmc->class_dev, "bytes xfered: %u\n",
> + data->bytes_xfered);
> +
The debug output here is already provided by the MMC core.
> +
> +static int __exit atmci_remove(struct platform_device *pdev)
> +{
> + struct atmel_mci *host = platform_get_drvdata(pdev);
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + if (host) {
> + atmci_cleanup_debugfs(host);
> +
> + if (host->detect_pin >= 0) {
> + free_irq(gpio_to_irq(host->detect_pin), host->mmc);
> + cancel_delayed_work(&host->mmc->detect);
Not for driver poking. Hands off! :)
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists