[PATCH] [R852] Few fixes

[PATCH] [R852] Few fixes

[Maxim Levitsky]

Hi,

This is patch series that implement few fixes in regard to
suspend/resume that I finally (after 2 years) managed to fix on my
notebook.

Currently there are exactly 3 problems, and I am investigating the
solutions.

1) I am finally confident that CONFIG_MMC_RICOH_MMC causes the SD/MMC
portion of the controller to die  (and trigger an interrupt storm).
I don't know if this is a regression, because all I did was to execute
the code that disables MMC controller a bit earlier, because otherwise
my xD driver will bind to wrong pci function.
I still continue to investigate this issue, and maybe the only solution
to it, is to write a full driver for the MMC controller 
(this is done in windows).

When this happens (it doesn't depend on whether card was in slot or not,
controller causes interrupt storm, and continues it till sdhci-pci is
reloaded, at which point it refuses to load.)
Since r852 shares (at least here) the interrupt with sdhci-pci, if I
attempt to reload it, it gets interrupt immediately, and accesses
uninitialized spinlock.
First patch in that series addresses that issue.


2) For some yet unknown to me reason, DMA stops working on r852 after
resume IF the card was inserted during time system was sleeping.
Just card reload works around that issue, but my driver still oopses
when it tries to complete() an completion that time-outed.
The second patch fixes that oops.

3) This is the weirdest problem, and it might be a cause of
CONFIG_MMC_RICOH_MMC too. I didn't yet managed to fully understand when
it happens.
Again when card is inserted during suspend (maybe even in other cases),
the controller stops reporting card insert/remove events.
You can go ahead, remove the xD card, insert SD card, etc, it still
thinks it has xD card.
A suspend/resume cycle allows it to detect the card state 'once', for
example suspending without card makes it finally understand that there
is no card, and allow to insert other cards, but insert of an xD card
doesn't make any difference.

The above is for reference, I will try soon to fix these problems.
(Especially the 2nd).


In addition to that i send a patch to fix problem in Kconfig.
All nand entries weren't shown under mtd menu + at least for now this
driver should depend on smartmedia byte order.
(Technically the SM_FTL should depends on it, but this way is safer)

Best regards,
Maxim Levitsky

[PATCH 1/3] r852: register IRQ as last step

[Maxim Levitsky]

From: Maxim Levitsky <maximlevitsky@gmail.com>

Otherwise, if it fires right away, it might access
uninitialized spinlock

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/mtd/nand/r852.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/r852.c b/drivers/mtd/nand/r852.c
index 78a4232..20a654a 100644
--- a/drivers/mtd/nand/r852.c
+++ b/drivers/mtd/nand/r852.c
@@ -940,18 +940,19 @@ int  r852_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
 
 	r852_dma_test(dev);
 
+	dev->irq = pci_dev->irq;
+	spin_lock_init(&dev->irqlock);
+
+	dev->card_detected = 0;
+	r852_card_update_present(dev);
+
 	/*register irq handler*/
 	error = -ENODEV;
 	if (request_irq(pci_dev->irq, &r852_irq, IRQF_SHARED,
 			  DRV_NAME, dev))
 		goto error10;
 
-	dev->irq = pci_dev->irq;
-	spin_lock_init(&dev->irqlock);
-
 	/* kick initial present test */
-	dev->card_detected = 0;
-	r852_card_update_present(dev);
 	queue_delayed_work(dev->card_workqueue,
 		&dev->card_detect_work, 0);
 
-- 
1.7.0.4

[PATCH 2/3] r852: Fixes in case of DMA timeout

[Maxim Levitsky]

* Don't call complete on dma completion
* do a INIT_COMPLETE before using it each time
* Report DMA read error via ecc 'correct'

I finally managed to make my system do suspend to ram propertly, and I see that
if card was inserted during suspend (while system was off), I get dma timeouts
on resume. Simple card reinsert solves the issue.
This patch solves a crash that would happen otherwise

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/mtd/nand/r852.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/r852.c b/drivers/mtd/nand/r852.c
index 20a654a..3f219e6 100644
--- a/drivers/mtd/nand/r852.c
+++ b/drivers/mtd/nand/r852.c
@@ -150,7 +150,6 @@ static void r852_dma_done(struct r852_device *dev, int error)
 	if (dev->phys_dma_addr && dev->phys_dma_addr != dev->phys_bounce_buffer)
 		pci_unmap_single(dev->pci_dev, dev->phys_dma_addr, R852_DMA_LEN,
 			dev->dma_dir ? PCI_DMA_FROMDEVICE : PCI_DMA_TODEVICE);
