Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] Port ricoh_mmc from driver to pci quirk.

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


maximlevitsky at gmail

Nov 25, 2009, 6:58 AM

Post #1 of 7 (163 views)
Permalink
[PATCH] Port ricoh_mmc from driver to pci quirk.

>From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky [at] gmail>
Date: Wed, 25 Nov 2009 16:37:46 +0200
Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
This is much cleaner and correct solution

Signed-off-by: Maxim Levitsky <maximlevitsky [at] gmail>
---
drivers/mmc/host/Kconfig | 17 ---
drivers/mmc/host/Makefile | 1 -
drivers/mmc/host/ricoh_mmc.c | 262 ------------------------------------------
drivers/pci/quirks.c | 77 ++++++++++++
4 files changed, 77 insertions(+), 280 deletions(-)
delete mode 100644 drivers/mmc/host/ricoh_mmc.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 432ae83..4edce18 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -55,23 +55,6 @@ config MMC_SDHCI_PCI

If unsure, say N.

-config MMC_RICOH_MMC
- tristate "Ricoh MMC Controller Disabler (EXPERIMENTAL)"
- depends on MMC_SDHCI_PCI
- help
- This selects the disabler for the Ricoh MMC Controller. This
- proprietary controller is unnecessary because the SDHCI driver
- supports MMC cards on the SD controller, but if it is not
- disabled, it will steal the MMC cards away - rendering them
- useless. It is safe to select this driver even if you don't
- have a Ricoh based card reader.
-
-
- To compile this driver as a module, choose M here:
- the module will be called ricoh_mmc.
-
- If unsure, say Y.
-
config MMC_SDHCI_OF
tristate "SDHCI support on OpenFirmware platforms"
depends on MMC_SDHCI && PPC_OF
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index abcb040..7b82394 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_MMC_IMX) += imxmmc.o
obj-$(CONFIG_MMC_MXC) += mxcmmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
-obj-$(CONFIG_MMC_RICOH_MMC) += ricoh_mmc.o
obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o
obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
diff --git a/drivers/mmc/host/ricoh_mmc.c b/drivers/mmc/host/ricoh_mmc.c
deleted file mode 100644
index f627905..0000000
--- a/drivers/mmc/host/ricoh_mmc.c
+++ /dev/null
@@ -1,262 +0,0 @@
-/*
- * ricoh_mmc.c - Dummy driver to disable the Rioch MMC controller.
- *
- * Copyright (C) 2007 Philip Langdale, All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- */
-
-/*
- * This is a conceptually ridiculous driver, but it is required by the way
- * the Ricoh multi-function chips (R5CXXX) work. These chips implement
- * the four main memory card controllers (SD, MMC, MS, xD) and one or both
- * of cardbus or firewire. It happens that they implement SD and MMC
- * support as separate controllers (and PCI functions). The linux SDHCI
- * driver supports MMC cards but the chip detects MMC cards in hardware
- * and directs them to the MMC controller - so the SDHCI driver never sees
- * them. To get around this, we must disable the useless MMC controller.
- * At that point, the SDHCI controller will start seeing them. As a bonus,
- * a detection event occurs immediately, even if the MMC card is already
- * in the reader.
- *
- * It seems to be the case that the relevant PCI registers to deactivate the
- * MMC controller live on PCI function 0, which might be the cardbus controller
- * or the firewire controller, depending on the particular chip in question. As
- * such, it makes what this driver has to do unavoidably ugly. Such is life.
- */
-
-#include <linux/pci.h>
-
-#define DRIVER_NAME "ricoh-mmc"
-
-static const struct pci_device_id pci_ids[] __devinitdata = {
- {
- .vendor = PCI_VENDOR_ID_RICOH,
- .device = PCI_DEVICE_ID_RICOH_R5C843,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- },
- { /* end: all zeroes */ },
-};
-
-MODULE_DEVICE_TABLE(pci, pci_ids);
-
-static int ricoh_mmc_disable(struct pci_dev *fw_dev)
-{
- u8 write_enable;
- u8 write_target;
- u8 disable;
-
- 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);
- }
-
- 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 write_target;
- u8 disable;
-
- 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");
-
- return 0;
-}
-
-static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
- const struct pci_device_id *ent)
-{
- u8 rev;
- u8 ctrlfound = 0;
-
- struct pci_dev *fw_dev = NULL;
-
- BUG_ON(pdev == NULL);
- BUG_ON(ent == NULL);
-
- pci_read_config_byte(pdev, PCI_CLASS_REVISION, &rev);
-
- printk(KERN_INFO DRIVER_NAME
- ": Ricoh MMC controller found at %s [%04x:%04x] (rev %x)\n",
- 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_RL5C476, fw_dev))) {
- if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn) &&
- PCI_FUNC(fw_dev->devfn) == 0 &&
- pdev->bus == fw_dev->bus) {
- if (ricoh_mmc_disable(fw_dev) != 0)
- return -ENODEV;
-
- pci_set_drvdata(pdev, fw_dev);
-
- ++ctrlfound;
- break;
- }
- }
-
- 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) &&
- PCI_FUNC(fw_dev->devfn) == 0 &&
- 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 Ricoh function not found. Cannot disable controller.\n");
- return -ENODEV;
- }
-
- return 0;
-}
-
-static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
-{
- struct pci_dev *fw_dev = NULL;
-
- fw_dev = pci_get_drvdata(pdev);
- BUG_ON(fw_dev == NULL);
-
- ricoh_mmc_enable(fw_dev);
-
- pci_set_drvdata(pdev, NULL);
-}
-
-static int ricoh_mmc_suspend_late(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_early(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_late = ricoh_mmc_suspend_late,
- .resume_early = ricoh_mmc_resume_early,
-};
-
-/*****************************************************************************\
- * *
- * Driver init/exit *
- * *
-\*****************************************************************************/
-
-static int __init ricoh_mmc_drv_init(void)
-{
- printk(KERN_INFO DRIVER_NAME
- ": Ricoh MMC Controller disabling driver\n");
- printk(KERN_INFO DRIVER_NAME ": Copyright(c) Philip Langdale\n");
-
- return pci_register_driver(&ricoh_mmc_driver);
-}
-
-static void __exit ricoh_mmc_drv_exit(void)
-{
- pci_unregister_driver(&ricoh_mmc_driver);
-}
-
-module_init(ricoh_mmc_drv_init);
-module_exit(ricoh_mmc_drv_exit);
-
-MODULE_AUTHOR("Philip Langdale <philipl [at] alumni>");
-MODULE_DESCRIPTION("Ricoh MMC Controller disabling driver");
-MODULE_LICENSE("GPL");
-
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 245d2cd..a53197e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2516,6 +2516,83 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x150d, quirk_i82576_sriov);

