LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476
@ 2008-01-31 17:38 Frank Seidel
  2008-02-02  7:16 ` Philip Langdale
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Seidel @ 2008-01-31 17:38 UTC (permalink / raw)
  To: sdhci-devel
  Cc: linux-kernel, Pierre Ossman, Philip Langdale, Andrew de Quincey

From: Frank Seidel <fseidel@suse.de>

This patch (based on current linus git tree) adds support for
the Ricoh RL5c476 chip: with this the mmc adapter that needs this
disabler (R5C843) can also be handled correctly when it sits
on a RL5c476.

Signed-off-by: Frank Seidel <fseidel@suse.de>
---
 drivers/mmc/host/ricoh_mmc.c |   91 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 16 deletions(-)

--- a/drivers/mmc/host/ricoh_mmc.c
+++ b/drivers/mmc/host/ricoh_mmc.c
@@ -45,8 +45,10 @@ static int __devinit ricoh_mmc_probe(str
 				     const struct pci_device_id *ent)
 {
 	u8 rev;
+	u8 ctrlfound = 0;
 
 	struct pci_dev *fw_dev = NULL;
+	struct pci_dev *main_dev = NULL; /* for Ricoh RL5c476 II */
 
 	BUG_ON(pdev == NULL);
 	BUG_ON(ent == NULL);
@@ -58,7 +60,47 @@ static int __devinit ricoh_mmc_probe(str
 		pci_name(pdev), (int)pdev->vendor, (int)pdev->device,
 		(int)rev);
 
-	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
+	/* disable mmc controller via main function (RL5C476) */
+	while ((main_dev =
+		pci_get_device(PCI_VENDOR_ID_RICOH,
+				PCI_DEVICE_ID_RICOH_RL5C476, main_dev))) {
+		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(main_dev->devfn) &&
+		    pdev->bus == main_dev->bus) {
+			u8 write_enable;
+			u8 write_target;
+			u8 disable;
+
+			pci_read_config_byte(main_dev, 0xB7, &disable);
+			if (disable & 0x02) {
+				printk(KERN_INFO DRIVER_NAME
+					": Controller already disabled. " \
+					"Nothing to do.\n");
+				return -ENODEV;
+			}
+
+			pci_read_config_byte(main_dev, 0x8E, &write_enable);
+			pci_write_config_byte(main_dev, 0x8E, 0xAA);
+			pci_read_config_byte(main_dev, 0x8D, &write_target);
+			pci_write_config_byte(main_dev, 0x8D, 0xB7);
+			pci_write_config_byte(main_dev, 0xB7, disable | 0x02);
+			pci_write_config_byte(main_dev, 0x8E, write_enable);
+			pci_write_config_byte(main_dev, 0x8D, write_target);
+
+			pci_set_drvdata(pdev, main_dev);
+
+			printk(KERN_INFO DRIVER_NAME
+				": Controller is now disabled " \
+				"(via R5L5C476).\n");
+
+			ctrlfound = 1;
+			break;
+		}
+	}
+
+	/* disable mmc controller via firewire function (R5C832) */
+	while (!ctrlfound &&
+	    (fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
+					PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
 		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
 		    pdev->bus == fw_dev->bus) {
 			u8 write_enable;
@@ -67,7 +109,8 @@ static int __devinit ricoh_mmc_probe(str
 			pci_read_config_byte(fw_dev, 0xCB, &disable);
 			if (disable & 0x02) {
 				printk(KERN_INFO DRIVER_NAME
-				       ": Controller already disabled. Nothing to do.\n");
+					": Controller already disabled. " \
+					"Nothing to do.\n");
 				return -ENODEV;
 			}
 
@@ -79,15 +122,16 @@ static int __devinit ricoh_mmc_probe(str
 			pci_set_drvdata(pdev, fw_dev);
 
 			printk(KERN_INFO DRIVER_NAME
-			       ": Controller is now disabled.\n");
+			       ": Controller is now disabled (via R5C832).\n");
 
-			break;
+			ctrlfound = 1;
 		}
 	}
 
-	if (pci_get_drvdata(pdev) == NULL) {
+	if (!ctrlfound) {
 		printk(KERN_WARNING DRIVER_NAME
-		       ": Main firewire function not found. Cannot disable controller.\n");
+			": Needed function was not found. " \
+			"Cannot disable controller.\n");
 		return -ENODEV;
 	}
 
@@ -97,20 +141,35 @@ static int __devinit ricoh_mmc_probe(str
 static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
 {
 	u8 write_enable;
+	u8 write_target;
 	u8 disable;
-	struct pci_dev *fw_dev = NULL;
+	struct pci_dev *ctrl_dev = NULL;
 
-	fw_dev = pci_get_drvdata(pdev);
-	BUG_ON(fw_dev == NULL);
+	ctrl_dev = pci_get_drvdata(pdev);
+	BUG_ON(ctrl_dev == NULL);
 
-	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
-	pci_read_config_byte(fw_dev, 0xCB, &disable);
-	pci_write_config_byte(fw_dev, 0xCA, 0x57);
-	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
-	pci_write_config_byte(fw_dev, 0xCA, write_enable);
+	if (ctrl_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
+		pci_read_config_byte(ctrl_dev, 0x8E, &write_enable);
+		pci_write_config_byte(ctrl_dev, 0x8E, 0xAA);
+		pci_read_config_byte(ctrl_dev, 0x8D, &write_target);
+		pci_write_config_byte(ctrl_dev, 0x8D, 0xB7);
+		pci_read_config_byte(ctrl_dev, 0xB7, &disable);
+		pci_write_config_byte(ctrl_dev, 0xB7, disable & ~0x02);
+		pci_write_config_byte(ctrl_dev, 0x8E, write_enable);
+		pci_write_config_byte(ctrl_dev, 0x8D, write_target);
+
+		printk(KERN_INFO DRIVER_NAME
+			": Controller is now re-enabled (via R5L5C476).\n");
+	} else {
+		pci_read_config_byte(ctrl_dev, 0xCA, &write_enable);
+		pci_read_config_byte(ctrl_dev, 0xCB, &disable);
+		pci_write_config_byte(ctrl_dev, 0xCA, 0x57);
+		pci_write_config_byte(ctrl_dev, 0xCB, disable & ~0x02);
+		pci_write_config_byte(ctrl_dev, 0xCA, write_enable);
 
-	printk(KERN_INFO DRIVER_NAME
-	       ": Controller is now re-enabled.\n");
+		printk(KERN_INFO DRIVER_NAME
+			": Controller is now re-enabled (via R5C832).\n");
+	}
 
 	pci_set_drvdata(pdev, NULL);
 }

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-01-31 17:38 [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
@ 2008-02-02  7:16 ` Philip Langdale
  2008-02-02  9:30   ` Frank Seidel
                     ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Philip Langdale @ 2008-02-02  7:16 UTC (permalink / raw)
  To: Frank Seidel; +Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey

Frank Seidel wrote:
> From: Frank Seidel <fseidel@suse.de>
> 
> This patch (based on current linus git tree) adds support for
> the Ricoh RL5c476 chip: with this the mmc adapter that needs this
> disabler (R5C843) can also be handled correctly when it sits
> on a RL5c476.

Again, thanks a lot for investigating and finding the appropriate magic
incantations. My main comment is to please base this on top of my suspend/resume
patch which Pierre said he accepted but which isn't in his tree yet - I guess
he's busy offline right now - haven't heard from him in a while.

> Signed-off-by: Frank Seidel <fseidel@suse.de>
> ---
>  drivers/mmc/host/ricoh_mmc.c |   91 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 75 insertions(+), 16 deletions(-)
> 
> --- a/drivers/mmc/host/ricoh_mmc.c
> +++ b/drivers/mmc/host/ricoh_mmc.c
> @@ -45,8 +45,10 @@ static int __devinit ricoh_mmc_probe(str
>  				     const struct pci_device_id *ent)
>  {
>  	u8 rev;
> +	u8 ctrlfound = 0;
>  
>  	struct pci_dev *fw_dev = NULL;
> +	struct pci_dev *main_dev = NULL; /* for Ricoh RL5c476 II */

There's no need to have two pointers. One will do fine.

>  	BUG_ON(pdev == NULL);
>  	BUG_ON(ent == NULL);
> @@ -58,7 +60,47 @@ static int __devinit ricoh_mmc_probe(str
>  		pci_name(pdev), (int)pdev->vendor, (int)pdev->device,
>  		(int)rev);
>  
> -	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
> +	/* disable mmc controller via main function (RL5C476) */
> +	while ((main_dev =
> +		pci_get_device(PCI_VENDOR_ID_RICOH,
> +				PCI_DEVICE_ID_RICOH_RL5C476, main_dev))) {
> +		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(main_dev->devfn) &&
> +		    pdev->bus == main_dev->bus) {
> +			u8 write_enable;
> +			u8 write_target;
> +			u8 disable;
> +
> +			pci_read_config_byte(main_dev, 0xB7, &disable);
> +			if (disable & 0x02) {
> +				printk(KERN_INFO DRIVER_NAME
> +					": Controller already disabled. " \
> +					"Nothing to do.\n");
> +				return -ENODEV;
> +			}
> +
> +			pci_read_config_byte(main_dev, 0x8E, &write_enable);
> +			pci_write_config_byte(main_dev, 0x8E, 0xAA);
> +			pci_read_config_byte(main_dev, 0x8D, &write_target);
> +			pci_write_config_byte(main_dev, 0x8D, 0xB7);
> +			pci_write_config_byte(main_dev, 0xB7, disable | 0x02);
> +			pci_write_config_byte(main_dev, 0x8E, write_enable);
> +			pci_write_config_byte(main_dev, 0x8D, write_target);
> +
> +			pci_set_drvdata(pdev, main_dev);
> +
> +			printk(KERN_INFO DRIVER_NAME
> +				": Controller is now disabled " \
> +				"(via R5L5C476).\n");
> +
> +			ctrlfound = 1;
> +			break;
> +		}
> +	}
> +
> +	/* disable mmc controller via firewire function (R5C832) */
> +	while (!ctrlfound &&
> +	    (fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
> +					PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
>  		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
>  		    pdev->bus == fw_dev->bus) {
>  			u8 write_enable;

It feels like there's a bit too much code duplication going on here, but I think
that after you rebase the patch it'll look a lot tidier and I won't feel bad about
it :-)

> @@ -67,7 +109,8 @@ static int __devinit ricoh_mmc_probe(str
>  			pci_read_config_byte(fw_dev, 0xCB, &disable);
>  			if (disable & 0x02) {
>  				printk(KERN_INFO DRIVER_NAME
> -				       ": Controller already disabled. Nothing to do.\n");
> +					": Controller already disabled. " \
> +					"Nothing to do.\n");
>  				return -ENODEV;
>  			}
>  
> @@ -79,15 +122,16 @@ static int __devinit ricoh_mmc_probe(str
>  			pci_set_drvdata(pdev, fw_dev);
>  
>  			printk(KERN_INFO DRIVER_NAME
> -			       ": Controller is now disabled.\n");
> +			       ": Controller is now disabled (via R5C832).\n");
>  
> -			break;
> +			ctrlfound = 1;
>  		}
>  	}
>  
> -	if (pci_get_drvdata(pdev) == NULL) {
> +	if (!ctrlfound) {
>  		printk(KERN_WARNING DRIVER_NAME
> -		       ": Main firewire function not found. Cannot disable controller.\n");
> +			": Needed function was not found. " \
> +			"Cannot disable controller.\n");
>  		return -ENODEV;
>  	}
>  
> @@ -97,20 +141,35 @@ static int __devinit ricoh_mmc_probe(str
>  static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
>  {
>  	u8 write_enable;
> +	u8 write_target;
>  	u8 disable;
> -	struct pci_dev *fw_dev = NULL;
> +	struct pci_dev *ctrl_dev = NULL;

Same as in probe. Just use one pointer.

> -	fw_dev = pci_get_drvdata(pdev);
> -	BUG_ON(fw_dev == NULL);
> +	ctrl_dev = pci_get_drvdata(pdev);
> +	BUG_ON(ctrl_dev == NULL);
>  
> -	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> -	pci_read_config_byte(fw_dev, 0xCB, &disable);
> -	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> -	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
> -	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +	if (ctrl_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
> +		pci_read_config_byte(ctrl_dev, 0x8E, &write_enable);
> +		pci_write_config_byte(ctrl_dev, 0x8E, 0xAA);
> +		pci_read_config_byte(ctrl_dev, 0x8D, &write_target);
> +		pci_write_config_byte(ctrl_dev, 0x8D, 0xB7);
> +		pci_read_config_byte(ctrl_dev, 0xB7, &disable);
> +		pci_write_config_byte(ctrl_dev, 0xB7, disable & ~0x02);
> +		pci_write_config_byte(ctrl_dev, 0x8E, write_enable);
> +		pci_write_config_byte(ctrl_dev, 0x8D, write_target);
> +
> +		printk(KERN_INFO DRIVER_NAME
> +			": Controller is now re-enabled (via R5L5C476).\n");
> +	} else {
> +		pci_read_config_byte(ctrl_dev, 0xCA, &write_enable);
> +		pci_read_config_byte(ctrl_dev, 0xCB, &disable);
> +		pci_write_config_byte(ctrl_dev, 0xCA, 0x57);
> +		pci_write_config_byte(ctrl_dev, 0xCB, disable & ~0x02);
> +		pci_write_config_byte(ctrl_dev, 0xCA, write_enable);
>  
> -	printk(KERN_INFO DRIVER_NAME
> -	       ": Controller is now re-enabled.\n");
> +		printk(KERN_INFO DRIVER_NAME
> +			": Controller is now re-enabled (via R5C832).\n");
> +	}
>  
>  	pci_set_drvdata(pdev, NULL);
>  }
> 

--phil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-02  7:16 ` Philip Langdale
@ 2008-02-02  9:30   ` Frank Seidel
  2008-02-04 18:25   ` Frank Seidel
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Frank Seidel @ 2008-02-02  9:30 UTC (permalink / raw)
  To: Philip Langdale
  Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey

