* [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
* 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
* [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 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