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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090609224145.GA11001@yamamaya.is-a-geek.org>
Date:	Wed, 10 Jun 2009 00:41:45 +0200
From:	Tobias Diedrich <ranma+kernel@...edrich.de>
To:	Pierre Ossman <pierre@...man.eu>
Cc:	SDHCI development <sdhci-devel@...ts.ossman.eu>,
	linux-kernel@...r.kernel.org
Subject: QUIRK_FORCE_HISPD (was: Re: Ricoh R5C822 and QUIRK_FORCE_DMA)

Pierre Ossman wrote:
> Might be a system problem so it's Lenovo that disabled it. You're the
> first report I've seen on this since the DMA logic was reworked so that
> DMA didn't have to be forced for most cases.

FWIW, so far I haven't had any errors due to dma being enabled.

I tried changing the CAPS using setpci, but they seem to be
read-only (unless theres are write-protect bit somewhere in the
config space).

On a related note, I had a little time to play a bit more with my
R5C822 and found that I can also force-enable HISPD mode, which
boost performance further.

And the kernel controlled LED toggling does not work at all.
(SDHCI_USE_LEDS_CLASS).

I had a look at the HOST_CONTROL register and found that while the
HISPD bit can be toggled, I can't toggle the LED bit.

With the patched sdhci I now get 14 MB/s reading a hispeed card:


USB:    Reading: 19.8 MB/s  Writing: 13.8 MB/s
patched
SDHCI:  Reading: 14.5 MB/s  Writing:  3.5 MB/s
unpatched
SDHCI:  Reading:  3.4 MB/s  Writing:  6.6 MB/s

Interestingly PIO write seems to be faster than DMA write.
Also interesting:
With unpatched SDHCI (PIO), reading hogs the CPU and makes the system
very unresponsive (jumping X cursor, slow terminal refresh).
However writing is not as bad, about 25% system usage, still
responsive (and also faster than reads!?).

For some reason enabling HISPD did not improve write speed at all,
only the read speed doubled.

I tried improving the write speed by adding support for
APP_CMD_SET_WR_BLOCK_COUNT to the block driver, but did not see any
improvement.
It's still in the patch, in case you are interested.

dmesg, unpatched:
[ 5295.130010] sdhci: Secure Digital Host Controller Interface driver
[ 5295.130010] sdhci: Copyright(c) Pierre Ossman
[ 5295.140012] sdhci-pci 0000:04:00.1: SDHCI controller found [1180:0822] (rev 13)
[ 5295.140012] sdhci-pci 0000:04:00.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
[ 5295.140012] Registered led device: mmc0::
[ 5295.140012] mmc0: SDHCI controller on PCI [0000:04:00.1] using PIO
[ 5295.380271] mmc0: new SDHC card at address b368
[ 5295.380271] mmcblk0: mmc0:b368 SDC   14.9 GiB 
[ 5295.380271]  mmcblk0: p1

dmesg, patched:
[ 5906.950010] sdhci: Secure Digital Host Controller Interface driver
[ 5906.950010] sdhci: Copyright(c) Pierre Ossman
[ 5906.960011] sdhci-pci 0000:04:00.1: SDHCI controller found [1180:0822] (rev 13)
[ 5906.960011] sdhci-pci 0000:04:00.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17
[ 5906.960011] mmc0: Controller vendor_ver=02 sdhci_ver=00.
[ 5906.960011] mmc0: Controller caps=018021a1.
[ 5906.960011] DMA forced on (host quirk)
[ 5906.960011] sdhci-pci 0000:04:00.1: Will use DMA mode even though HW doesn't fully claim to support it.
[ 5906.960011] HISPD forced on (host quirk)
[ 5906.960011] Verified that HOST_CONTROL bit SDHCI_CTRL_HISPD can be toggled
[ 5906.960011] Could not set SDHCI_CTRL_LED to 1!
[ 5906.960011] mmc0: SDHCI controller on PCI [0000:04:00.1] using DMA
[ 5907.211304] mmc0: new high speed SDHC card at address b368
[ 5907.211304] mmcblk0: mmc0:b368 SDC   14.9 GiB 
[ 5907.211304]  mmcblk0: p1