-	complete(&dev->dma_done);
 }
 
 /*
@@ -182,6 +181,7 @@ static void r852_do_dma(struct r852_device *dev, uint8_t *buf, int do_read)
 	/* Set dma direction */
 	dev->dma_dir = do_read;
 	dev->dma_stage = 1;
+	INIT_COMPLETION(dev->dma_done);
 
 	dbg_verbose("doing dma %s ", do_read ? "read" : "write");
 
@@ -494,6 +494,11 @@ int r852_ecc_correct(struct mtd_info *mtd, uint8_t *dat,
 	if (dev->card_unstable)
 		return 0;
 
+	if (dev->dma_error) {
+		dev->dma_error = 0;
+		return -1;
+	}
+
 	r852_write_reg(dev, R852_CTL, dev->ctlreg | R852_CTL_ECC_ACCESS);
 	ecc_reg = r852_read_reg_dword(dev, R852_DATALINE);
 	r852_write_reg(dev, R852_CTL, dev->ctlreg);
@@ -796,6 +801,7 @@ static irqreturn_t r852_irq(int irq, void *data)
 		if (dma_status & R852_DMA_IRQ_ERROR) {
 			dbg("recieved dma error IRQ");
 			r852_dma_done(dev, -EIO);
+			complete(&dev->dma_done);
 			goto out;
 		}
 
@@ -825,8 +831,10 @@ static irqreturn_t r852_irq(int irq, void *data)
 			r852_dma_enable(dev);
 
 		/* Operation done */
-		if (dev->dma_stage == 3)
+		if (dev->dma_stage == 3) {
 			r852_dma_done(dev, 0);
+			complete(&dev->dma_done);
+		}
 		goto out;
 	}
 
@@ -1082,7 +1090,7 @@ int r852_resume(struct device *device)
 			dev->card_detected ? "added" : "removed");
 
 		queue_delayed_work(dev->card_workqueue,
-		&dev->card_detect_work, 1000);
+		&dev->card_detect_work, msecs_to_jiffies(1000));
 		return 0;
 	}
 
-- 
1.7.0.4

[PATCH 3/3] MTD: Kconfig cleanups

[Maxim Levitsky]

Move all nand options under 'if MTD_NAND'
otherwise xconfig output is broken.

make Ricoh xD driver depend on MTD_NAND_ECC_SMC, untill
proper solution is made

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/mtd/nand/Kconfig |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 98a04b3..8db333f 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -8,6 +8,8 @@ menuconfig MTD_NAND
 	  devices. For further information see
 	  <http://www.linux-mtd.infradead.org/doc/nand.html>.
 
+if MTD_NAND
+
 config MTD_NAND_ECC
 	tristate
 
@@ -19,7 +21,7 @@ config MTD_NAND_ECC_SMC
 	  Software ECC according to the Smart Media Specification.
 	  The original Linux implementation had byte 0 and 1 swapped.
 
-if MTD_NAND
+
 
 config MTD_NAND_VERIFY_WRITE
 	bool "Verify NAND page writes"
@@ -129,6 +131,7 @@ config MTD_NAND_RICOH
 	default n
 	depends on PCI
 	select MTD_SM_COMMON
+	select MTD_NAND_ECC_SMC
 	help
 	  Enable support for Ricoh R5C852 xD card reader
 	  You also need to enable ether
-- 
1.7.0.4

Re: [PATCH 3/3] MTD: Kconfig cleanups

[David Woodhouse]

On Sat, 2010-05-22 at 04:07 +0300, Maxim Levitsky wrote:
> 
> @@ -129,6 +131,7 @@ config MTD_NAND_RICOH
>         default n
>         depends on PCI
>         select MTD_SM_COMMON
> +       select MTD_NAND_ECC_SMC 

I thought we had a better solution than this -- weren't we going to make
it a runtime thing? And default to the 'correct' order?

Then, only the legacy devices which need to retain the 'wrong' byteorder
for compatibility reasons will need to do it.

-- 
dwmw2