On Saturday 02 February 2008 08:16, Philip Langdale wrote:
> Again, thanks a lot for investigating and finding the appropriate magic
> incantations. My main comment is to please base this on top of my suspend/resume
> patch which Pierre said he accepted but which isn't in his tree yet - I guess
> he's busy offline right now - haven't heard from him in a while.

Yes, sorry, in the meantime i saw your suspend/resume patch .. will rebase this on
it, but also due to being a bit bussy now probably not until beginning
of next week.

> >  	struct pci_dev *fw_dev = NULL;
> > +	struct pci_dev *main_dev = NULL; /* for Ricoh RL5c476 II */
> 
> There's no need to have two pointers. One will do fine.

yes, fully agree ..

> It feels like there's a bit too much code duplication going on here, but I think
> that after you rebase the patch it'll look a lot tidier and I won't feel bad about
> it :-)

yes, also fully agree .. sadfully i was in a bit of a hurry that day needing a working
patch for a internal deadline.

> > -	struct pci_dev *fw_dev = NULL;
> > +	struct pci_dev *ctrl_dev = NULL;
> 
> Same as in probe. Just use one pointer.

Hehe, no, not here ... its "-", "+" ;-)

Thanks,
Frank

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-02  7:16 ` Philip Langdale
  2008-02-02  9:30   ` Frank Seidel
