login
Search
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 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 --phil --
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 yes, also fully agree .. sadfully i was in a bit of a hurry that day needing a working Hehe, no, not here ... its "-", "+" ;-) Thanks, Frank --
Hi, 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 --
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 .../*****************************************************************************\ ACK. Thanks for fixing it up. --phil --
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, ...On Mon, 4 Feb 2008 19:25:42 +0100 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 expl= 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
Hi, 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 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 --
On Thu, 7 Feb 2008 09:20:38 +0100 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