Re: [PATCH 3/3] MTD: Kconfig cleanups

[Maxim Levitsky]

On Sat, 2010-05-22 at 08:47 +0100, David Woodhouse wrote: 
> On Sat, 2010-05-22 at 04:07 +0300, Maxim Levitsky wrote:
> > 
> > @@ -129,6 +131,7 @@ config MTD_NAND_RICOH
> >         default n
> >         depends on PCI
> >         select MTD_SM_COMMON
> > +       select MTD_NAND_ECC_SMC 
> 
> I thought we had a better solution than this -- weren't we going to make
> it a runtime thing? And default to the 'correct' order?
> 
> Then, only the legacy devices which need to retain the 'wrong' byteorder
> for compatibility reasons will need to do it.
> 
Sure, but I'll need to do several changes to drivers that I can't test.
For now this ensures correctness, then I fix all of this.

Best regards,
Maxim Levitsky

Re: [PATCH 3/3] MTD: Kconfig cleanups

[Maxim Levitsky]

On Sat, 2010-05-22 at 10:58 +0300, Maxim Levitsky wrote: 
> On Sat, 2010-05-22 at 08:47 +0100, David Woodhouse wrote: 
> > On Sat, 2010-05-22 at 04:07 +0300, Maxim Levitsky wrote:
> > > 
> > > @@ -129,6 +131,7 @@ config MTD_NAND_RICOH
> > >         default n
> > >         depends on PCI
> > >         select MTD_SM_COMMON
> > > +       select MTD_NAND_ECC_SMC 
> > 
> > I thought we had a better solution than this -- weren't we going to make
> > it a runtime thing? And default to the 'correct' order?
> > 
> > Then, only the legacy devices which need to retain the 'wrong' byteorder
> > for compatibility reasons will need to do it.
> > 
> Sure, but I'll need to do several changes to drivers that I can't test.
> For now this ensures correctness, then I fix all of this.
In other words, when I get a bit more free time I promise I fix this and
other problems. For now this ensures correctness at least of this thing.

Best regards,
Maxim Levitsky

Re: [PATCH 3/3] MTD: Kconfig cleanups

[Maxim Levitsky]

On Sat, 2010-05-22 at 11:06 +0300, Maxim Levitsky wrote: 
> On Sat, 2010-05-22 at 10:58 +0300, Maxim Levitsky wrote: 
> > On Sat, 2010-05-22 at 08:47 +0100, David Woodhouse wrote: 
> > > On Sat, 2010-05-22 at 04:07 +0300, Maxim Levitsky wrote:
> > > > 
> > > > @@ -129,6 +131,7 @@ config MTD_NAND_RICOH
> > > >         default n
> > > >         depends on PCI
> > > >         select MTD_SM_COMMON
> > > > +       select MTD_NAND_ECC_SMC 
> > > 
> > > I thought we had a better solution than this -- weren't we going to make
> > > it a runtime thing? And default to the 'correct' order?
> > > 
> > > Then, only the legacy devices which need to retain the 'wrong' byteorder
> > > for compatibility reasons will need to do it.
> > > 
> > Sure, but I'll need to do several changes to drivers that I can't test.
> > For now this ensures correctness, then I fix all of this.
> In other words, when I get a bit more free time I promise I fix this and
> other problems. For now this ensures correctness at least of this thing.
Thinking again about this, this doesn't help much ether, so lets keep it
as is, except that 'if MTD_NAND' should be moved higher.


Best regards,
Maxim Levitsky

[PATCH v2] MTD: Fix NAND submenu

[Maxim Levitsky]

Move all nand options under 'if MTD_NAND'
otherwise xconfig output is broken.


Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/mtd/nand/Kconfig |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 98a04b3..5927239 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -8,6 +8,8 @@ menuconfig MTD_NAND
 	  devices. For further information see
 	  <http://www.linux-mtd.infradead.org/doc/nand.html>.
 
+if MTD_NAND
+
 config MTD_NAND_ECC
 	tristate
 
@@ -19,7 +21,7 @@ config MTD_NAND_ECC_SMC
 	  Software ECC according to the Smart Media Specification.
 	  The original Linux implementation had byte 0 and 1 swapped.
 
-if MTD_NAND
+
 
 config MTD_NAND_VERIFY_WRITE
 	bool "Verify NAND page writes"
-- 
1.7.0.4