@ 2008-02-04 18:25   ` Frank Seidel
  2008-02-04 18:25   ` [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed) Frank Seidel
  2008-02-04 18:25   ` [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
  3 siblings, 0 replies; 11+ messages in thread
From: Frank Seidel @ 2008-02-04 18:25 UTC (permalink / raw)
  To: Philip Langdale
  Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey

Hi,

On Samstag 02 Februar 2008 08:16:48, you (Philip Langdale) wrote:
> Again, thanks a lot for investigating and finding the appropriate magic
> incantations. My main comment is to please base this on top of my suspend/resume
> patch which Pierre said he accepted but which isn't in his tree yet - I guess
> he's busy offline right now - haven't heard from him in a while.

to any reason i had some strange problems applying your patch. But i finally made
it and rebased my patch on top of it.
So, in a few moments i'll sent it here as v2 and also a quilt refreshed version
of yours (just in case also somebody had problems.. ah and i fixed a typo
in your signed-off and added mine).

Thanks,
Frank

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed)
  2008-02-02  7:16 ` Philip Langdale
  2008-02-02  9:30   ` Frank Seidel
  2008-02-04 18:25   ` Frank Seidel
@ 2008-02-04 18:25   ` Frank Seidel
  2008-02-04 20:17     ` Philip Langdale
  2008-02-04 18:25   ` [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
  3 siblings, 1 reply; 11+ messages in thread
From: Frank Seidel @ 2008-02-04 18:25 UTC (permalink / raw)
  To: Philip Langdale
  Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey

From: Philip Langdale <philipl@overt.org>

As pci config space is reinitialised on suspend/resume cycle, the
disabler needs to work its magic at resume time. For symmetry this
change also explicitly enables the controller at suspend time but
it's not strictly necessary.

Signed-off-by: Philip Langdale <philipl@overt.org>
Signed-off-by: Frank Seidel <fseidel@suse.de>
---
 drivers/mmc/host/ricoh_mmc.c |   97 +++++++++++++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 25 deletions(-)

--- a/drivers/mmc/host/ricoh_mmc.c
+++ b/drivers/mmc/host/ricoh_mmc.c
@@ -41,6 +41,46 @@ static const struct pci_device_id pci_id
 
 MODULE_DEVICE_TABLE(pci, pci_ids);
 
+static int ricoh_mmc_disable(struct pci_dev *fw_dev)
+{
+	u8 write_enable;
+	u8 disable;
+
+	pci_read_config_byte(fw_dev, 0xCB, &disable);
+	if (disable & 0x02) {
+		printk(KERN_INFO DRIVER_NAME
+		       ": Controller already disabled. Nothing to do.\n");
+		return -ENODEV;
+	}
+
+	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
+	pci_write_config_byte(fw_dev, 0xCA, 0x57);
+	pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
+	pci_write_config_byte(fw_dev, 0xCA, write_enable);
+
+	printk(KERN_INFO DRIVER_NAME
+	       ": Controller is now disabled.\n");
+
+	return 0;
+}
+
+static int ricoh_mmc_enable(struct pci_dev *fw_dev)
+{
+	u8 write_enable;
+	u8 disable;
+
+	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
+	pci_read_config_byte(fw_dev, 0xCB, &disable);
+	pci_write_config_byte(fw_dev, 0xCA, 0x57);
+	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
+	pci_write_config_byte(fw_dev, 0xCA, write_enable);
+
+	printk(KERN_INFO DRIVER_NAME
+	       ": Controller is now re-enabled.\n");
+
+	return 0;
+}
+
 static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {
@@ -61,26 +101,12 @@ static int __devinit ricoh_mmc_probe(str
 	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
 		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
 		    pdev->bus == fw_dev->bus) {
-			u8 write_enable;
-			u8 disable;
-
-			pci_read_config_byte(fw_dev, 0xCB, &disable);
-			if (disable & 0x02) {
-				printk(KERN_INFO DRIVER_NAME
-				       ": Controller already disabled. Nothing to do.\n");
+			if (ricoh_mmc_disable(fw_dev) != 0) {
 				return -ENODEV;
 			}
 
-			pci_read_config_byte(fw_dev, 0xCA, &write_enable);
-			pci_write_config_byte(fw_dev, 0xCA, 0x57);
-			pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
-			pci_write_config_byte(fw_dev, 0xCA, write_enable);
-
 			pci_set_drvdata(pdev, fw_dev);
 
-			printk(KERN_INFO DRIVER_NAME
-			       ": Controller is now disabled.\n");
-
 			break;
 		}
 	}
@@ -96,30 +122,51 @@ static int __devinit ricoh_mmc_probe(str
 
 static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
 {
-	u8 write_enable;
-	u8 disable;
 	struct pci_dev *fw_dev = NULL;
 
 	fw_dev = pci_get_drvdata(pdev);
 	BUG_ON(fw_dev == NULL);
 
-	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
-	pci_read_config_byte(fw_dev, 0xCB, &disable);
-	pci_write_config_byte(fw_dev, 0xCA, 0x57);
-	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
-	pci_write_config_byte(fw_dev, 0xCA, write_enable);
-
-	printk(KERN_INFO DRIVER_NAME
-	       ": Controller is now re-enabled.\n");
+	ricoh_mmc_enable(fw_dev);
 
 	pci_set_drvdata(pdev, NULL);
 }
 
+static int ricoh_mmc_suspend (struct pci_dev *pdev, pm_message_t state)
+{
+	struct pci_dev *fw_dev = NULL;
+
+	fw_dev = pci_get_drvdata(pdev);
+	BUG_ON(fw_dev == NULL);
+
+	printk(KERN_INFO DRIVER_NAME ": Suspending.\n");
+
+	ricoh_mmc_enable(fw_dev);
+
+	return 0;
+}
+
+static int ricoh_mmc_resume (struct pci_dev *pdev)
+{
+	struct pci_dev *fw_dev = NULL;
+
+	fw_dev = pci_get_drvdata(pdev);
+	BUG_ON(fw_dev == NULL);
+
+	printk(KERN_INFO DRIVER_NAME ": Resuming.\n");
+
+	ricoh_mmc_disable(fw_dev);
+
+	return 0;
+}
+
 static struct pci_driver ricoh_mmc_driver = {
 	.name = 	DRIVER_NAME,
 	.id_table =	pci_ids,
 	.probe = 	ricoh_mmc_probe,
 	.remove =	__devexit_p(ricoh_mmc_remove),
+	.suspend =	ricoh_mmc_suspend,
+	.resume =	ricoh_mmc_resume,
 };
 
 /*****************************************************************************\

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-02  7:16 ` Philip Langdale
                     ` (2 preceding siblings ...)
  2008-02-04 18:25   ` [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed) Frank Seidel