#endif /* CONFIG_PCI_IOV */

+/*
+ * This is a quirk for Ricoh MMC controller found as a part of
+ * multifunction chip.
+ * It is very similiar and based on ricoh_mmc driver written by Philip Langdale
+ * Thanks for these magic sequences.
+ * These chips implement the four main memory card
+ * controllers (SD, MMC, MS, xD) and one or both
+ * of cardbus or firewire. It happens that they implement SD and MMC
+ * support as separate controllers (and PCI functions). The linux SDHCI
+ * driver supports MMC cards but the chip detects MMC cards in hardware
+ * and directs them to the MMC controller - so the SDHCI driver never sees
+ * them. To get around this, we must disable the useless MMC controller.
+ * At that point, the SDHCI controller will start seeing them
+ * It seems to be the case that the relevant PCI registers to deactivate the
+ * MMC controller live on PCI function 0, which might be the cardbus controller
+ * or the firewire controller, depending on the particular chip in question
+ */
+
+static void ricoh_mmc_fixup_rl5c476(struct pci_dev *dev)
+{
+ /* disable via cardbus interface */
+ u8 write_enable;
+ u8 write_target;
+ u8 disable;
+
+ /* disable must be done via function #0 */
+ if (PCI_FUNC(dev->devfn))
+ return;
+
+ pci_read_config_byte(dev, 0xB7, &disable);
+ if (disable & 0x02)
+ return;
+
+ pci_read_config_byte(dev, 0x8E, &write_enable);
+ pci_write_config_byte(dev, 0x8E, 0xAA);
+ pci_read_config_byte(dev, 0x8D, &write_target);
+ pci_write_config_byte(dev, 0x8D, 0xB7);
+ pci_write_config_byte(dev, 0xB7, disable | 0x02);
+ pci_write_config_byte(dev, 0x8E, write_enable);
+ pci_write_config_byte(dev, 0x8D, write_target);
+}
+
+static void ricoh_mmc_fixup_r5c832(struct pci_dev *dev)
+{
+ /* disable via firewire interface */
+ u8 write_enable;
+ u8 disable;
+
+ /* disable must be done via function #0 */
+ if (PCI_FUNC(dev->devfn))
+ return;
+
+ pci_read_config_byte(dev, 0xCB, &disable);
+
+ if (disable & 0x02)
+ return;
+
+ pci_read_config_byte(dev, 0xCA, &write_enable);
+ pci_write_config_byte(dev, 0xCA, 0x57);
+ pci_write_config_byte(dev, 0xCB, disable | 0x02);
+ pci_write_config_byte(dev, 0xCA, write_enable);
+}
+
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_RL5C476,
+ ricoh_mmc_fixup_rl5c476);
+
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_RL5C476,
+ ricoh_mmc_fixup_rl5c476);
+
+
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832,
+ ricoh_mmc_fixup_r5c832);
+
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH, PCI_DEVICE_ID_RICOH_R5C832,
+ ricoh_mmc_fixup_r5c832);
+
+
static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
struct pci_fixup *end)
{
--
1.6.3.3



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


philipl at overt

Nov 25, 2009, 8:53 AM

Post #2 of 7 (153 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

On Wed, 25 Nov 2009 16:58:41 +0200
Maxim Levitsky <maximlevitsky [at] gmail> wrote:

> >From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00
> >2001
> From: Maxim Levitsky <maximlevitsky [at] gmail>
> Date: Wed, 25 Nov 2009 16:37:46 +0200
> Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
> This is much cleaner and correct solution

I'm fine with the concept, but when I originally started work on
Ricoh support, Pierre specifically didn't want a pci quirk.

Pierre wrote:
> I'd rather we didn't. The current style of quirks are bad enough,
> making them even more vendor or device specific is a bit more than I'm
> willing to accept right now (seriously, how hard can it be to follow
> the damn spec?).

Pierre's not officially the maintainer anymore but I still respect his
opinions here. Given that a pci quirk solves this problem so simply,
I think it's justified at this point.

Pierre, do you want to comment?

> Signed-off-by: Maxim Levitsky <maximlevitsky [at] gmail>
> ---
> drivers/mmc/host/Kconfig | 17 ---
> drivers/mmc/host/Makefile | 1 -
> drivers/mmc/host/ricoh_mmc.c | 262
> ------------------------------------------
> drivers/pci/quirks.c | 77 ++++++++++++ 4 files changed, 77
> insertions(+), 280 deletions(-) delete mode 100644
> drivers/mmc/host/ricoh_mmc.c
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 432ae83..4edce18 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -55,23 +55,6 @@ config MMC_SDHCI_PCI
>
> If unsure, say N.
>
> -config MMC_RICOH_MMC
> - tristate "Ricoh MMC Controller Disabler (EXPERIMENTAL)"
> - depends on MMC_SDHCI_PCI
> - help
> - This selects the disabler for the Ricoh MMC Controller.
> This
> - proprietary controller is unnecessary because the SDHCI
> driver
> - supports MMC cards on the SD controller, but if it is not
> - disabled, it will steal the MMC cards away - rendering them
> - useless. It is safe to select this driver even if you don't
> - have a Ricoh based card reader.
> -
> -
> - To compile this driver as a module, choose M here:
> - the module will be called ricoh_mmc.
> -
> - If unsure, say Y.
> -
> config MMC_SDHCI_OF
> tristate "SDHCI support on OpenFirmware platforms"
> depends on MMC_SDHCI && PPC_OF
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index abcb040..7b82394 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_MMC_IMX) += imxmmc.o
> obj-$(CONFIG_MMC_MXC) += mxcmmc.o
> obj-$(CONFIG_MMC_SDHCI) += sdhci.o
> obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
> -obj-$(CONFIG_MMC_RICOH_MMC) += ricoh_mmc.o
> obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o
> obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
> obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o
> diff --git a/drivers/mmc/host/ricoh_mmc.c
> b/drivers/mmc/host/ricoh_mmc.c deleted file mode 100644
> index f627905..0000000
> --- a/drivers/mmc/host/ricoh_mmc.c
> +++ /dev/null
> @@ -1,262 +0,0 @@
> -/*
> - * ricoh_mmc.c - Dummy driver to disable the Rioch MMC controller.
> - *
> - * Copyright (C) 2007 Philip Langdale, All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or
> modify
> - * it under the terms of the GNU General Public License as published
> by
> - * the Free Software Foundation; either version 2 of the License, or
> (at
> - * your option) any later version.
> - */
> -
> -/*
> - * This is a conceptually ridiculous driver, but it is required by
> the way
> - * the Ricoh multi-function chips (R5CXXX) work. These chips
> implement
> - * the four main memory card controllers (SD, MMC, MS, xD) and one
> or both
> - * of cardbus or firewire. It happens that they implement SD and MMC
> - * support as separate controllers (and PCI functions). The linux
> SDHCI
> - * driver supports MMC cards but the chip detects MMC cards in
> hardware
> - * and directs them to the MMC controller - so the SDHCI driver
> never sees
> - * them. To get around this, we must disable the useless MMC
> controller.
> - * At that point, the SDHCI controller will start seeing them. As a
> bonus,
> - * a detection event occurs immediately, even if the MMC card is
> already
> - * in the reader.
> - *
> - * It seems to be the case that the relevant PCI registers to
> deactivate the
> - * MMC controller live on PCI function 0, which might be the cardbus
> controller
> - * or the firewire controller, depending on the particular chip in
> question. As
> - * such, it makes what this driver has to do unavoidably ugly. Such
> is life.
> - */
> -
> -#include <linux/pci.h>
> -
> -#define DRIVER_NAME "ricoh-mmc"
> -
> -static const struct pci_device_id pci_ids[] __devinitdata = {
> - {
> - .vendor = PCI_VENDOR_ID_RICOH,
> - .device = PCI_DEVICE_ID_RICOH_R5C843,
> - .subvendor = PCI_ANY_ID,
> - .subdevice = PCI_ANY_ID,
> - },
> - { /* end: all zeroes */ },
> -};
> -
> -MODULE_DEVICE_TABLE(pci, pci_ids);
> -
> -static int ricoh_mmc_disable(struct pci_dev *fw_dev)
> -{
> - u8 write_enable;
> - u8 write_target;
> - u8 disable;
> -
> - 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);
> - }
> -
> - 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 write_target;
> - u8 disable;
> -
> - 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");
> -
> - return 0;
> -}
> -
> -static int __devinit ricoh_mmc_probe(struct pci_dev *pdev,
> - const struct pci_device_id *ent)
> -{
> - u8 rev;
> - u8 ctrlfound = 0;
> -
> - struct pci_dev *fw_dev = NULL;
> -
> - BUG_ON(pdev == NULL);
> - BUG_ON(ent == NULL);
> -
> - pci_read_config_byte(pdev, PCI_CLASS_REVISION, &rev);
> -
> - printk(KERN_INFO DRIVER_NAME
> - ": Ricoh MMC controller found at %s [%04x:%04x] (rev
> %x)\n",
> - 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_RL5C476, fw_dev))) {
> - if (PCI_SLOT(pdev->devfn) == PCI_SLOT(fw_dev->devfn)
> &&
> - PCI_FUNC(fw_dev->devfn) == 0 &&
> - pdev->bus == fw_dev->bus) {
> - if (ricoh_mmc_disable(fw_dev) != 0)
> - return -ENODEV;
> -
> - pci_set_drvdata(pdev, fw_dev);
> -
> - ++ctrlfound;
> - break;
> - }
> - }
> -
> - 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)
> &&
> - PCI_FUNC(fw_dev->devfn) == 0 &&
> - 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 Ricoh function not found. Cannot
> disable controller.\n");
> - return -ENODEV;
> - }
> -
> - return 0;
> -}
> -
> -static void __devexit ricoh_mmc_remove(struct pci_dev *pdev)
> -{
> - struct pci_dev *fw_dev = NULL;
> -
> - fw_dev = pci_get_drvdata(pdev);
> - BUG_ON(fw_dev == NULL);
> -
> - ricoh_mmc_enable(fw_dev);
> -
> - pci_set_drvdata(pdev, NULL);
> -}
> -
> -static int ricoh_mmc_suspend_late(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_early(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_late = ricoh_mmc_suspend_late,
> - .resume_early = ricoh_mmc_resume_early,
> -};
> -
> -/*****************************************************************************\
> -
> *
> *
> - * Driver
> init/exit *
> -
> *
> *
> -\*****************************************************************************/
> - -static int __init ricoh_mmc_drv_init(void) -{
> - printk(KERN_INFO DRIVER_NAME
> - ": Ricoh MMC Controller disabling driver\n");
> - printk(KERN_INFO DRIVER_NAME ": Copyright(c) Philip
> Langdale\n"); -
> - return pci_register_driver(&ricoh_mmc_driver);
> -}
> -
> -static void __exit ricoh_mmc_drv_exit(void)
> -{
> - pci_unregister_driver(&ricoh_mmc_driver);
> -}
> -
> -module_init(ricoh_mmc_drv_init);
> -module_exit(ricoh_mmc_drv_exit);
> -
> -MODULE_AUTHOR("Philip Langdale <philipl [at] alumni>");
> -MODULE_DESCRIPTION("Ricoh MMC Controller disabling driver");
> -MODULE_LICENSE("GPL");
> -
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 245d2cd..a53197e 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2516,6 +2516,83 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,
> 0x150d, quirk_i82576_sriov);
> #endif /* CONFIG_PCI_IOV */
>
> +/*
> + * This is a quirk for Ricoh MMC controller found as a part of
> + * multifunction chip.
> + * It is very similiar and based on ricoh_mmc driver written by
> Philip Langdale
> + * Thanks for these magic sequences.
> + * These chips implement the four main memory card
> + * controllers (SD, MMC, MS, xD) and one or both
> + * of cardbus or firewire. It happens that they implement SD and MMC
> + * support as separate controllers (and PCI functions). The linux
> SDHCI
> + * driver supports MMC cards but the chip detects MMC cards in
> hardware
> + * and directs them to the MMC controller - so the SDHCI driver
> never sees
> + * them. To get around this, we must disable the useless MMC
> controller.
> + * At that point, the SDHCI controller will start seeing them
> + * It seems to be the case that the relevant PCI registers to
> deactivate the
> + * MMC controller live on PCI function 0, which might be the cardbus
> controller
> + * or the firewire controller, depending on the particular chip in
> question
> + */
> +
> +static void ricoh_mmc_fixup_rl5c476(struct pci_dev *dev)
> +{
> + /* disable via cardbus interface */
> + u8 write_enable;
> + u8 write_target;
> + u8 disable;
> +
> + /* disable must be done via function #0 */
> + if (PCI_FUNC(dev->devfn))
> + return;
> +
> + pci_read_config_byte(dev, 0xB7, &disable);
> + if (disable & 0x02)
> + return;
> +
> + pci_read_config_byte(dev, 0x8E, &write_enable);
> + pci_write_config_byte(dev, 0x8E, 0xAA);
> + pci_read_config_byte(dev, 0x8D, &write_target);
> + pci_write_config_byte(dev, 0x8D, 0xB7);
> + pci_write_config_byte(dev, 0xB7, disable | 0x02);
> + pci_write_config_byte(dev, 0x8E, write_enable);
> + pci_write_config_byte(dev, 0x8D, write_target);
> +}
> +
> +static void ricoh_mmc_fixup_r5c832(struct pci_dev *dev)
> +{
> + /* disable via firewire interface */
> + u8 write_enable;
> + u8 disable;
> +
> + /* disable must be done via function #0 */
> + if (PCI_FUNC(dev->devfn))
> + return;
> +
> + pci_read_config_byte(dev, 0xCB, &disable);
> +
> + if (disable & 0x02)
> + return;
> +
> + pci_read_config_byte(dev, 0xCA, &write_enable);
> + pci_write_config_byte(dev, 0xCA, 0x57);
> + pci_write_config_byte(dev, 0xCB, disable | 0x02);
> + pci_write_config_byte(dev, 0xCA, write_enable);
> +}
> +
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_RL5C476,
> + ricoh_mmc_fixup_rl5c476);
> +
> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_RL5C476,
> + ricoh_mmc_fixup_rl5c476);
> +
> +
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_R5C832,
> + ricoh_mmc_fixup_r5c832);
> +
> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_RICOH,
> PCI_DEVICE_ID_RICOH_R5C832,
> + ricoh_mmc_fixup_r5c832);
> +
> +
> static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
> struct pci_fixup *end)
> {

I would at least suggest a printk so ensure people know this is
happening - otherwise there's no visible evidence that the system
even has an MMC controller that's been disabled.

Thanks,

--phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


akpm at linux-foundation

Nov 25, 2009, 11:35 AM

Post #3 of 7 (159 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

On Wed, 25 Nov 2009 08:53:24 -0800
Philip Langdale <philipl [at] overt> wrote:

> On Wed, 25 Nov 2009 16:58:41 +0200
> Maxim Levitsky <maximlevitsky [at] gmail> wrote:
>
> > >From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00
> > >2001
> > From: Maxim Levitsky <maximlevitsky [at] gmail>
> > Date: Wed, 25 Nov 2009 16:37:46 +0200
> > Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
> > This is much cleaner and correct solution

This patch actually fixes a bug but the changelog forgot to tell us
this important fact.

> I'm fine with the concept, but when I originally started work on
> Ricoh support, Pierre specifically didn't want a pci quirk.
>
> Pierre wrote:
> > I'd rather we didn't. The current style of quirks are bad enough,
> > making them even more vendor or device specific is a bit more than I'm
> > willing to accept right now (seriously, how hard can it be to follow
> > the damn spec?).

Can the bug be fixed by other means, within ricoh_mmc.c?

It's a bit sad to add a lump of code to everyone's kernel like this -
what percentage of those machines actually have a ricoh mmc controller?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


pierre at ossman

Nov 25, 2009, 12:30 PM

Post #4 of 7 (150 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

On Wed, 25 Nov 2009 08:53:24 -0800
Philip Langdale <philipl [at] overt> wrote:

> On Wed, 25 Nov 2009 16:58:41 +0200
> Maxim Levitsky <maximlevitsky [at] gmail> wrote:
>
> > >From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00
> > >2001
> > From: Maxim Levitsky <maximlevitsky [at] gmail>
> > Date: Wed, 25 Nov 2009 16:37:46 +0200
> > Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
> > This is much cleaner and correct solution
>
> I'm fine with the concept, but when I originally started work on
> Ricoh support, Pierre specifically didn't want a pci quirk.
>
> Pierre wrote:
> > I'd rather we didn't. The current style of quirks are bad enough,
> > making them even more vendor or device specific is a bit more than I'm
> > willing to accept right now (seriously, how hard can it be to follow
> > the damn spec?).
>
> Pierre's not officially the maintainer anymore but I still respect his
> opinions here. Given that a pci quirk solves this problem so simply,
> I think it's justified at this point.
>
> Pierre, do you want to comment?
>

I was talking about the quirks mechanism in sdhci, not pci quirks. :)

