2008-07-24 12:20:08

by Haavard Skinnemoen

[permalink] [raw]
Subject: [PATCH v3 1/3] mmc: Export internal host state through debugfs

When CONFIG_DEBUG_FS is set, create a few files under /sys/kernel/debug
containing information about an mmc host's internal state. Currently,
just a single file is created, "ios", which contains information about
the current operating parameters for the bus (clock speed, bus width,
etc.)

Host drivers can add additional files and directories under the host's
root directory by passing the debugfs_root field in struct mmc_host as
the 'parent' parameter to debugfs_create_*.

Signed-off-by: Haavard Skinnemoen <[email protected]>

Changes since v2:
* Don't rely on the compiler to optimize out unused code

Changes since v1:
* Fold with the ios patch
* Remove MMC_DEBUG_FS config option and inline dummy functions
* Use debugfs_remove_recursive to clean up. This allows us to remove
lots of struct dentry fields from struct mmc_host and struct
mmc_ios.
* Move debugfs functionality into a separate file, debugfs.c
* Convert "ios" directory to a single file containing all the
information.
---
drivers/mmc/core/Makefile | 1 +
drivers/mmc/core/core.h | 4 +
drivers/mmc/core/debugfs.c | 164 ++++++++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/host.c | 8 ++
include/linux/mmc/host.h | 2 +
5 files changed, 179 insertions(+), 0 deletions(-)
create mode 100644 drivers/mmc/core/debugfs.c

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 19a1a25..889e5f8 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -12,3 +12,4 @@ mmc_core-y := core.o bus.o host.o \
sdio.o sdio_ops.o sdio_bus.o \
sdio_cis.o sdio_io.o sdio_irq.o

+mmc_core-$(CONFIG_DEBUG_FS) += debugfs.o
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index cdb332b..745da98 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -52,5 +52,9 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr);

extern int use_spi_crc;

