On Sun, 7 Sep 2008 23:52:28 +0200

Hmm... I thought the CB710/720 only had an SDHCI interface for the



Requests are serialised, yes. But you could in theory get set_ios()
calls during an ongoing request. Doing so would be very undefined
though so it should be sufficient to just avoid crashing the entire

Not much really. The cmd->data pointer is for convenience as it allows

None at all. You have to specify your restrictions in the mmc_host
structure fields (note that you cannot restrict alignment in any way).

The requirements are generally low for merging. Mostly you just need to
promise to stick around and fix problems. You should be aware that the
style requirements and API adherence are very important. Run you stuff
through checkpatch.pl before submitting. Sparse can usually also find

As I said, to my knowledge the sdhci driver covers this hardware, but
the support is rather crappy as the controller is buggy as hell. And
ENE does indeed ignore all attempts at contact.

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.
--

[ message continues ]

SDHCI supports CB712/714 parts. I already investigated that CB710 is not

I checked yesterday that indeed nothing bad happens, but looking at
the code I can see that mmc_request_done can call ->request() with
a retried request so, unless gcc optimizes tail-calls, recursion


Is it safe to assume that cmd->data is NULL when command type is other
than ADTC? I've looked briefly through drivers/mmc/core/*_ops.c, but

I'll give it a try.

Thanks for your reply. I'll post the code for review when it does more
than just blinking LEDs. ;)

Best Regards,
Michał Mirosław

--

[ message continues ]

Hello,

Here's a rough version of a driver for CB710/720 memory card reader.
It was tested with Canon SDC-16M card and actually works... until
first command error as there's no good error recovery. Driver spits
out a lot of debugging output as it goes and currently when it starts
failing commands it's necessary to reload the module to get it
working again.

For the MMC part, TODO list includes at least:
 - split the driver to a bus-like common device driver and reader
   specific parts like tifm driver has
 - maybe get rid of sgbuf.c if there already is some similar API
   or more assumptions on scatterlist elements can be made

If you're brave enough to test this please do so, but I wouldn't
bet my important data on it. ;)

Best Regards,
Michał Mirosław