@ 2008-02-04 18:25   ` Frank Seidel
  2008-02-04 20:18     ` Philip Langdale
  2008-02-07  8:08     ` Pierre Ossman
  3 siblings, 2 replies; 11+ messages in thread
From: Frank Seidel @ 2008-02-04 18:25 UTC (permalink / raw)
  To: Philip Langdale
  Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey

From: Frank Seidel <fseidel@suse.de>

This patch (base on current linus git tree plus Philip Langdales
suspend/resume patch) adds support for the Ricoh RL5c476 chip:
with this the mmc adapter that needs this disabler (R5C843) can
also be handled correctly when it sits on a RL5c476.
(+ minor style changes (removed spaces between function names
and open parenthesis .. checkpatch warned from previos patch))

Signed-off-by: Frank Seidel <fseidel@suse.de>
---
 drivers/mmc/host/ricoh_mmc.c |  101 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 80 insertions(+), 21 deletions(-)

--- a/drivers/mmc/host/ricoh_mmc.c
+++ b/drivers/mmc/host/ricoh_mmc.c
@@ -44,19 +44,43 @@ MODULE_DEVICE_TABLE(pci, pci_ids);
 static int ricoh_mmc_disable(struct pci_dev *fw_dev)
 {
 	u8 write_enable;
+	u8 write_target;
 	u8 disable;
 
-	pci_read_config_byte(fw_dev, 0xCB, &disable);
-	if (disable & 0x02) {
-		printk(KERN_INFO DRIVER_NAME
-		       ": Controller already disabled. Nothing to do.\n");
-		return -ENODEV;
-	}
+	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
+		/* via RL5C476 */
+
+		pci_read_config_byte(fw_dev, 0xB7, &disable);
+		if (disable & 0x02) {
+			printk(KERN_INFO DRIVER_NAME
+				": Controller already disabled. " \
+				"Nothing to do.\n");
+			return -ENODEV;
+		}
+
+		pci_read_config_byte(fw_dev, 0x8E, &write_enable);
+		pci_write_config_byte(fw_dev, 0x8E, 0xAA);
+		pci_read_config_byte(fw_dev, 0x8D, &write_target);
+		pci_write_config_byte(fw_dev, 0x8D, 0xB7);
+		pci_write_config_byte(fw_dev, 0xB7, disable | 0x02);
+		pci_write_config_byte(fw_dev, 0x8E, write_enable);
+		pci_write_config_byte(fw_dev, 0x8D, write_target);
+	} else {
+		/* via R5C832 */
+
+		pci_read_config_byte(fw_dev, 0xCB, &disable);
+		if (disable & 0x02) {
+			printk(KERN_INFO DRIVER_NAME
+			       ": Controller already disabled. " \
+				"Nothing to do.\n");
+			return -ENODEV;
+		}
 
-	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
-	pci_write_config_byte(fw_dev, 0xCA, 0x57);
-	pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
-	pci_write_config_byte(fw_dev, 0xCA, write_enable);
+		pci_read_config_byte(fw_dev, 0xCA, &write_enable);
+		pci_write_config_byte(fw_dev, 0xCA, 0x57);
+		pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
+		pci_write_config_byte(fw_dev, 0xCA, write_enable);
+	}
 
 	printk(KERN_INFO DRIVER_NAME
 	       ": Controller is now disabled.\n");
@@ -67,13 +91,29 @@ static int ricoh_mmc_disable(struct pci_
 static int ricoh_mmc_enable(struct pci_dev *fw_dev)
 {
 	u8 write_enable;
+	u8 write_target;
 	u8 disable;
 
-	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
-	pci_read_config_byte(fw_dev, 0xCB, &disable);
-	pci_write_config_byte(fw_dev, 0xCA, 0x57);
-	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
-	pci_write_config_byte(fw_dev, 0xCA, write_enable);
+	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
+		/* via RL5C476 */
+
+		pci_read_config_byte(fw_dev, 0x8E, &write_enable);
+		pci_write_config_byte(fw_dev, 0x8E, 0xAA);
+		pci_read_config_byte(fw_dev, 0x8D, &write_target);
+		pci_write_config_byte(fw_dev, 0x8D, 0xB7);
+		pci_read_config_byte(fw_dev, 0xB7, &disable);
+		pci_write_config_byte(fw_dev, 0xB7, disable & ~0x02);
+		pci_write_config_byte(fw_dev, 0x8E, write_enable);
+		pci_write_config_byte(fw_dev, 0x8D, write_target);
+	} else {
+		/* via R5C832 */
+
+		pci_read_config_byte(fw_dev, 0xCA, &write_enable);
+		pci_read_config_byte(fw_dev, 0xCB, &disable);
+		pci_write_config_byte(fw_dev, 0xCA, 0x57);
+		pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
+		pci_write_config_byte(fw_dev, 0xCA, write_enable);
+	}
 
 	printk(KERN_INFO DRIVER_NAME
 	       ": Controller is now re-enabled.\n");
@@ -85,6 +125,7 @@ static int __devinit ricoh_mmc_probe(str
 				     const struct pci_device_id *ent)
 {
 	u8 rev;
+	u8 ctrlfound = 0;
 
 	struct pci_dev *fw_dev = NULL;
 
@@ -98,20 +139,38 @@ static int __devinit ricoh_mmc_probe(str
 		pci_name(pdev), (int)pdev->vendor, (int)pdev->device,
 		(int)rev);
 
-	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
+	while ((fw_dev =
+		pci_get_device(PCI_VENDOR_ID_RICOH,
+			PCI_DEVICE_ID_RICOH_RL5C476, fw_dev))) {
 		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
 		    pdev->bus == fw_dev->bus) {
-			if (ricoh_mmc_disable(fw_dev) != 0) {
+			if (ricoh_mmc_disable(fw_dev) != 0)
 				return -ENODEV;
-			}
 
 			pci_set_drvdata(pdev, fw_dev);
 
+			++ctrlfound;
 			break;
 		}
 	}
 
-	if (pci_get_drvdata(pdev) == NULL) {
+	fw_dev = NULL;
+
+	while (!ctrlfound &&
+	    (fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
+					PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
+		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
+		    pdev->bus == fw_dev->bus) {
+			if (ricoh_mmc_disable(fw_dev) != 0)
+				return -ENODEV;
+
+			pci_set_drvdata(pdev, fw_dev);
+
+			++ctrlfound;
+		}
+	}
+
+	if (!ctrlfound) {
 		printk(KERN_WARNING DRIVER_NAME
 		       ": Main firewire function not found. Cannot disable controller.\n");
 		return -ENODEV;
@@ -132,7 +191,7 @@ static void __devexit ricoh_mmc_remove(s
 	pci_set_drvdata(pdev, NULL);
 }
 
-static int ricoh_mmc_suspend (struct pci_dev *pdev, pm_message_t state)
+static int ricoh_mmc_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct pci_dev *fw_dev = NULL;
 
@@ -146,7 +205,7 @@ static int ricoh_mmc_suspend (struct pci
 	return 0;
 }
 
-static int ricoh_mmc_resume (struct pci_dev *pdev)
+static int ricoh_mmc_resume(struct pci_dev *pdev)
 {
 	struct pci_dev *fw_dev = NULL;
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed)
  2008-02-04 18:25   ` [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed) Frank Seidel