I have no objections to this patch.

>
> I would at least suggest a printk so ensure people know this is
> happening - otherwise there's no visible evidence that the system
> even has an MMC controller that's been disabled.
>

Agreed. There might be cases where we cause problems since we don't
fully understand this hardware.

Rgds
--
-- Pierre Ossman

WARNING: This correspondence is being monitored by FRA, a
Swedish intelligence agency. Make sure your server uses
encryption for SMTP traffic and consider using PGP for
end-to-end encryption.
Attachments: signature.asc (0.19 KB)


maximlevitsky at gmail

Nov 25, 2009, 3:40 PM

Post #5 of 7 (149 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

On Wed, 2009-11-25 at 11:35 -0800, Andrew Morton wrote:
> On Wed, 25 Nov 2009 08:53:24 -0800
> Philip Langdale <philipl [at] overt> wrote:
>
> > On Wed, 25 Nov 2009 16:58:41 +0200
> > Maxim Levitsky <maximlevitsky [at] gmail> wrote:
> >
> > > >From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00
> > > >2001
> > > From: Maxim Levitsky <maximlevitsky [at] gmail>
> > > Date: Wed, 25 Nov 2009 16:37:46 +0200
> > > Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
> > > This is much cleaner and correct solution
>
> This patch actually fixes a bug but the changelog forgot to tell us
> this important fact.
>
> > I'm fine with the concept, but when I originally started work on
> > Ricoh support, Pierre specifically didn't want a pci quirk.
> >
> > Pierre wrote:
> > > I'd rather we didn't. The current style of quirks are bad enough,
> > > making them even more vendor or device specific is a bit more than I'm
> > > willing to accept right now (seriously, how hard can it be to follow
> > > the damn spec?).
>
> Can the bug be fixed by other means, within ricoh_mmc.c?
>
> It's a bit sad to add a lump of code to everyone's kernel like this -
> what percentage of those machines actually have a ricoh mmc controller?