Index: linux-2.6.30-rc8/drivers/mmc/host/sdhci.c
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/host/sdhci.c	2009-06-09 23:11:43.329261011 +0200
+++ linux-2.6.30-rc8/drivers/mmc/host/sdhci.c	2009-06-09 23:12:11.489274926 +0200
@@ -1614,6 +1614,10 @@
 	sdhci_reset(host, SDHCI_RESET_ALL);
 
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+	printk(KERN_ERR "%s: Controller vendor_ver=%02x sdhci_ver=%02x.\n",
+	       mmc_hostname(mmc),
+	       (host->version & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT,
+	       (host->version & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT);
 	host->version = (host->version & SDHCI_SPEC_VER_MASK)
 				>> SDHCI_SPEC_VER_SHIFT;
 	if (host->version > SDHCI_SPEC_200) {
@@ -1623,11 +1627,14 @@
 	}
 
 	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+	printk(KERN_ERR "%s: Controller caps=%08x.\n",
+	       mmc_hostname(mmc), caps);
 
-	if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
+	if (host->quirks & SDHCI_QUIRK_FORCE_DMA) {
+		printk(KERN_WARNING "DMA forced on (host quirk)\n");
 		host->flags |= SDHCI_USE_DMA;
-	else if (!(caps & SDHCI_CAN_DO_DMA))
-		DBG("Controller doesn't have DMA capability\n");
+	} else if (!(caps & SDHCI_CAN_DO_DMA))
+		printk(KERN_WARNING "Controller doesn't have DMA capability\n");
 	else
 		host->flags |= SDHCI_USE_DMA;
 
@@ -1723,7 +1730,34 @@
 	mmc->f_max = host->max_clk;
 	mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ;
 
-	if (caps & SDHCI_CAN_DO_HISPD)
+
+	if (host->quirks & SDHCI_QUIRK_FORCE_HISPD) {
+		unsigned char ctrl;
+		int toggle_ok = 1;
+
+		printk(KERN_WARNING "HISPD forced on (host quirk)\n");
+
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		ctrl |= SDHCI_CTRL_HISPD;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		if (!(ctrl & SDHCI_CTRL_HISPD)) {
+			printk(KERN_WARNING "Could not set SDHCI_CTRL_HISPD to 1!\n");
+			toggle_ok = 0;
+		}
+		ctrl &= ~SDHCI_CTRL_HISPD;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		if (ctrl & SDHCI_CTRL_HISPD) {
+			printk(KERN_WARNING "Could not set SDHCI_CTRL_HISPD to 0!\n");
+			toggle_ok = 0;
+		}
+		if (toggle_ok) {
+			printk(KERN_WARNING "Verified that HOST_CONTROL bit SDHCI_CTRL_HISPD can be toggled\n");
+		}
+
+		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
+	} else if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
 
 	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
@@ -1818,16 +1852,41 @@
 #endif
 
 #ifdef SDHCI_USE_LEDS_CLASS
-	snprintf(host->led_name, sizeof(host->led_name),
-		"%s::", mmc_hostname(mmc));
-	host->led.name = host->led_name;
-	host->led.brightness = LED_OFF;
-	host->led.default_trigger = mmc_hostname(mmc);
-	host->led.brightness_set = sdhci_led_control;
-
-	ret = led_classdev_register(mmc_dev(mmc), &host->led);
-	if (ret)
-		goto reset;
+	{
+		unsigned char ctrl;
+		int toggle_ok = 1;
+
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		ctrl |= SDHCI_CTRL_LED;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		if (!(ctrl & SDHCI_CTRL_LED)) {
+			printk(KERN_WARNING "Could not set SDHCI_CTRL_LED to 1!\n");
+			toggle_ok = 0;
+		}
+		ctrl &= ~SDHCI_CTRL_LED;
+		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+		if (ctrl & SDHCI_CTRL_LED) {
+			printk(KERN_WARNING "Could not set SDHCI_CTRL_LED to 0!\n");
+			toggle_ok = 0;
+		}
+		if (toggle_ok) {
+			printk(KERN_WARNING "Verified that HOST_CONTROL bit SDHCI_CTRL_LED can be toggled\n");
+			snprintf(host->led_name, sizeof(host->led_name),
+				"%s::", mmc_hostname(mmc));
+			host->led.name = host->led_name;
+			host->led.brightness = LED_OFF;
+			host->led.default_trigger = mmc_hostname(mmc);
+			host->led.brightness_set = sdhci_led_control;
+
+			ret = led_classdev_register(mmc_dev(mmc), &host->led);
+			if (ret)
+				goto reset;
+		} else {
+			host->led.name = NULL;
+		}
+	}
 #endif
 
 	mmiowb();
@@ -1882,7 +1941,8 @@
 	mmc_remove_host(host->mmc);
 
 #ifdef SDHCI_USE_LEDS_CLASS
-	led_classdev_unregister(&host->led);
+	if (host->led.name) /* name is NULL if LED can not be controlled */
+		led_classdev_unregister(&host->led);
 #endif
 
 	if (!dead)
Index: linux-2.6.30-rc8/drivers/mmc/host/sdhci-pci.c
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/host/sdhci-pci.c	2009-06-09 23:11:43.349260831 +0200
+++ linux-2.6.30-rc8/drivers/mmc/host/sdhci-pci.c	2009-06-09 23:12:11.489274926 +0200
@@ -91,7 +91,9 @@
 
 static const struct sdhci_pci_fixes sdhci_ricoh = {
 	.probe		= ricoh_probe,
-	.quirks		= SDHCI_QUIRK_32BIT_DMA_ADDR,
+	.quirks		= SDHCI_QUIRK_32BIT_DMA_ADDR |
+			  SDHCI_QUIRK_FORCE_DMA |
+			  SDHCI_QUIRK_FORCE_HISPD,
 };
 
 static const struct sdhci_pci_fixes sdhci_ene_712 = {
Index: linux-2.6.30-rc8/drivers/mmc/host/sdhci.h
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/host/sdhci.h	2009-06-09 23:11:43.309261052 +0200
+++ linux-2.6.30-rc8/drivers/mmc/host/sdhci.h	2009-06-09 23:12:11.489274926 +0200
@@ -226,6 +226,8 @@
 #define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET		(1<<19)
 /* Controller has to be forced to use block size of 2048 bytes */
 #define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<20)
+/* Controller has bad caps bits, but really supports HISPD */
+#define SDHCI_QUIRK_FORCE_HISPD				(1<<21)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
Index: linux-2.6.30-rc8/drivers/mmc/card/block.c
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/card/block.c	2009-06-09 23:11:43.369261139 +0200
+++ linux-2.6.30-rc8/drivers/mmc/card/block.c	2009-06-09 23:12:11.499260622 +0200
@@ -40,6 +40,7 @@
 #include <asm/uaccess.h>
 
 #include "queue.h"
+#include "../core/sd_ops.h"
 
 MODULE_ALIAS("mmc:block");
 
@@ -262,6 +263,22 @@
 			brq.data.blocks = card->host->max_blk_count;
 
 		/*
+		 * Inform the card about the number of blocks to be written.
+		 */
+		if (card->type == MMC_TYPE_SD &&
+		    rq_data_dir(req) == WRITE) {
+			int err;
+
+			err = mmc_app_set_wr_blk_erase_cnt(card, brq.data.blocks);
+			if (err)
+				printk(KERN_WARNING
+				       "%s: mmc_app_set_wr_blk_erase_cnt "
+				       "failed: %d\n",
+				       req->rq_disk->disk_name,
+				       err);
+		}
+
+		/*
 		 * After a read error, we redo the request one sector at a time
 		 * in order to accurately determine which sectors can be read
 		 * successfully.
Index: linux-2.6.30-rc8/drivers/mmc/core/sd_ops.c
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/core/sd_ops.c	2009-06-09 23:11:43.409260079 +0200
+++ linux-2.6.30-rc8/drivers/mmc/core/sd_ops.c	2009-06-09 23:12:11.499260622 +0200
@@ -299,6 +299,28 @@
 	return 0;
 }
 
+int mmc_app_set_wr_blk_erase_cnt(struct mmc_card *card, int count)
+{
+	int err;
+	struct mmc_command cmd;
+
+	BUG_ON(!card);
+	BUG_ON(!card->host);
+
+	memset(&cmd, 0, sizeof(struct mmc_command));
+
+	cmd.opcode = SD_APP_SET_WR_BLK_ERASE_CNT;
+	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+	cmd.arg = count;
+
+	err = mmc_wait_for_app_cmd(card->host, card, &cmd, MMC_CMD_RETRIES);
+	if (err)
+		return err;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_app_set_wr_blk_erase_cnt);
+
 int mmc_sd_switch(struct mmc_card *card, int mode, int group,
 	u8 value, u8 *resp)
 {
Index: linux-2.6.30-rc8/drivers/mmc/core/sd_ops.h
===================================================================
--- linux-2.6.30-rc8.orig/drivers/mmc/core/sd_ops.h	2009-06-09 23:11:43.389260399 +0200
+++ linux-2.6.30-rc8/drivers/mmc/core/sd_ops.h	2009-06-09 23:12:11.499260622 +0200
@@ -13,6 +13,7 @@
 #define _MMC_SD_OPS_H
 
 int mmc_app_set_bus_width(struct mmc_card *card, int width);
+int mmc_app_set_wr_blk_erase_cnt(struct mmc_card *card, int count);
 int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
 int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
 int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
Index: linux-2.6.30-rc8/include/linux/mmc/sd.h
===================================================================
--- linux-2.6.30-rc8.orig/include/linux/mmc/sd.h	2009-06-09 23:11:43.439260121 +0200
+++ linux-2.6.30-rc8/include/linux/mmc/sd.h	2009-06-09 23:13:35.834952507 +0200
@@ -12,20 +12,21 @@
 #ifndef MMC_SD_H
 #define MMC_SD_H
 
-/* SD commands                           type  argument     response */
+/* SD commands                               type  argument     response */
   /* class 0 */
 /* This is basically the same command as for MMC with some quirks. */
-#define SD_SEND_RELATIVE_ADDR     3   /* bcr                     R6  */
-#define SD_SEND_IF_COND           8   /* bcr  [11:0] See below   R7  */
+#define SD_SEND_RELATIVE_ADDR        3    /* bcr                     R6  */
+#define SD_SEND_IF_COND              8    /* bcr  [11:0] See below   R7  */
 
   /* class 10 */
-#define SD_SWITCH                 6   /* adtc [31:0] See below   R1  */
+#define SD_SWITCH                    6    /* adtc [31:0] See below   R1  */
 
   /* Application commands */
-#define SD_APP_SET_BUS_WIDTH      6   /* ac   [1:0] bus width    R1  */
-#define SD_APP_SEND_NUM_WR_BLKS  22   /* adtc                    R1  */
-#define SD_APP_OP_COND           41   /* bcr  [31:0] OCR         R3  */
-#define SD_APP_SEND_SCR          51   /* adtc                    R1  */
+#define SD_APP_SET_BUS_WIDTH         6    /* ac   [1:0] bus width    R1  */
+#define SD_APP_SEND_NUM_WR_BLKS     22    /* adtc                    R1  */
+#define SD_APP_SET_WR_BLK_ERASE_CNT 23    /* adtc                    R1  */
+#define SD_APP_OP_COND              41    /* bcr  [31:0] OCR         R3  */
+#define SD_APP_SEND_SCR             51    /* adtc                    R1  */
 
 /*
  * SD_SWITCH argument format:

-- 
Tobias						PGP: http://9ac7e0bc.uguu.de
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