@ 2008-02-04 20:17     ` Philip Langdale
  0 siblings, 0 replies; 11+ messages in thread
From: Philip Langdale @ 2008-02-04 20:17 UTC (permalink / raw)
  To: Frank Seidel; +Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey


On Mon, 4 Feb 2008 19:25:38 +0100, Frank Seidel <fseidel@suse.de> wrote:
> From: Philip Langdale <philipl@overt.org>
> 
> As pci config space is reinitialised on suspend/resume cycle, the
> disabler needs to work its magic at resume time. For symmetry this
> change also explicitly enables the controller at suspend time but
> it's not strictly necessary.
> 
> Signed-off-by: Philip Langdale <philipl@overt.org>
> Signed-off-by: Frank Seidel <fseidel@suse.de>
> ---
>  drivers/mmc/host/ricoh_mmc.c |   97
> +++++++++++++++++++++++++++++++------------
>  1 file changed, 72 insertions(+), 25 deletions(-)
> 
> --- a/drivers/mmc/host/ricoh_mmc.c
> +++ b/drivers/mmc/host/ricoh_mmc.c
> @@ -41,6 +41,46 @@ static const struct pci_device_id pci_id
> 
>  MODULE_DEVICE_TABLE(pci, pci_ids);
> 
> +static int ricoh_mmc_disable(struct pci_dev *fw_dev)
> +{
> +	u8 write_enable;
> +	u8 disable;
> +
> +	pci_read_config_byte(fw_dev, 0xCB, &disable);
> +	if (disable & 0x02) {
> +		printk(KERN_INFO DRIVER_NAME
> +		       ": Controller already disabled. Nothing to do.\n");
> +		return -ENODEV;
> +	}
> +
> +	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> +	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> +	pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
> +	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +
> +	printk(KERN_INFO DRIVER_NAME
> +	       ": Controller is now disabled.\n");
> +
> +	return 0;
> +}
> +
> +static int ricoh_mmc_enable(struct pci_dev *fw_dev)
> +{
> +	u8 write_enable;
> +	u8 disable;
> +
> +	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> +	pci_read_config_byte(fw_dev, 0xCB, &disable);
> +	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> +	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
> +	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +
> +	printk(KERN_INFO DRIVER_NAME
> +	       ": Controller is now re-enabled.\n");
> +
> +	return 0;
> +}
> +
>  static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
>  				     const struct pci_device_id *ent)
>  {
> @@ -61,26 +101,12 @@ static int __devinit ricoh_mmc_probe(str
>  	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
>  		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
>  		    pdev->bus == fw_dev->bus) {
> -			u8 write_enable;
> -			u8 disable;
> -
> -			pci_read_config_byte(fw_dev, 0xCB, &disable);
> -			if (disable & 0x02) {
> -				printk(KERN_INFO DRIVER_NAME
> -				       ": Controller already disabled. Nothing to do.\n");
> +			if (ricoh_mmc_disable(fw_dev) != 0) {
>  				return -ENODEV;
>  			}
> 
> -			pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> -			pci_write_config_byte(fw_dev, 0xCA, 0x57);
> -			pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
> -			pci_write_config_byte(fw_dev, 0xCA, write_enable);
> -
>  			pci_set_drvdata(pdev, fw_dev);
> 
> -			printk(KERN_INFO DRIVER_NAME
> -			       ": Controller is now disabled.\n");
> -
>  			break;
>  		}
>  	}
> @@ -96,30 +122,51 @@ static int __devinit ricoh_mmc_probe(str
> 
>  static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
>  {
> -	u8 write_enable;
> -	u8 disable;
>  	struct pci_dev *fw_dev = NULL;
> 
>  	fw_dev = pci_get_drvdata(pdev);
>  	BUG_ON(fw_dev == NULL);
> 
> -	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> -	pci_read_config_byte(fw_dev, 0xCB, &disable);
> -	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> -	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
> -	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> -
> -	printk(KERN_INFO DRIVER_NAME
> -	       ": Controller is now re-enabled.\n");
> +	ricoh_mmc_enable(fw_dev);
> 
>  	pci_set_drvdata(pdev, NULL);
>  }
> 
> +static int ricoh_mmc_suspend (struct pci_dev *pdev, pm_message_t state)
> +{
> +	struct pci_dev *fw_dev = NULL;
> +
> +	fw_dev = pci_get_drvdata(pdev);
> +	BUG_ON(fw_dev == NULL);
> +
> +	printk(KERN_INFO DRIVER_NAME ": Suspending.\n");
> +
> +	ricoh_mmc_enable(fw_dev);
> +
> +	return 0;
> +}
> +
> +static int ricoh_mmc_resume (struct pci_dev *pdev)
> +{
> +	struct pci_dev *fw_dev = NULL;
> +
> +	fw_dev = pci_get_drvdata(pdev);
> +	BUG_ON(fw_dev == NULL);
> +
> +	printk(KERN_INFO DRIVER_NAME ": Resuming.\n");
> +
> +	ricoh_mmc_disable(fw_dev);
> +
> +	return 0;
> +}
> +
>  static struct pci_driver ricoh_mmc_driver = {
>  	.name = 	DRIVER_NAME,
>  	.id_table =	pci_ids,
>  	.probe = 	ricoh_mmc_probe,
>  	.remove =	__devexit_p(ricoh_mmc_remove),
> +	.suspend =	ricoh_mmc_suspend,
> +	.resume =	ricoh_mmc_resume,
>  };
> 
> 
>
/*****************************************************************************\

ACK.

Thanks for fixing it up.

--phil


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-04 18:25   ` [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
@ 2008-02-04 20:18     ` Philip Langdale
  2008-02-07  8:08     ` Pierre Ossman
  1 sibling, 0 replies; 11+ messages in thread
From: Philip Langdale @ 2008-02-04 20:18 UTC (permalink / raw)
  To: Frank Seidel; +Cc: sdhci-devel, linux-kernel, Pierre Ossman, Andrew de Quincey


On Mon, 4 Feb 2008 19:25:42 +0100, Frank Seidel <fseidel@suse.de> wrote:
> From: Frank Seidel <fseidel@suse.de>
> 
> This patch (base on current linus git tree plus Philip Langdales
> suspend/resume patch) adds support for the Ricoh RL5c476 chip:
> with this the mmc adapter that needs this disabler (R5C843) can
> also be handled correctly when it sits on a RL5c476.
> (+ minor style changes (removed spaces between function names
> and open parenthesis .. checkpatch warned from previos patch))
> 
> Signed-off-by: Frank Seidel <fseidel@suse.de>
> ---
>  drivers/mmc/host/ricoh_mmc.c |  101
> ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 80 insertions(+), 21 deletions(-)
> 
> --- a/drivers/mmc/host/ricoh_mmc.c
> +++ b/drivers/mmc/host/ricoh_mmc.c
> @@ -44,19 +44,43 @@ MODULE_DEVICE_TABLE(pci, pci_ids);
>  static int ricoh_mmc_disable(struct pci_dev *fw_dev)
>  {
>  	u8 write_enable;
> +	u8 write_target;
>  	u8 disable;
> 
> -	pci_read_config_byte(fw_dev, 0xCB, &disable);
> -	if (disable & 0x02) {
> -		printk(KERN_INFO DRIVER_NAME
> -		       ": Controller already disabled. Nothing to do.\n");
> -		return -ENODEV;
> -	}
> +	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
> +		/* via RL5C476 */
> +
> +		pci_read_config_byte(fw_dev, 0xB7, &disable);
> +		if (disable & 0x02) {
> +			printk(KERN_INFO DRIVER_NAME
> +				": Controller already disabled. " \
> +				"Nothing to do.\n");
> +			return -ENODEV;
> +		}
> +
> +		pci_read_config_byte(fw_dev, 0x8E, &write_enable);
> +		pci_write_config_byte(fw_dev, 0x8E, 0xAA);
> +		pci_read_config_byte(fw_dev, 0x8D, &write_target);
> +		pci_write_config_byte(fw_dev, 0x8D, 0xB7);
> +		pci_write_config_byte(fw_dev, 0xB7, disable | 0x02);
> +		pci_write_config_byte(fw_dev, 0x8E, write_enable);
> +		pci_write_config_byte(fw_dev, 0x8D, write_target);
> +	} else {
> +		/* via R5C832 */
> +
> +		pci_read_config_byte(fw_dev, 0xCB, &disable);
> +		if (disable & 0x02) {
> +			printk(KERN_INFO DRIVER_NAME
> +			       ": Controller already disabled. " \
> +				"Nothing to do.\n");
> +			return -ENODEV;
> +		}
> 
> -	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> -	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> -	pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
> -	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +		pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> +		pci_write_config_byte(fw_dev, 0xCA, 0x57);
> +		pci_write_config_byte(fw_dev, 0xCB, disable | 0x02);
> +		pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +	}
> 
>  	printk(KERN_INFO DRIVER_NAME
>  	       ": Controller is now disabled.\n");
> @@ -67,13 +91,29 @@ static int ricoh_mmc_disable(struct pci_
>  static int ricoh_mmc_enable(struct pci_dev *fw_dev)
>  {
>  	u8 write_enable;
> +	u8 write_target;
>  	u8 disable;
> 
> -	pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> -	pci_read_config_byte(fw_dev, 0xCB, &disable);
> -	pci_write_config_byte(fw_dev, 0xCA, 0x57);
> -	pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
> -	pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
> +		/* via RL5C476 */
> +
> +		pci_read_config_byte(fw_dev, 0x8E, &write_enable);
> +		pci_write_config_byte(fw_dev, 0x8E, 0xAA);
> +		pci_read_config_byte(fw_dev, 0x8D, &write_target);
> +		pci_write_config_byte(fw_dev, 0x8D, 0xB7);
> +		pci_read_config_byte(fw_dev, 0xB7, &disable);
> +		pci_write_config_byte(fw_dev, 0xB7, disable & ~0x02);
> +		pci_write_config_byte(fw_dev, 0x8E, write_enable);
> +		pci_write_config_byte(fw_dev, 0x8D, write_target);
> +	} else {
> +		/* via R5C832 */
> +
> +		pci_read_config_byte(fw_dev, 0xCA, &write_enable);
> +		pci_read_config_byte(fw_dev, 0xCB, &disable);
> +		pci_write_config_byte(fw_dev, 0xCA, 0x57);
> +		pci_write_config_byte(fw_dev, 0xCB, disable & ~0x02);
> +		pci_write_config_byte(fw_dev, 0xCA, write_enable);
> +	}
> 
>  	printk(KERN_INFO DRIVER_NAME
>  	       ": Controller is now re-enabled.\n");
> @@ -85,6 +125,7 @@ static int __devinit ricoh_mmc_probe(str
>  				     const struct pci_device_id *ent)
>  {
>  	u8 rev;
> +	u8 ctrlfound = 0;
> 
>  	struct pci_dev *fw_dev = NULL;
> 
> @@ -98,20 +139,38 @@ static int __devinit ricoh_mmc_probe(str
>  		pci_name(pdev), (int)pdev->vendor, (int)pdev->device,
>  		(int)rev);
> 
> -	while ((fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
> +	while ((fw_dev =
> +		pci_get_device(PCI_VENDOR_ID_RICOH,
> +			PCI_DEVICE_ID_RICOH_RL5C476, fw_dev))) {
>  		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
>  		    pdev->bus == fw_dev->bus) {
> -			if (ricoh_mmc_disable(fw_dev) != 0) {
> +			if (ricoh_mmc_disable(fw_dev) != 0)
>  				return -ENODEV;
> -			}
> 
>  			pci_set_drvdata(pdev, fw_dev);
> 
> +			++ctrlfound;
>  			break;
>  		}
>  	}
> 
> -	if (pci_get_drvdata(pdev) == NULL) {
> +	fw_dev = NULL;
> +
> +	while (!ctrlfound &&
> +	    (fw_dev = pci_get_device(PCI_VENDOR_ID_RICOH,
> +					PCI_DEVICE_ID_RICOH_R5C832, fw_dev))) {
> +		if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
> +		    pdev->bus == fw_dev->bus) {
> +			if (ricoh_mmc_disable(fw_dev) != 0)
> +				return -ENODEV;
> +
> +			pci_set_drvdata(pdev, fw_dev);
> +
> +			++ctrlfound;
> +		}
> +	}
> +
> +	if (!ctrlfound) {
>  		printk(KERN_WARNING DRIVER_NAME
>  		       ": Main firewire function not found. Cannot disable
> controller.\n");
>  		return -ENODEV;
> @@ -132,7 +191,7 @@ static void __devexit ricoh_mmc_remove(s
>  	pci_set_drvdata(pdev, NULL);
>  }
> 
> -static int ricoh_mmc_suspend (struct pci_dev *pdev, pm_message_t state)
> +static int ricoh_mmc_suspend(struct pci_dev *pdev, pm_message_t state)
>  {
>  	struct pci_dev *fw_dev = NULL;
> 
> @@ -146,7 +205,7 @@ static int ricoh_mmc_suspend (struct pci
>  	return 0;
>  }
> 
> -static int ricoh_mmc_resume (struct pci_dev *pdev)
> +static int ricoh_mmc_resume(struct pci_dev *pdev)
>  {
>  	struct pci_dev *fw_dev = NULL;

ACK.

Hopefully Pierre will re-emerge soon to accept this into his tree.

--phil


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-04 18:25   ` [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
  2008-02-04 20:18     ` Philip Langdale
@ 2008-02-07  8:08     ` Pierre Ossman
  2008-02-07  8:20       ` Frank Seidel
  1 sibling, 1 reply; 11+ messages in thread
From: Pierre Ossman @ 2008-02-07  8:08 UTC (permalink / raw)
  To: Frank Seidel
  Cc: Philip Langdale, sdhci-devel, linux-kernel, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

On Mon, 4 Feb 2008 19:25:42 +0100
Frank Seidel <fseidel@suse.de> wrote:

> From: Frank Seidel <fseidel@suse.de>
> 
> This patch (base on current linus git tree plus Philip Langdales
> suspend/resume patch) adds support for the Ricoh RL5c476 chip:
> with this the mmc adapter that needs this disabler (R5C843) can
> also be handled correctly when it sits on a RL5c476.
> (+ minor style changes (removed spaces between function names
> and open parenthesis .. checkpatch warned from previos patch))
> 
> Signed-off-by: Frank Seidel <fseidel@suse.de>

I see you've guys have kept yourself busy in my absence. :)

As for the patch, it looks ok although I'm not really a fan of more voodoo constants that noone knows what they mean. Could you add some comments explaining some of them at least?

> +	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {

*snip*

> +	} else {
> +		/* via R5C832 */

Wouldn't it be prudent to have a check that this is indeed a R5C832, and a failure mode if it's none of the two known devices?

Rgds
Pierre

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-07  8:08     ` Pierre Ossman
@ 2008-02-07  8:20       ` Frank Seidel
  2008-02-07 17:19         ` Pierre Ossman
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Seidel @ 2008-02-07  8:20 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: Philip Langdale, sdhci-devel, linux-kernel, Andrew de Quincey

Hi,

On Thursday 07 February 2008 09:08, Pierre Ossman wrote:
> On Mon, 4 Feb 2008 19:25:42 +0100
> Frank Seidel <fseidel@suse.de> wrote:
> > Signed-off-by: Frank Seidel <fseidel@suse.de>
> 
> I see you've guys have kept yourself busy in my absence. :)

Yes, we really did :)

> As for the patch, it looks ok although I'm not really a fan of more
> voodoo constants that noone knows what they mean. Could you add some
> comments explaining some of them at least?  

I also don't like that voodoo, but as long as we don't have more information
or let alone specs.. well, i really wish i could provide more than
the already IMHO obvious sequence.. voodoo-adress and value to make config
bit writeable, set voodo-bit and cleanup again.

> > +	if (fw_dev->device == PCI_DEVICE_ID_RICOH_RL5C476) {
> 
> *snip*
> 
> > +	} else {
> > +		/* via R5C832 */
> 
> Wouldn't it be prudent to have a check that this is indeed a R5C832,
> and a failure mode if it's none of the two known devices? 

Also thought about that, but as far as i can see this cannot happen since
we only probe for those two devices and deny to continue if anything else
/those two were not found in the beginning.

Thanks,
Frank

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476
  2008-02-07  8:20       ` Frank Seidel
@ 2008-02-07 17:19         ` Pierre Ossman
  0 siblings, 0 replies; 11+ messages in thread
From: Pierre Ossman @ 2008-02-07 17:19 UTC (permalink / raw)
  To: Frank Seidel
  Cc: Philip Langdale, sdhci-devel, linux-kernel, Andrew de Quincey

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Thu, 7 Feb 2008 09:20:38 +0100
Frank Seidel <fseidel@suse.de> wrote:

> > 
> > Wouldn't it be prudent to have a check that this is indeed a R5C832,
> > and a failure mode if it's none of the two known devices? 
> 
> Also thought about that, but as far as i can see this cannot happen since
> we only probe for those two devices and deny to continue if anything else
> /those two were not found in the beginning.
> 

Can never be too careful though. :)

I've applied the patch for now. Feel free to keep digging and finding some docs for those regs though.

Rgds
Pierre

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-02-07 17:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-31 17:38 [PATCH] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
2008-02-02  7:16 ` Philip Langdale
2008-02-02  9:30   ` Frank Seidel
2008-02-04 18:25   ` Frank Seidel
2008-02-04 18:25   ` [PATCH] mmc: Handle suspend/resume in Ricoh MMC disabler (resent refreshed) Frank Seidel
2008-02-04 20:17     ` Philip Langdale
2008-02-04 18:25   ` [PATCH v2] mmc: extend ricoh_mmc to support Ricoh RL5c476 Frank Seidel
2008-02-04 20:18     ` Philip Langdale
2008-02-07  8:08     ` Pierre Ossman
2008-02-07  8:20       ` Frank Seidel
2008-02-07 17:19         ` Pierre Ossman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).