You have valid point here.
However let me explain the situation:

We have a device with 5 functions,
One of the functions is propertary MMC contoller that writing driver for
will be a coolosal waste of time, because it can be disabled.

However, when it is disabled, it actually vanishes, and all pci
functions that belong to controller decrease by one.

Example:
07:00.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller (rev 05)
07:00.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 22)
07:00.2 System peripheral: Ricoh Co Ltd R5C843 MMC Host Controller (rev 12)
07:00.3 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 12)
07:00.4 System peripheral: Ricoh Co Ltd xD-Picture Card Controller (rev ff)

However the correct listing is:
07:00.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller (rev 05)
07:00.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 22)
07:00.2 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 12)
07:00.3 System peripheral: Ricoh Co Ltd xD-Picture Card Controller (rev 12)


PCI core can handle hotplug, but it sure can't handle the sudden shift
in function numbers.

Thus the disable step should be done before it enumerates the device, as
my patch does, and unfortunately this can't be module.
Up to now this driver did work, because both functions whose numbers are
affected didn't have a driver, but I am going soon to implement one
driver, and maybe another too.

Of course, I can put that under config condition, that users that are
sure that have no such device could save few hundreds of bytes.
However a distro probably will enable this option.

I also agree that I need to print some notice to user about device being
disabled.