+/* Debugfs information for hosts and cards */
+void mmc_add_host_debugfs(struct mmc_host *host);
+void mmc_remove_host_debugfs(struct mmc_host *host);
+
#endif

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
new file mode 100644
index 0000000..133c6e5
--- /dev/null
+++ b/drivers/mmc/core/debugfs.c
@@ -0,0 +1,164 @@
+/*
+ * Debugfs support for hosts and cards
+ *
+ * Copyright (C) 2008 Atmel Corporation
+ *
+ * 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/debugfs.h>
+#include <linux/fs.h>
+#include <linux/seq_file.h>
+#include <linux/stat.h>
+
+#include <linux/mmc/host.h>
+
+#include "core.h"
+
+/* The debugfs functions are optimized away when CONFIG_DEBUG_FS isn't set. */
+static int mmc_ios_show(struct seq_file *s, void *data)
+{
+ static const char *vdd_str[] = {
+ [8] = "2.0",
+ [9] = "2.1",
+ [10] = "2.2",
+ [11] = "2.3",
+ [12] = "2.4",
+ [13] = "2.5",
+ [14] = "2.6",
+ [15] = "2.7",
+ [16] = "2.8",
+ [17] = "2.9",
+ [18] = "3.0",
+ [19] = "3.1",
+ [20] = "3.2",
+ [21] = "3.3",
+ [22] = "3.4",
+ [23] = "3.5",
+ [24] = "3.6",
+ };
+ struct mmc_host *host = s->private;
+ struct mmc_ios *ios = &host->ios;
+ const char *str;
+
+ seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
+ seq_printf(s, "vdd:\t\t%u ", ios->vdd);
+ if ((1 << ios->vdd) & MMC_VDD_165_195)
+ seq_printf(s, "(1.65 - 1.95 V)\n");
+ else if (ios->vdd < (ARRAY_SIZE(vdd_str) - 1)
+ && vdd_str[ios->vdd] && vdd_str[ios->vdd + 1])
+ seq_printf(s, "(%s ~ %s V)\n", vdd_str[ios->vdd],
+ vdd_str[ios->vdd + 1]);
+ else
+ seq_printf(s, "(invalid)\n");
+
+ switch (ios->bus_mode) {
+ case MMC_BUSMODE_OPENDRAIN:
+ str = "open drain";
+ break;
+ case MMC_BUSMODE_PUSHPULL:
+ str = "push-pull";
+ break;
+ default:
+ str = "invalid";
+ break;
+ }
+ seq_printf(s, "bus mode:\t%u (%s)\n", ios->bus_mode, str);
+
+ switch (ios->chip_select) {
+ case MMC_CS_DONTCARE:
+ str = "don't care";
+ break;
+ case MMC_CS_HIGH:
+ str = "active high";
+ break;
+ case MMC_CS_LOW:
+ str = "active low";
+ break;
+ default:
+ str = "invalid";
+ break;
+ }
+ seq_printf(s, "chip select:\t%u (%s)\n", ios->chip_select, str);
+
+ switch (ios->power_mode) {
+ case MMC_POWER_OFF:
+ str = "off";
+ break;
+ case MMC_POWER_UP:
+ str = "up";
+ break;
+ case MMC_POWER_ON:
+ str = "on";
+ break;
+ default:
+ str = "invalid";
+ break;
+ }
+ seq_printf(s, "power mode:\t%u (%s)\n", ios->power_mode, str);
+ seq_printf(s, "bus width:\t%u (%u bits)\n",
+ ios->bus_width, 1 << ios->bus_width);
+
+ switch (ios->timing) {
+ case MMC_TIMING_LEGACY:
+ str = "legacy";
+ break;
+ case MMC_TIMING_MMC_HS:
+ str = "mmc high-speed";
+ break;
+ case MMC_TIMING_SD_HS:
+ str = "sd high-speed";
+ break;
+ default:
+ str = "invalid";
+ break;
+ }
+ seq_printf(s, "timing spec:\t%u (%s)\n", ios->timing, str);
+
+ return 0;
+}
+
+static int mmc_ios_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, mmc_ios_show, inode->i_private);
+}
+
+static const struct file_operations mmc_ios_fops = {
+ .open = mmc_ios_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+void mmc_add_host_debugfs(struct mmc_host *host)
+{
+ struct dentry *root;
+
+ root = debugfs_create_dir(mmc_hostname(host), NULL);
+ if (IS_ERR(root))
+ /* Don't complain -- debugfs just isn't enabled */
+ return;
+ if (!root)
+ /* Complain -- debugfs is enabled, but it failed to
+ * create the directory. */
+ goto err_root;
+
+ host->debugfs_root = root;
+
+ if (!debugfs_create_file("ios", S_IRUSR, root, host, &mmc_ios_fops))
+ goto err_ios;
+
+ return;
+
+err_ios:
+ debugfs_remove_recursive(root);
+ host->debugfs_root = NULL;
+err_root:
+ dev_err(&host->class_dev, "failed to initialize debugfs\n");
+}
+
+void mmc_remove_host_debugfs(struct mmc_host *host)
+{
+ debugfs_remove_recursive(host->debugfs_root);
+}
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 1d795c5..6da80fd 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -127,6 +127,10 @@ int mmc_add_host(struct mmc_host *host)
if (err)
return err;

+#ifdef CONFIG_DEBUG_FS
+ mmc_add_host_debugfs(host);
+#endif
+
mmc_start_host(host);

return 0;
@@ -146,6 +150,10 @@ void mmc_remove_host(struct mmc_host *host)
{
mmc_stop_host(host);

+#ifdef CONFIG_DEBUG_FS
+ mmc_remove_host_debugfs(host);
+#endif
+
device_del(&host->class_dev);

led_trigger_unregister_simple(host->led);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 10a2080..9c288c9 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -157,6 +157,8 @@ struct mmc_host {
struct led_trigger *led; /* activity led */
#endif

+ struct dentry *debugfs_root;
+
unsigned long private[0] ____cacheline_aligned;
};

--
1.5.6.2


2008-07-24 12:20:32

by Haavard Skinnemoen

[permalink] [raw]
Subject: [PATCH v3 2/3] mmc: Add per-card debugfs support

For each card successfully added to the bus, create a subdirectory under
the host's debugfs root with information about the card.

At the moment, only a single file is added to the card directory for
all cards: "state". It reflects the "state" field in struct mmc_card,
indicating whether the card is present, readonly, etc.

For MMC and SD cards (not SDIO), another file is added: "status".
Reading this file will ask the card about its current status and
return it. This can be useful if the card just refuses to respond to
any commands, which might indicate that the card state is not what the
MMC core thinks it is (due to a missing stop command, for example.)

Signed-off-by: Haavard Skinnemoen <[email protected]>

Changes since v2:
* Don't rely on the compiler to optimize out unused code

Changes since v1:
* move card debugfs stuff into debugfs.c
* add "state" file corresponding to the "state" field in struct
mmc_card.
* only create the "status" file if the card is an MMC or SD memory
card since SDIO doesn't support the SEND_STATUS command.
---
drivers/mmc/core/bus.c | 8 ++++++
drivers/mmc/core/core.h | 3 ++
drivers/mmc/core/debugfs.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/card.h | 2 +
4 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index fd95b18..0d9b2d6 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -252,6 +252,10 @@ int mmc_add_card(struct mmc_card *card)
if (ret)
return ret;

+#ifdef CONFIG_DEBUG_FS
+ mmc_add_card_debugfs(card);
+#endif
+
mmc_card_set_present(card);

return 0;
@@ -263,6 +267,10 @@ int mmc_add_card(struct mmc_card *card)
*/
void mmc_remove_card(struct mmc_card *card)
{
+#ifdef CONFIG_DEBUG_FS
+ mmc_remove_card_debugfs(card);
+#endif
+
if (mmc_card_present(card)) {
if (mmc_host_is_spi(card->host)) {
printk(KERN_INFO "%s: SPI card removed\n",
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 745da98..c819eff 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -56,5 +56,8 @@ extern int use_spi_crc;
void mmc_add_host_debugfs(struct mmc_host *host);
void mmc_remove_host_debugfs(struct mmc_host *host);

+void mmc_add_card_debugfs(struct mmc_card *card);
+void mmc_remove_card_debugfs(struct mmc_card *card);
+
#endif

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 133c6e5..1237bb4 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -12,9 +12,11 @@
#include <linux/seq_file.h>
#include <linux/stat.h>

+#include <linux/mmc/card.h>
#include <linux/mmc/host.h>

#include "core.h"
+#include "mmc_ops.h"

/* The debugfs functions are optimized away when CONFIG_DEBUG_FS isn't set. */
static int mmc_ios_show(struct seq_file *s, void *data)
@@ -162,3 +164,62 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
{
debugfs_remove_recursive(host->debugfs_root);
}
+
+static int mmc_dbg_card_status_get(void *data, u64 *val)
+{
+ struct mmc_card *card = data;
+ u32 status;
+ int ret;
+
+ mmc_claim_host(card->host);
+
+ ret = mmc_send_status(data, &status);
+ if (!ret)
+ *val = status;
+
+ mmc_release_host(card->host);
+
+ return ret;
+}
+DEFINE_SIMPLE_ATTRIBUTE(mmc_dbg_card_status_fops, mmc_dbg_card_status_get,
+ NULL, "%08llx\n");
+
+void mmc_add_card_debugfs(struct mmc_card *card)
+{
+ struct mmc_host *host = card->host;
+ struct dentry *root;
+
+ if (!host->debugfs_root)
+ return;
+
+ root = debugfs_create_dir(mmc_card_id(card), host->debugfs_root);
+ if (IS_ERR(root))
+ /* Don't complain -- debugfs just isn't enabled */
+ return;
+ if (!root)
+ /* Complain -- debugfs is enabled, but it failed to
+ * create the directory. */
+ goto err;
+
+ card->debugfs_root = root;
+
+ if (!debugfs_create_x32("state", S_IRUSR, root, &card->state))
+ goto err;
+
+ if (mmc_card_mmc(card) || mmc_card_sd(card))
+ if (!debugfs_create_file("status", S_IRUSR, root, card,
+ &mmc_dbg_card_status_fops))
+ goto err;
+
+ return;
+
+err:
+ debugfs_remove_recursive(root);
+ card->debugfs_root = NULL;
+ dev_err(&card->dev, "failed to initialize debugfs\n");
+}
+
+void mmc_remove_card_debugfs(struct mmc_card *card)
+{
+ debugfs_remove_recursive(card->debugfs_root);
+}
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 0d508ac..ee6e822 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -111,6 +111,8 @@ struct mmc_card {
unsigned num_info; /* number of info strings */
const char **info; /* info strings */
struct sdio_func_tuple *tuples; /* unknown common tuples */
+
+ struct dentry *debugfs_root;
};

#define mmc_card_mmc(c) ((c)->type == MMC_TYPE_MMC)
--
1.5.6.2

2008-07-24 12:20:48

by Haavard Skinnemoen

[permalink] [raw]
Subject: [PATCH v3 3/3] atmel-mci: debugfs support

Create additional files under the host's debugfs directory containing
additional host-specific debug information.

Signed-off-by: Haavard Skinnemoen <[email protected]>

Changes since atmel-mci v4:
* Use seq_file to simplify `req' file ops
* Make the output from the "regs" debugfs file more readable to humans
---
drivers/mmc/host/atmel-mci-regs.h | 2 +
drivers/mmc/host/atmel-mci.c | 189 +++++++++++++++++++++++++++++++++++++
2 files changed, 191 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci-regs.h b/drivers/mmc/host/atmel-mci-regs.h
index a9a5657..26bd80e 100644
--- a/drivers/mmc/host/atmel-mci-regs.h
+++ b/drivers/mmc/host/atmel-mci-regs.h
@@ -82,6 +82,8 @@
# define MCI_OVRE ( 1 << 30) /* RX Overrun Error */
# define MCI_UNRE ( 1 << 31) /* TX Underrun Error */

+#define MCI_REGS_SIZE 0x100
+
/* Register access macros */
#define mci_readl(port,reg) \
__raw_readl((port)->regs + MCI_##reg)
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index cce873c..b68381f 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -9,6 +9,7 @@
*/
#include <linux/blkdev.h>
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/init.h>
#include <linux/interrupt.h>
@@ -16,6 +17,8 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/scatterlist.h>
+#include <linux/seq_file.h>
+#include <linux/stat.h>

#include <linux/mmc/host.h>

@@ -88,6 +91,188 @@ struct atmel_mci {
#define atmci_clear_pending(host, event) \
clear_bit(event, &host->pending_events)

+/*
+ * The debugfs stuff below is mostly optimized away when
+ * CONFIG_DEBUG_FS is not set.
+ */
+static int atmci_req_show(struct seq_file *s, void *v)
+{
+ struct atmel_mci *host = s->private;
+ struct mmc_request *mrq = host->mrq;
+ struct mmc_command *cmd;
+ struct mmc_command *stop;
+ struct mmc_data *data;
+
+ /* Make sure we get a consistent snapshot */
+ spin_lock_irq(&host->mmc->lock);
+
+ if (mrq) {
+ cmd = mrq->cmd;
+ data = mrq->data;
+ stop = mrq->stop;
+
+ if (cmd)
+ seq_printf(s,
+ "CMD%u(0x%x) flg %x rsp %x %x %x %x err %d\n",
+ cmd->opcode, cmd->arg, cmd->flags,
+ cmd->resp[0], cmd->resp[1], cmd->resp[2],
+ cmd->resp[2], cmd->error);
+ if (data)
+ seq_printf(s, "DATA %u / %u * %u flg %x err %d\n",
+ data->bytes_xfered, data->blocks,
+ data->blksz, data->flags, data->error);
+ if (stop)
+ seq_printf(s,
+ "CMD%u(0x%x) flg %x rsp %x %x %x %x err %d\n",
+ stop->opcode, stop->arg, stop->flags,
+ stop->resp[0], stop->resp[1], stop->resp[2],
+ stop->resp[2], stop->error);
+ }
+
+ spin_unlock_irq(&host->mmc->lock);
+
+ return 0;
+}
+
+static int atmci_req_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, atmci_req_show, inode->i_private);
+}
+
+static const struct file_operations atmci_req_fops = {
+ .owner = THIS_MODULE,
+ .open = atmci_req_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static void atmci_show_status_reg(struct seq_file *s,
+ const char *regname, u32 value)
+{
+ static const char *sr_bit[] = {
+ [0] = "CMDRDY",
+ [1] = "RXRDY",
+ [2] = "TXRDY",
+ [3] = "BLKE",
+ [4] = "DTIP",
+ [5] = "NOTBUSY",
+ [8] = "SDIOIRQA",
+ [9] = "SDIOIRQB",
+ [16] = "RINDE",
+ [17] = "RDIRE",
+ [18] = "RCRCE",
+ [19] = "RENDE",
+ [20] = "RTOE",
+ [21] = "DCRCE",
+ [22] = "DTOE",
+ [30] = "OVRE",
+ [31] = "UNRE",
+ };
+ unsigned int i;
+
+ seq_printf(s, "%s:\t0x%08x", regname, value);
+ for (i = 0; i < ARRAY_SIZE(sr_bit); i++) {
+ if (value & (1 << i)) {
+ if (sr_bit[i])
+ seq_printf(s, " %s", sr_bit[i]);
+ else
+ seq_puts(s, " UNKNOWN");
+ }
+ }
+ seq_putc(s, '\n');
+}
+
+static int atmci_regs_show(struct seq_file *s, void *v)
+{
+ struct atmel_mci *host = s->private;
+ u32 *buf;
+
+ buf = kmalloc(MCI_REGS_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* Grab a more or less consistent snapshot */
+ spin_lock_irq(&host->mmc->lock);
+ memcpy_fromio(buf, host->regs, MCI_REGS_SIZE);
+ spin_unlock_irq(&host->mmc->lock);
+
+ seq_printf(s, "MR:\t0x%08x%s%s CLKDIV=%u\n",
+ buf[MCI_MR / 4],
+ buf[MCI_MR / 4] & MCI_MR_RDPROOF ? " RDPROOF" : "",
+ buf[MCI_MR / 4] & MCI_MR_WRPROOF ? " WRPROOF" : "",
+ buf[MCI_MR / 4] & 0xff);
+ seq_printf(s, "DTOR:\t0x%08x\n", buf[MCI_DTOR / 4]);
+ seq_printf(s, "SDCR:\t0x%08x\n", buf[MCI_SDCR / 4]);
+ seq_printf(s, "ARGR:\t0x%08x\n", buf[MCI_ARGR / 4]);
+ seq_printf(s, "BLKR:\t0x%08x BCNT=%u BLKLEN=%u\n",
+ buf[MCI_BLKR / 4],
+ buf[MCI_BLKR / 4] & 0xffff,
+ (buf[MCI_BLKR / 4] >> 16) & 0xffff);
+
+ /* Don't read RSPR and RDR; it will consume the data there */
+
+ atmci_show_status_reg(s, "SR", buf[MCI_SR / 4]);
+ atmci_show_status_reg(s, "IMR", buf[MCI_IMR / 4]);
+
+ return 0;
+}
+
+static int atmci_regs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, atmci_regs_show, inode->i_private);
+}
+
+static const struct file_operations atmci_regs_fops = {
+ .owner = THIS_MODULE,
+ .open = atmci_regs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static void atmci_init_debugfs(struct atmel_mci *host)
+{
+ struct mmc_host *mmc;
+ struct dentry *root;
+ struct dentry *node;
+ struct resource *res;
+
+ mmc = host->mmc;
+ root = mmc->debugfs_root;
+ if (!root)
+ return;
+
+ node = debugfs_create_file("regs", S_IRUSR, root, host,
+ &atmci_regs_fops);
+ if (IS_ERR(node))
+ return;
+ if (!node)
+ goto err;
+
+ res = platform_get_resource(host->pdev, IORESOURCE_MEM, 0);
+ node->d_inode->i_size = res->end - res->start + 1;
+
+ node = debugfs_create_file("req", S_IRUSR, root, host, &atmci_req_fops);
+ if (!node)
+ goto err;
+
+ node = debugfs_create_x32("pending_events", S_IRUSR, root,
+ (u32 *)&host->pending_events);
+ if (!node)
+ goto err;
+
+ node = debugfs_create_x32("completed_events", S_IRUSR, root,
+ (u32 *)&host->completed_events);
+ if (!node)
+ goto err;
+
+ return;
+
+err:
+ dev_err(&host->pdev->dev,
+ "failed to initialize debugfs for controller\n");
+}

static void atmci_enable(struct atmel_mci *host)
{
@@ -905,6 +1090,8 @@ static int __init atmci_probe(struct platform_device *pdev)
"Atmel MCI controller at 0x%08lx irq %d\n",
host->mapbase, irq);

+ atmci_init_debugfs(host);
+
return 0;

err_request_irq:
@@ -923,6 +1110,8 @@ static int __exit atmci_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);

if (host) {
+ /* Debugfs stuff is cleaned up by mmc core */
+
if (host->detect_pin >= 0) {
int pin = host->detect_pin;

--
1.5.6.2

2008-07-25 08:24:48

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] mmc: Add per-card debugfs support

On Thu, Jul 24, 2008 at 02:18:58PM +0200, Haavard Skinnemoen wrote:
> For each card successfully added to the bus, create a subdirectory under
> the host's debugfs root with information about the card.
>
> At the moment, only a single file is added to the card directory for
> all cards: "state". It reflects the "state" field in struct mmc_card,
> indicating whether the card is present, readonly, etc.
>
> For MMC and SD cards (not SDIO), another file is added: "status".
> Reading this file will ask the card about its current status and
> return it. This can be useful if the card just refuses to respond to
> any commands, which might indicate that the card state is not what the
> MMC core thinks it is (due to a missing stop command, for example.)
>
> Signed-off-by: Haavard Skinnemoen <[email protected]>

out of interest, why not have an standard sysfs node for the
current voltage setting?

> Changes since v2:
> * Don't rely on the compiler to optimize out unused code

which compiler? the gcc 4 series seem quite good at it, gcc 3.4
and later tended to eliminate only the code and not the associated
data created with it.

> Changes since v1:
> * move card debugfs stuff into debugfs.c
> * add "state" file corresponding to the "state" field in struct
> mmc_card.
> * only create the "status" file if the card is an MMC or SD memory
> card since SDIO doesn't support the SEND_STATUS command.
> ---
> drivers/mmc/core/bus.c | 8 ++++++
> drivers/mmc/core/core.h | 3 ++
> drivers/mmc/core/debugfs.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mmc/card.h | 2 +
> 4 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index fd95b18..0d9b2d6 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -252,6 +252,10 @@ int mmc_add_card(struct mmc_card *card)
> if (ret)
> return ret;
>
> +#ifdef CONFIG_DEBUG_FS
> + mmc_add_card_debugfs(card);
> +#endif
> +

why not make mmc_add_card_debugfs() an empty function
in the header if there is no debugfs support?

> mmc_card_set_present(card);
>
> return 0;
> @@ -263,6 +267,10 @@ int mmc_add_card(struct mmc_card *card)
> */
> void mmc_remove_card(struct mmc_card *card)
> {
> +#ifdef CONFIG_DEBUG_FS
> + mmc_remove_card_debugfs(card);
> +#endif
> +

ditto above comment.

> if (mmc_card_present(card)) {
> if (mmc_host_is_spi(card->host)) {
> printk(KERN_INFO "%s: SPI card removed\n",
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 745da98..c819eff 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -56,5 +56,8 @@ extern int use_spi_crc;
> void mmc_add_host_debugfs(struct mmc_host *host);
> void mmc_remove_host_debugfs(struct mmc_host *host);
>
#ifdef CONFIG_DEBUG_FS
void mmc_add_card_debugfs(struct mmc_card *card);
void mmc_remove_card_debugfs(struct mmc_card *card);
#else
static inline void mmc_add_card_debugfs(struct mmc_card *card) { }
static inline void mmc_remove_card_debugfs(struct mmc_card *card) { }

> #endif
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 133c6e5..1237bb4 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -12,9 +12,11 @@
> #include <linux/seq_file.h>
> #include <linux/stat.h>
>
> +#include <linux/mmc/card.h>
> #include <linux/mmc/host.h>
>
> #include "core.h"
> +#include "mmc_ops.h"
>
> /* The debugfs functions are optimized away when CONFIG_DEBUG_FS isn't set. */
> static int mmc_ios_show(struct seq_file *s, void *data)
> @@ -162,3 +164,62 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
> {
> debugfs_remove_recursive(host->debugfs_root);
> }
> +
> +static int mmc_dbg_card_status_get(void *data, u64 *val)
> +{
> + struct mmc_card *card = data;
> + u32 status;
> + int ret;
> +
> + mmc_claim_host(card->host);
> +
> + ret = mmc_send_status(data, &status);
> + if (!ret)
> + *val = status;
> +
> + mmc_release_host(card->host);
> +
> + return ret;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(mmc_dbg_card_status_fops, mmc_dbg_card_status_get,
> + NULL, "%08llx\n");
> +
> +void mmc_add_card_debugfs(struct mmc_card *card)
> +{
> + struct mmc_host *host = card->host;
> + struct dentry *root;
> +
> + if (!host->debugfs_root)
> + return;
> +
> + root = debugfs_create_dir(mmc_card_id(card), host->debugfs_root);
> + if (IS_ERR(root))
> + /* Don't complain -- debugfs just isn't enabled */
> + return;
> + if (!root)
> + /* Complain -- debugfs is enabled, but it failed to
> + * create the directory. */
> + goto err;
> +
> + card->debugfs_root = root;
> +
> + if (!debugfs_create_x32("state", S_IRUSR, root, &card->state))
> + goto err;
> +
> + if (mmc_card_mmc(card) || mmc_card_sd(card))
> + if (!debugfs_create_file("status", S_IRUSR, root, card,
> + &mmc_dbg_card_status_fops))
> + goto err;
> +
> + return;
> +
> +err:
> + debugfs_remove_recursive(root);
> + card->debugfs_root = NULL;
> + dev_err(&card->dev, "failed to initialize debugfs\n");
> +}
> +
> +void mmc_remove_card_debugfs(struct mmc_card *card)
> +{
> + debugfs_remove_recursive(card->debugfs_root);
> +}
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 0d508ac..ee6e822 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -111,6 +111,8 @@ struct mmc_card {
> unsigned num_info; /* number of info strings */
> const char **info; /* info strings */
> struct sdio_func_tuple *tuples; /* unknown common tuples */
> +
> + struct dentry *debugfs_root;
> };
>
> #define mmc_card_mmc(c) ((c)->type == MMC_TYPE_MMC)
> --
> 1.5.6.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2008-07-25 09:05:54

by Haavard Skinnemoen

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] mmc: Add per-card debugfs support

On Fri, 25 Jul 2008 09:24:24 +0100
Ben Dooks <[email protected]> wrote:

> out of interest, why not have an standard sysfs node for the
> current voltage setting?

Dunno. Because nobody has added it? ;-)

> > Changes since v2:
> > * Don't rely on the compiler to optimize out unused code
>
> which compiler? the gcc 4 series seem quite good at it, gcc 3.4
> and later tended to eliminate only the code and not the associated
> data created with it.

4.2.x. It got rid of the code, the data associated with the code, but
not the code associated with that data again (i.e. the file operations
hooks).

> > +#ifdef CONFIG_DEBUG_FS
> > + mmc_add_card_debugfs(card);
> > +#endif
> > +
>
> why not make mmc_add_card_debugfs() an empty function
> in the header if there is no debugfs support?

Because Pierre didn't like it.

Haavard

2008-07-25 10:05:29

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] mmc: Add per-card debugfs support

On Fri, 25 Jul 2008 11:05:03 +0200
Haavard Skinnemoen <[email protected]> wrote:

> On Fri, 25 Jul 2008 09:24:24 +0100
> Ben Dooks <[email protected]> wrote:
>
> > out of interest, why not have an standard sysfs node for the
> > current voltage setting?
>
> Dunno. Because nobody has added it? ;-)
>

It is also a stable userspace ABI at that point, so it needs to be done
a lot more carefully.

> >
> > why not make mmc_add_card_debugfs() an empty function
> > in the header if there is no debugfs support?
>
> Because Pierre didn't like it.
>

If it's just in core.h, then I can live with it. I did not like it when
it added even more noise to the core .c files.

Rgds
--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.


Attachments:
signature.asc (197.00 B)

2008-07-26 23:27:01

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] mmc: Export internal host state through debugfs

On Thu, 24 Jul 2008 14:18:57 +0200
Haavard Skinnemoen <[email protected]> wrote:

> When CONFIG_DEBUG_FS is set, create a few files under /sys/kernel/debug
> containing information about an mmc host's internal state. Currently,
> just a single file is created, "ios", which contains information about
> the current operating parameters for the bus (clock speed, bus width,
> etc.)
>
> Host drivers can add additional files and directories under the host's
> root directory by passing the debugfs_root field in struct mmc_host as
> the 'parent' parameter to debugfs_create_*.
>
> Signed-off-by: Haavard Skinnemoen <[email protected]>
>

All three patches applied.

--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.


Attachments:
signature.asc (197.00 B)