This, obviously, is not ready to include anywhere.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urN empty/cb710.h cb710-pre-20080911/cb710.h
--- empty/cb710.h	1970-01-01 01:00:00.000000000 +0100
+++ cb710-pre-20080911/cb710.h	2008-09-11 21:06:36.000000000 +0200
@@ -0,0 +1,158 @@
+/*
+ *  cb710/cb710.h
+ *
+ *  Copyleft by Michał Mirosław, 2008
+ *
+ * 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.
+ */
+#ifndef LINUX_CB710_DRIVER_H
+#define LINUX_CB710_DRIVER_H
+
+#include <linux/spinlock.h>
+#include <linux/mmc/host.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+
+#define CB710_DRIVER_NAME "cb710"
+
+/* per-device structure */
+struct cb710_chip {
+	struct pci_dev *pdev;
+	void __iomem *iobase;
+
+	spinlock_t irq_lock;
+	unsigned irq_enabled;
+
+	struct mmc_host *mmc;
+};
+
+#define RDPORT(t, p) \
+	ioread##t(chip->iobase + (p))
+#define WRPORT(t, p, v) \
+	do { iowrite##t((v), chip->iobase + (p)); \
+		(void)ioread8(chip->iobase + 0x13); } while (0)
+#define UPDPORT(t, p, v, m) \
+	do { ...
[ message continues ]

Hello again,

Here is next version of CB710 MMC-host driver. Since I don't like
copying code I used platform device "bus" instead of duplicating
tifm_core and modifying it to cb710 specifics. Debugging printk()s
were retained for now.

Please comment,
Michał Mirosław

diff -urN empty/cb710.h cb710-pre-20080913/cb710.h
--- empty/cb710.h	1970-01-01 01:00:00.000000000 +0100
+++ cb710-pre-20080913/cb710.h	2008-09-13 00:30:51.000000000 +0200
@@ -0,0 +1,186 @@
+/*
+ *  cb710/cb710.h
+ *
+ *  Copyleft by Michał Mirosław, 2008
+ *
+ * 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.
+ */
+#ifndef LINUX_CB710_DRIVER_H
+#define LINUX_CB710_DRIVER_H
+
+#include <linux/spinlock.h>
+#include <linux/mmc/host.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+
+#define CB710_DRIVER_NAME "cb710"
+
+struct cb710_slot {
+	struct platform_device pdev;
+	int (*irq_handler)(struct cb710_slot *);
+};
+
+/* per-device structure */
+struct cb710_chip {
+	struct pci_dev *pdev;
+	void __iomem *iobase;
+
+	spinlock_t irq_lock;
+	unsigned irq_enabled;
+	unsigned platform_id;
+
+	unsigned slots;
+	struct cb710_slot slot[0];
+};
+
+#define CB710_SLOT_MASK		7
+#define CB710_SLOT_MMC		1
+#define CB710_SLOT_MS		2
+#define CB710_SLOT_SM		4
+#define CB710_SLOT_NR_SHIFT	8
+
+#define RDPORT(t, p) \
+	ioread##t(chip->iobase + (p))
+#define WRPORT(t, p, v) \
+	do { iowrite##t((v), chip->iobase + (p)); \
+		(void)ioread8(chip->iobase + 0x13); } while (0)
+#define UPDPORT(t, p, v, m) \
+	do { \
+		iowrite##t( \
+			(ioread##t(chip->iobase + (p)) & ~(m)) | (v), \
+			chip->iobase + (p)); \
+		(void)ioread8(chip->iobase + 0x13); \
+	} while (0)
+#define S_RDPORT(t, p, b, c) \
+	ioread##t##_rep(chip->iobase + (p), (b), (c))
+#define S_WRPORT(t, p, b, c) \
+	do { ...
[ message continues ]

On Sat, 13 Sep 2008 01:43:02 +0200

You should probably stick with "Copyright" even though it is a free

This is a pretty bad case of obfuscation. Don't construct macros that
reference local variables (chip in this case). Also,
iowrite16_rep(chip->iobase + CB700_FOO, buffer, 12); is quite readable
as it is. Using standard kernel functions allows your code to be more

Again, this just obfuscates. If you keep the ~ in calling code it

Why this complex system? Can't you use the handlers the kernel already
provides? You also get a lot of special handling with those, e.g.

This thing is a bit big. It'll be more readable if you turn it into a

You shouldn't be using the interrupt versions inside the interrupt

If this still is unknown voodoo, then please add a comment explaining


It's easier if you develop against the real kernel tree and send diffs




You seem to know what some of the "what" values are. Put some defines

ACMD:s are no different from "normal" commands. As you've discovered,

Looking at the opcode is not acceptable, so you need to figure out
what's really going on here. MMC_SET_BLOCKLEN has a common R1 response,


This isn't terribly readable. Something like this would be better:

	if (cmd->flags & MMC_RSP_OPCODE) {
		if (rsp_opcode != cmd->opcode)
			cmd->error = -EILSEQ;
	} else {
		if (rsp_opcode != 0x3F)
			cmd->error = -EILSEQ;
	}


You can probably never do proper counting when you don't know how the

NULL is the default so you don't need to specify that.

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.
--

[ message continues ]

Hello,

Thanks for your comments, Pierre - I have applied most of your
suggestions and replied to some below.

Changes in this version:
 - power management functions;
 - IRQ handler modifications: core driver uses RCU, MMC-specific handler
   uses spinlock only in the uncommon case of 'test' interrupt;
 - debugging printks converted to dev_dbg() and friends;
 - code split to files such that MMC specific parts are self-contained

This should be almost ready. mmc-test passes up to a point where it
tries non-sector-size writes to the card - I didn't have time to look
into this further, yet.


Sure. This was an attempt at imitating Perl-ish dynamically scoped

I looked at linux/lib/scatterlist.c and found nothing really useful. If
there were some preconditions always met for scatterlists - like that block
does not span multiple pages, or at least 16-byte block don't, then most
of this code can go away. It looks complicated, because it implements
a special iterator interface that always returns multiple-of-16-byte blocks
for reading or writing. This makes the PIO loops very simple and fast

Hmm. For me it was easier to have the driver out-of-tree, as it's
rebuilding went faster and all files are at hand. I will of course
make a proper diff against vanilla kernel when it's ready to go in.
I've put a .tgz at my server to make it easier for those that wan't

Can you expand on this? Right now irq handler doesn't call
__cb710_mmc_enable_irq() but modifies registers itself. This will


That's a lot of lines. I used another variable to make it clearer.

Thanks again for your comments,
Michał Mirosław


diff -urN empty/cb710.h cb710/cb710.h
--- empty/cb710.h	1970-01-01 01:00:00.000000000 +0100
+++ cb710/cb710.h	2008-09-25 07:07:40.000000000 +0200
@@ -0,0 +1,131 @@
+/*
+ *  cb710/cb710.h
+ *
+ *  Copyright by Michał Mirosław, 2008
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 ...
[ message continues ]

On Thu, 25 Sep 2008 08:29:24 +0200

If you can't figure it out, just make sure the driver fails the

The buffers leave no alignment guarantees unfortunately. 

Have a look at the current sdhci.c PIO routines though. It uses the sg
iterator helpers and keeps track of four byte chunks (as opposed to the

Just require that the lock must be held by the caller when invoking
cb710_mmc_enable_irq(). It's usually easier to keep track of locks by
keeping the lock handling at entry points into the driver.

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.
--

[ message continues ]

Here is the driver for CB710 SD/MMC reader updated to recently released
kernel version 2.6.27. The code is divided into PCI device driver
(wrapped that registers three platform devices) and MMC reader driver.
This idea was taken from TIFM driver.

Changes from v2:
 - return EINVAL for unsupported data transfer sizes
 - use sg_mapping_iter for PIO from/to scatterlist
 - add slot release function and prevent kobject leak on device registration's
   error path

If this code looks reasonably good, then I'll post patches against the
real kernel tree. The file distribution plan is:

   cb710-mmc.h	-> drivers/mmc/host/cb710-mmc.h
   mmc.c	-> drivers/mmc/host/cb710-mmc.c
   sgbuf2.h	-> merge to include/linux/scatterlist.h
   sgbuf2.c	-> merge to lib/scatterlist.c
   cb710.h	-> include/linux/cb710.h
   core.c	-> drivers/misc/cb710-core.c
   debug.c	-> drivers/misc/cb710-debug.c



I got around it by introducing a small wrapper around sg_mapping_iter
functions that uses 32-bit blocks. This reduces to what is in SDHCI
driver's sdhci_*_block_pio() functions, but with optimized common case
where the buffers are properly aligned. That leaves a question what should

The chip->irq_lock (now in reader->irq_lock) protects only hardware
IRQ-enabling registers. I think there is no point in moving the lock
outside cb710_mmc_enable_irq().

Best Regards,
Michał Mirosław

diff -urN -x '.git*' empty/cb710.h cb710/cb710.h
--- empty/cb710.h	1970-01-01 01:00:00.000000000 +0100
+++ cb710/cb710.h	2008-10-29 12:49:55.000000000 +0100
@@ -0,0 +1,132 @@
+/*
+ *  cb710/cb710.h
+ *
+ *  Copyright by Michał Mirosław, 2008
+ *
+ * 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.
+ */
+#ifndef LINUX_CB710_DRIVER_H
+#define LINUX_CB710_DRIVER_H
+
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include ...
[ message continues ]

On Wed, 29 Oct 2008 15:11:47 +0100

It looks more or less good to go. I have some small notes, and you
probably need to poke some scatterlist folks to okay those changes

This is clearer than what you had before, but I still think you should


I couldn't find VERBOSE_DEBUG defined somewhere. For ease of use with


It's a matter of taste really. Many drivers have their defines in the
driver. Personally I think it is nice to have everything in pci_ids.h,

As this code is just useless, please clean it out before you submit the

Since you can't change the voltage, this seems wrong. My guess is that

Most drivers handle this by sticking their head in the sand, but the
proper way is probably to disable the card insertions interrupt before
mmc_remove_host().

(and there should be no other unsolicited interrupts)

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.
--

[ message continues ]

Hello again,

Sorry for a long delay with my reply.


There's no entry in MAINTAINERS list, but I assume it's Jens Axboe as he's

The really useful one here is cb710_modify_port_*(), read/write are for
consistency. I'd prefer to keep those wrappers because they help to eliminate

I thought this as the easiest way to assure that on return from
cb710_set_irq_handler() there's no interrupt handler running using the

This is mostly related to veryfying some assumptions on the MMC core
infrastructure. I can remove this altogether if the calls to driver

Hmm. I checked couple random drivers in kernel tree and none (including


That's what the original driver advertises. I can't verify the exact
voltage that is supplied to the card, so I'll assume that's 3.3V and

Will try.

Best Regards,
Michał Mirosław

--

[ message continues ]

On Sun, 1 Feb 2009 19:54:35 +0100


They are. Changing device settings during an ongoing request is very

Check sdhci.c, it frees the irq and re-requests it during suspend. This
is generally needed when you share interrupts as you need to avoid
trying to handle interrupts before your device has been resumed.

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.
--

[ message continues ]

Here is next version of the CB710 SD/MMC reader driver. As in previous
versions the code is divided in two parts - virtual 'bus' driver that
handles PCI device and registers three new devices one per card reader
type. The other device handles SD/MMC part of the reader.

Key changes from v3:
 - kerneldoc for sg_mapping_iterator extension
 - free/restore IRQ during suspend/resume
 - protect slot->irq_handler by spinlock instead of RCU
 - remove verify_serialization()
 - Kconfig
 - complete patch for linux kernel 2.6.29.2

My patches for sg_mapping_iterator extension went to /dev/null, so I have
incorporated them to the cb710 core driver module for now. Probably someone
with more karma points should look at them (sgbuf2.{c,h}) and push further
if they are of any value to other driver writers.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urN linux-2.6.29.2/drivers/misc/cb710/core.c cb710-tree/drivers/misc/cb710/core.c
--- linux-2.6.29.2/drivers/misc/cb710/core.c	1970-01-01 01:00:00.000000000 +0100
+++ cb710-tree/drivers/misc/cb710/core.c	2009-05-04 20:31:59.000000000 +0200
@@ -0,0 +1,356 @@
+/*
+ *  cb710/core.c
+ *
+ *  Copyright by Michał Mirosław, 2008-2009
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+#include <linux/idr.h>
+#include <linux/cb710.h>
+
+static DEFINE_IDR(cb710_idr);
+static DEFINE_SPINLOCK(cb710_idr_lock);
+
+void cb710_pci_update_config_reg(struct pci_dev *pdev,
+	int reg, uint32_t mask, uint32_t xor)
+{
+	u32 rval;
+
+	pci_read_config_dword(pdev, reg, &rval);
+	rval = (rval & mask) ^ xor;
+	pci_write_config_dword(pdev, reg, rval);
+}
+EXPORT_SYMBOL_GPL(cb710_pci_update_config_reg);
+
+/* Some magic writes based on Windows driver init ...
[ message continues ]

On Fri, 8 May 2009 17:16:22 +0200

Looks good to me. I just want you to add a MAINTAINERS entry so that

Give them to Andrew Morton. He's good at finding the right people and
making sure they notice. :)

Rgds
-- 
     -- Pierre Ossman

  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.
[ message continues ]

Here is a version of the CB710 SD/MMC reader driver ready for inclusion
upstream. The code is divided in two parts. There is a virtual 'bus' driver
that handles PCI device and registers three new devices one per card reader
type. The other driver handles SD/MMC part of the reader.

Changes from v4:
 - sg_mapping_iterator extension renamed to avoid namespace clashes
   when/if the extension would be merged upstream
 - appended sgbuf2.h to cb710.h
 - move from IDR to IDA for device ID generation
 - cleanup after IDA on module unload
 - added MAINTAINERS entry

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urpN linux-2.6.29.3/drivers/misc/cb710/core.c test2/drivers/misc/cb710/core.c
--- linux-2.6.29.3/drivers/misc/cb710/core.c	1970-01-01 01:00:00.000000000 +0100
+++ test2/drivers/misc/cb710/core.c	2009-05-22 19:47:19.000000000 +0200
@@ -0,0 +1,357 @@
+/*
+ *  cb710/core.c
+ *
+ *  Copyright by Michał Mirosław, 2008-2009
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+#include <linux/idr.h>
+#include <linux/cb710.h>
+
+static DEFINE_IDA(cb710_ida);
+static DEFINE_SPINLOCK(cb710_ida_lock);
+
+void cb710_pci_update_config_reg(struct pci_dev *pdev,
+	int reg, uint32_t mask, uint32_t xor)
+{
+	u32 rval;
+
+	pci_read_config_dword(pdev, reg, &rval);
+	rval = (rval & mask) ^ xor;
+	pci_write_config_dword(pdev, reg, rval);
+}
+EXPORT_SYMBOL_GPL(cb710_pci_update_config_reg);
+
+/* Some magic writes based on Windows driver init code */
+static int __devinit cb710_pci_configure(struct pci_dev *pdev)
+{
+	unsigned int devfn = PCI_DEVFN(PCI_SLOT(pdev->devfn), 0);
+	struct pci_dev *pdev0 = pci_get_slot(pdev->bus, devfn);
+	u32 val;
+
+	cb710_pci_update_config_reg(pdev, ...
[ message continues ]

[I messed up source trees for previous diff, please use this one instead.]

Here is a version of the CB710 SD/MMC reader driver ready for inclusion
upstream. The code is divided in two parts. There is a virtual 'bus' driver
that handles PCI device and registers three new devices one per card reader
type. The other driver handles SD/MMC part of the reader.

Changes from v4:
  - sg_mapping_iterator extension renamed to avoid namespace clashes
    when/if the extension would be merged upstream
  - appended sgbuf2.h to cb710.h
  - move from IDR to IDA for device ID generation
  - cleanup after IDA on module unload
  - added MAINTAINERS entry
 
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urpN linux-2.6.29.4/drivers/misc/cb710/core.c test2/drivers/misc/cb710/core.c
--- linux-2.6.29.4/drivers/misc/cb710/core.c	1970-01-01 01:00:00.000000000 +0100
+++ test2/drivers/misc/cb710/core.c	2009-05-22 20:28:02.000000000 +0200
@@ -0,0 +1,357 @@
+/*
+ *  cb710/core.c
+ *
+ *  Copyright by Michał Mirosław, 2008-2009
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+#include <linux/idr.h>
+#include <linux/cb710.h>
+
+static DEFINE_IDA(cb710_ida);
+static DEFINE_SPINLOCK(cb710_ida_lock);
+
+void cb710_pci_update_config_reg(struct pci_dev *pdev,
+	int reg, uint32_t mask, uint32_t xor)
+{
+	u32 rval;
+
+	pci_read_config_dword(pdev, reg, &rval);
+	rval = (rval & mask) ^ xor;
+	pci_write_config_dword(pdev, reg, rval);
+}
+EXPORT_SYMBOL_GPL(cb710_pci_update_config_reg);
+
+/* Some magic writes based on Windows driver init code */
+static int __devinit cb710_pci_configure(struct pci_dev *pdev)
+{
+	unsigned int devfn = PCI_DEVFN(PCI_SLOT(pdev->devfn), 0);
+	struct pci_dev *pdev0 = ...
[ message continues ]

On Fri, 22 May 2009 20:33:59 +0200

Queued.

Nice work. :)

-- 
     -- Pierre Ossman

  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.
[ message continues ]

One more fixup for the driver following.

Best Regards,
Michał Mirosław

---here---

cb710: Fix compilation warning in DEBUG case.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urpN test1/drivers/misc/cb710/Kconfig test2/drivers/misc/cb710/Kconfig
--- test1/drivers/misc/cb710/Kconfig	2009-06-04 12:20:29.000000000 +0200
+++ test2/drivers/misc/cb710/Kconfig	2009-06-04 10:53:34.000000000 +0200
@@ -19,3 +19,8 @@ config CB710_DEBUG
 	  This is an option for use by developers; most people should
 	  say N here.  This adds a lot of debugging output to dmesg.
 
+config CB710_DEBUG_ASSUMPTIONS
+	bool
+	depends on CB710_CORE != n
+	default y
+
diff -urpN test1/drivers/misc/cb710/Makefile test2/drivers/misc/cb710/Makefile
--- test1/drivers/misc/cb710/Makefile	2009-06-04 12:20:29.000000000 +0200
+++ test2/drivers/misc/cb710/Makefile	2009-06-04 10:53:34.000000000 +0200
@@ -2,3 +2,5 @@ obj-$(CONFIG_CB710_CORE)	+= cb710.o
 
 cb710-y				:= core.o sgbuf2.o
 cb710-$(CONFIG_CB710_DEBUG)	+= debug.o
+
+ccflags-$(CONFIG_CB710_DEBUG)	+= -DDEBUG
diff -urpN test1/drivers/mmc/host/Makefile test2/drivers/mmc/host/Makefile
--- test1/drivers/mmc/host/Makefile	2009-06-04 12:20:29.000000000 +0200
+++ test2/drivers/mmc/host/Makefile	2009-06-04 10:53:34.000000000 +0200
@@ -29,3 +29,7 @@ obj-$(CONFIG_MMC_SDRICOH_CS)	+= sdricoh_
 obj-$(CONFIG_MMC_TMIO)		+= tmio_mmc.o
 obj-$(CONFIG_MMC_CB710)	+= cb710-mmc.o
 
+ifeq ($(CONFIG_CB710_DEBUG),y)
+CFLAGS-cb710-mmc		+= -DDEBUG
+endif
+
diff -urpN test1/include/linux/cb710.h test2/include/linux/cb710.h
--- test1/include/linux/cb710.h	2009-06-04 12:20:29.000000000 +0200
+++ test2/include/linux/cb710.h	2009-06-04 10:53:34.000000000 +0200
@@ -10,13 +10,6 @@
 #ifndef LINUX_CB710_DRIVER_H
 #define LINUX_CB710_DRIVER_H
 
-#ifdef CONFIG_CB710_DEBUG
-#define DEBUG
-#endif
-
-/* verify assumptions on platform_device framework */
-#define CONFIG_CB710_DEBUG_ASSUMPTIONS
-
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include ...
[ message continues ]

cb710: Fix compilation warnings

Fix compilation warnings reported by Stephen Rothwell and change
the only dev_vdbg() to dev_dbg().

drivers/misc/cb710/debug.c: In function 'cb710_read_regs_8':                                                           
drivers/misc/cb710/debug.c:100: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' 
drivers/misc/cb710/debug.c: In function 'cb710_read_regs_16':                                                          
drivers/misc/cb710/debug.c:101: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' 
drivers/misc/cb710/debug.c: In function 'cb710_read_regs_32':                                                          
drivers/misc/cb710/debug.c:102: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' 
drivers/mmc/host/cb710-mmc.c: In function 'cb710_mmc_set_transfer_size':                                               
drivers/mmc/host/cb710-mmc.c:222: warning: format '%d' expects type 'int', but argument 4 has type 'size_t'            
drivers/mmc/host/cb710-mmc.c:222: warning: format '%d' expects type 'int', but argument 6 has type 'size_t'            

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff -urpN test2/drivers/misc/cb710/debug.c test3/drivers/misc/cb710/debug.c
--- test2/drivers/misc/cb710/debug.c	2009-06-04 10:53:34.000000000 +0200
+++ test3/drivers/misc/cb710/debug.c	2009-06-05 11:22:38.000000000 +0200
@@ -37,7 +37,7 @@ static void cb710_read_regs_##t(void __i
 	unsigned i, j;							\
 									\
 	for (i = 0; i < ARRAY_SIZE(allow); ++i, reg += 16/(t/8)) {	\
-		if (!select & (1 << i))					\
+		if (!(select & (1 << i)))				\
 			continue;					\
 									\
 		for (j = 0; j < 0x10/(t/8); ++j) {			\
diff -urpN test2/drivers/mmc/host/cb710-mmc.c test3/drivers/mmc/host/cb710-mmc.c
--- test2/drivers/mmc/host/cb710-mmc.c	2009-06-04 10:53:34.000000000 +0200
+++ test3/drivers/mmc/host/cb710-mmc.c	2009-06-05 ...
[ message continues ]

On Thu, 4 Jun 2009 12:24:55 +0200

I had already done some of this, but I've merged the rest of the
changes you had here.

Rgds
-- 
     -- Pierre Ossman

  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.
[ message continues ]

I like the general idea and I think that this could serve as a good starting
point into making generic PIO handling library (which has been on my personal
TODO of things to look into for a long time).


Hehe, please keep me on cc:.

Thanks.
Bart
--

[ message continues ]