Aside from being almost an 1:1 copy of original driver, this patch was
sort of RFC, so I update it soon.

Best regards,
Maxim Levitsky

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


philipl at overt

Nov 25, 2009, 5:30 PM

Post #6 of 7 (153 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

On Thu, 26 Nov 2009 01:40:01 +0200
Maxim Levitsky <maximlevitsky [at] gmail> wrote:


> Of course, I can put that under config condition, that users that are
> sure that have no such device could save few hundreds of bytes.
> However a distro probably will enable this option.

Yeah, this is the only way I can think of to make it optional - and
i guess we could actually reuse the current CONFIG value to make it
transparent. I kind of like this, but there's no precedent for it
in PCI quirks as far as I can tell.

> I also agree that I need to print some notice to user about device
> being disabled.
>
> Aside from being almost an 1:1 copy of original driver, this patch was
> sort of RFC, so I update it soon.

Thanks!

--phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


maximlevitsky at gmail

Nov 26, 2009, 3:51 PM

Post #7 of 7 (145 views)
Permalink
Re: [PATCH] Port ricoh_mmc from driver to pci quirk. [In reply to]

I send now an updated version, and hope it addresses most of the issues.
I added proper changelog, put the quirk under kconfig define (same as
was before) and added a message to kernel log about the disablement.

I must note that this wasn't tested with real mmc card.
It should work correctly though.

What does work for sure is that MMC device disappears just like it did
before, but now pci understands this.

Best regards,
Maxim Levitsky

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.