* [PATCH 0/3] Add SDHCI ACPI driver
@ 2012-11-22 8:43 Adrian Hunter
2012-11-22 8:43 ` [PATCH 1/3] PNPACPI: exclude SDHCI devices Adrian Hunter
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Adrian Hunter @ 2012-11-22 8:43 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel
Hi
Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support
so I suggest Rafael takes the patches with Chris' Ack.
Please note that I would prefer this to be queued for 3.8
Adrian Hunter (3):
PNPACPI: exclude SDHCI devices
ACPI: add SDHCI to ACPI platform devices
mmc: sdhci-acpi: add SDHCI ACPI driver
drivers/acpi/scan.c | 2 +
drivers/mmc/host/Kconfig | 12 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-acpi.c | 304 ++++++++++++++++++++++++++++++++++++++++++
drivers/pnp/pnpacpi/core.c | 1 +
5 files changed, 320 insertions(+)
create mode 100644 drivers/mmc/host/sdhci-acpi.c
Regards
Adrian Hunter
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] PNPACPI: exclude SDHCI devices 2012-11-22 8:43 [PATCH 0/3] Add SDHCI ACPI driver Adrian Hunter @ 2012-11-22 8:43 ` Adrian Hunter 2012-11-22 8:43 ` [PATCH 2/3] ACPI: add SDHCI to ACPI platform devices Adrian Hunter ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Adrian Hunter @ 2012-11-22 8:43 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel These devices will be handled by a ACPI Platform driver. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/pnp/pnpacpi/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 26b5d4b..5b17cc8 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -43,6 +43,7 @@ static struct acpi_device_id excluded_id_list[] __initdata = { {"PNP0C0F", 0}, /* Link device */ {"PNP0000", 0}, /* PIC */ {"PNP0100", 0}, /* Timer */ + {"PNP0D40", 0}, /* SDHCI */ {"", 0}, }; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] ACPI: add SDHCI to ACPI platform devices 2012-11-22 8:43 [PATCH 0/3] Add SDHCI ACPI driver Adrian Hunter 2012-11-22 8:43 ` [PATCH 1/3] PNPACPI: exclude SDHCI devices Adrian Hunter @ 2012-11-22 8:43 ` Adrian Hunter 2012-11-22 8:43 ` [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver Adrian Hunter 2012-11-22 13:55 ` [PATCH 0/3] Add " Chris Ball 3 siblings, 0 replies; 13+ messages in thread From: Adrian Hunter @ 2012-11-22 8:43 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/acpi/scan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 8c4ac6d..67a7fa6 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -35,6 +35,8 @@ static const char *dummy_hid = "device"; */ static const struct acpi_device_id acpi_platform_device_ids[] = { + { "PNP0D40" }, + { } }; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver 2012-11-22 8:43 [PATCH 0/3] Add SDHCI ACPI driver Adrian Hunter 2012-11-22 8:43 ` [PATCH 1/3] PNPACPI: exclude SDHCI devices Adrian Hunter 2012-11-22 8:43 ` [PATCH 2/3] ACPI: add SDHCI to ACPI platform devices Adrian Hunter @ 2012-11-22 8:43 ` Adrian Hunter 2012-11-23 9:44 ` Mika Westerberg 2012-11-22 13:55 ` [PATCH 0/3] Add " Chris Ball 3 siblings, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-11-22 8:43 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/host/Kconfig | 12 ++ drivers/mmc/host/Makefile | 1 + drivers/mmc/host/sdhci-acpi.c | 304 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 317 insertions(+) create mode 100644 drivers/mmc/host/sdhci-acpi.c diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 9bf10e7..56eac10 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -81,6 +81,18 @@ config MMC_RICOH_MMC If unsure, say Y. +config MMC_SDHCI_ACPI + tristate "SDHCI support for ACPI enumerated SDHCI controllers" + depends on MMC_SDHCI && ACPI + help + This selects support for ACPI enumerated SDHCI controllers, + identified by ACPI Compatibility ID PNP0D40 or specific + ACPI Hardware IDs. + + If you have a controller with this interface, say Y or M here. + + If unsure, say N. + config MMC_SDHCI_PLTFM tristate "SDHCI platform and OF driver helper" depends on MMC_SDHCI diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index 17ad0a7..0e4960a 100644 --- a/drivers/mmc/host/Makefile +++ b/drivers/mmc/host/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_MMC_MXS) += mxs-mmc.o obj-$(CONFIG_MMC_SDHCI) += sdhci.o obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI)) += sdhci-pci-data.o +obj-$(CONFIG_MMC_SDHCI_ACPI) += sdhci-acpi.o obj-$(CONFIG_MMC_SDHCI_PXAV3) += sdhci-pxav3.o obj-$(CONFIG_MMC_SDHCI_PXAV2) += sdhci-pxav2.o obj-$(CONFIG_MMC_SDHCI_S3C) += sdhci-s3c.o diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c new file mode 100644 index 0000000..f582fe7 --- /dev/null +++ b/drivers/mmc/host/sdhci-acpi.c @@ -0,0 +1,304 @@ +/* + * Secure Digital Host Controller Interface ACPI driver. + * + * Copyright (c) 2012, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +#include <linux/init.h> +#include <linux/export.h> +#include <linux/module.h> +#include <linux/device.h> +#include <linux/platform_device.h> +#include <linux/ioport.h> +#include <linux/io.h> +#include <linux/dma-mapping.h> +#include <linux/compiler.h> +#include <linux/stddef.h> +#include <linux/bitops.h> +#include <linux/types.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/acpi.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> + +#include <linux/mmc/host.h> +#include <linux/mmc/pm.h> +#include <linux/mmc/sdhci.h> + +#include "sdhci.h" + +enum { + SDHCI_ACPI_SD_CD = BIT(0), + SDHCI_ACPI_RUNTIME_PM = BIT(1), +}; + +struct sdhci_acpi_chip { + const struct sdhci_ops *ops; + unsigned int quirks; + unsigned int quirks2; + unsigned long caps; + unsigned int caps2; + mmc_pm_flag_t pm_caps; +}; + +struct sdhci_acpi_slot { + const struct sdhci_acpi_chip *chip; + unsigned int quirks; + unsigned int quirks2; + unsigned long caps; + unsigned int caps2; + mmc_pm_flag_t pm_caps; + unsigned int flags; +}; + +struct sdhci_acpi_host { + struct sdhci_host *host; + const struct sdhci_acpi_slot *slot; + struct platform_device *pdev; + bool use_runtime_pm; +}; + +static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag) +{ + return c->slot && (c->slot->flags & flag); +} + +static int sdhci_acpi_enable_dma(struct sdhci_host *host) +{ + return 0; +} + +static const struct sdhci_ops sdhci_acpi_ops_dflt = { + .enable_dma = sdhci_acpi_enable_dma, +}; + +static const struct acpi_device_id sdhci_acpi_ids[] = { + { "PNP0D40" }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids); + +static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid) +{ + const struct acpi_device_id *id; + + for (id = sdhci_acpi_ids; id->id[0]; id++) + if (!strcmp(id->id, hid)) + return (const struct sdhci_acpi_slot *)id->driver_data; + return NULL; +} + +static int __devinit sdhci_acpi_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + acpi_handle handle = dev->acpi_handle; + struct acpi_device *device; + struct sdhci_acpi_host *c; + struct sdhci_host *host; + struct resource *iomem; + resource_size_t len; + const char *hid; + int err; + + if (acpi_bus_get_device(handle, &device)) + return -ENODEV; + + if (acpi_bus_get_status(device) || !device->status.present) + return -ENODEV; + + hid = acpi_device_hid(device); + + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!iomem) + return -ENOMEM; + + len = resource_size(iomem); + if (len < 0x100) + dev_err(dev, "Invalid iomem size!\n"); + + if (!devm_request_mem_region(dev, iomem->start, len, dev_name(dev))) + return -ENOMEM; + + host = sdhci_alloc_host(dev, sizeof(struct sdhci_acpi_host)); + if (IS_ERR(host)) + return PTR_ERR(host); + + c = sdhci_priv(host); + c->host = host; + c->slot = sdhci_acpi_get_slot(hid); + c->pdev = pdev; + c->use_runtime_pm = sdhci_acpi_flag(c, SDHCI_ACPI_RUNTIME_PM); + + platform_set_drvdata(pdev, c); + + host->hw_name = "ACPI"; + host->ops = &sdhci_acpi_ops_dflt; + host->irq = platform_get_irq(pdev, 0); + + host->ioaddr = devm_ioremap_nocache(dev, iomem->start, + resource_size(iomem)); + if (host->ioaddr == NULL) { + err = -ENOMEM; + goto err_free; + } + + if (!dev->dma_mask) { + u64 dma_mask; + + if (sdhci_readl(host, SDHCI_CAPABILITIES) & SDHCI_CAN_64BIT) { + /* 64-bit DMA is not supported at present */ + dma_mask = DMA_BIT_MASK(32); + } else { + dma_mask = DMA_BIT_MASK(32); + } + + dev->dma_mask = &dev->coherent_dma_mask; + dev->coherent_dma_mask = dma_mask; + } + + if (c->slot) { + if (c->slot->chip) { + host->ops = c->slot->chip->ops; + host->quirks |= c->slot->chip->quirks; + host->quirks2 |= c->slot->chip->quirks2; + host->mmc->caps |= c->slot->chip->caps; + host->mmc->caps2 |= c->slot->chip->caps2; + host->mmc->pm_caps |= c->slot->chip->pm_caps; + } + host->quirks |= c->slot->quirks; + host->quirks2 |= c->slot->quirks2; + host->mmc->caps |= c->slot->caps; + host->mmc->caps2 |= c->slot->caps2; + host->mmc->pm_caps |= c->slot->pm_caps; + } + + err = sdhci_add_host(host); + if (err) + goto err_free; + + if (c->use_runtime_pm) { + pm_suspend_ignore_children(dev, 1); + pm_runtime_set_autosuspend_delay(dev, 50); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); + } + + return 0; + +err_free: + platform_set_drvdata(pdev, NULL); + sdhci_free_host(c->host); + return err; +} + +static int __devexit sdhci_acpi_remove(struct platform_device *pdev) +{ + struct sdhci_acpi_host *c = platform_get_drvdata(pdev); + struct device *dev = &pdev->dev; + int dead; + + if (c->use_runtime_pm) { + pm_runtime_get_sync(dev); + pm_runtime_disable(dev); + pm_runtime_put_noidle(dev); + } + + dead = (sdhci_readl(c->host, SDHCI_INT_STATUS) == ~0); + sdhci_remove_host(c->host, dead); + platform_set_drvdata(pdev, NULL); + sdhci_free_host(c->host); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP + +static int sdhci_acpi_suspend(struct device *dev) +{ + struct sdhci_acpi_host *c = dev_get_drvdata(dev); + + return sdhci_suspend_host(c->host); +} + +static int sdhci_acpi_resume(struct device *dev) +{ + struct sdhci_acpi_host *c = dev_get_drvdata(dev); + + return sdhci_resume_host(c->host); +} + +#else + +#define sdhci_acpi_suspend NULL +#define sdhci_acpi_resume NULL + +#endif + +#ifdef CONFIG_PM_RUNTIME + +static int sdhci_acpi_runtime_suspend(struct device *dev) +{ + struct sdhci_acpi_host *c = dev_get_drvdata(dev); + + return sdhci_runtime_suspend_host(c->host); +} + +static int sdhci_acpi_runtime_resume(struct device *dev) +{ + struct sdhci_acpi_host *c = dev_get_drvdata(dev); + + return sdhci_runtime_resume_host(c->host); +} + +static int sdhci_acpi_runtime_idle(struct device *dev) +{ + return 0; +} + +#else + +#define sdhci_acpi_runtime_suspend NULL +#define sdhci_acpi_runtime_resume NULL +#define sdhci_acpi_runtime_idle NULL + +#endif + +static const struct dev_pm_ops sdhci_acpi_pm_ops = { + .suspend = sdhci_acpi_suspend, + .resume = sdhci_acpi_resume, + .runtime_suspend = sdhci_acpi_runtime_suspend, + .runtime_resume = sdhci_acpi_runtime_resume, + .runtime_idle = sdhci_acpi_runtime_idle, +}; + +static struct platform_driver sdhci_acpi_driver = { + .driver = { + .name = "sdhci-acpi", + .owner = THIS_MODULE, + .acpi_match_table = sdhci_acpi_ids, + .pm = &sdhci_acpi_pm_ops, + }, + .probe = sdhci_acpi_probe, + .remove = __devexit_p(sdhci_acpi_remove), +}; + +module_platform_driver(sdhci_acpi_driver); + +MODULE_DESCRIPTION("Secure Digital Host Controller Interface ACPI driver"); +MODULE_AUTHOR("Adrian Hunter"); +MODULE_LICENSE("GPL v2"); -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver 2012-11-22 8:43 ` [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver Adrian Hunter @ 2012-11-23 9:44 ` Mika Westerberg 0 siblings, 0 replies; 13+ messages in thread From: Mika Westerberg @ 2012-11-23 9:44 UTC (permalink / raw) To: Adrian Hunter Cc: Rafael J. Wysocki, Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel On Thu, Nov 22, 2012 at 10:43:50AM +0200, Adrian Hunter wrote: > +static int __devinit sdhci_acpi_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + acpi_handle handle = dev->acpi_handle; This is not going to work anymore, you'll have to use ACPI_HANDLE(dev) for this (there was a new macro introduced with the struct acpi_dev_node). > + struct acpi_device *device; > + struct sdhci_acpi_host *c; > + struct sdhci_host *host; > + struct resource *iomem; > + resource_size_t len; > + const char *hid; > + int err; > + > + if (acpi_bus_get_device(handle, &device)) > + return -ENODEV; > + > + if (acpi_bus_get_status(device) || !device->status.present) > + return -ENODEV; This is a bit redundant as the platform code already checks whether the device is present or not and only creates the platform device in case it is present. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-22 8:43 [PATCH 0/3] Add SDHCI ACPI driver Adrian Hunter ` (2 preceding siblings ...) 2012-11-22 8:43 ` [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver Adrian Hunter @ 2012-11-22 13:55 ` Chris Ball 2012-11-22 14:46 ` Adrian Hunter 3 siblings, 1 reply; 13+ messages in thread From: Chris Ball @ 2012-11-22 13:55 UTC (permalink / raw) To: Adrian Hunter Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel Hi, On Thu, Nov 22 2012, Adrian Hunter wrote: > Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support > so I suggest Rafael takes the patches with Chris' Ack. > > Please note that I would prefer this to be queued for 3.8 Looks fine: Acked-by: Chris Ball <cjb@laptop.org> I have some dumb questions, though -- what kind of platforms ship with these devices? Do they ever have the controller on PCI too, and what happens with sdhci-pci vs. sdhci-acpi in that case? Thanks, - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-22 13:55 ` [PATCH 0/3] Add " Chris Ball @ 2012-11-22 14:46 ` Adrian Hunter 2012-11-22 21:24 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-11-22 14:46 UTC (permalink / raw) To: Chris Ball, Rafael J. Wysocki Cc: Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel On 22/11/12 15:55, Chris Ball wrote: > Hi, > > On Thu, Nov 22 2012, Adrian Hunter wrote: >> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support >> so I suggest Rafael takes the patches with Chris' Ack. >> >> Please note that I would prefer this to be queued for 3.8 > > Looks fine: > > Acked-by: Chris Ball <cjb@laptop.org> Thank you! > > I have some dumb questions, though -- what kind of platforms ship with > these devices? Do they ever have the controller on PCI too, and what > happens with sdhci-pci vs. sdhci-acpi in that case? Since the arrival of ACPI5, platform devices can be configured using ACPI tables. PCI can also be used, but the firmware ensures that the same device is not enumerated via both ACPI and PCI. Rafael can you take these patches? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-22 14:46 ` Adrian Hunter @ 2012-11-22 21:24 ` Rafael J. Wysocki 2012-11-23 9:34 ` Mika Westerberg 2012-11-23 10:23 ` Adrian Hunter 0 siblings, 2 replies; 13+ messages in thread From: Rafael J. Wysocki @ 2012-11-22 21:24 UTC (permalink / raw) To: Adrian Hunter Cc: Chris Ball, Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: > On 22/11/12 15:55, Chris Ball wrote: > > Hi, > > > > On Thu, Nov 22 2012, Adrian Hunter wrote: > >> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support > >> so I suggest Rafael takes the patches with Chris' Ack. > >> > >> Please note that I would prefer this to be queued for 3.8 > > > > Looks fine: > > > > Acked-by: Chris Ball <cjb@laptop.org> > > Thank you! > > > > > I have some dumb questions, though -- what kind of platforms ship with > > these devices? Do they ever have the controller on PCI too, and what > > happens with sdhci-pci vs. sdhci-acpi in that case? > > Since the arrival of ACPI5, platform devices can be configured using ACPI > tables. PCI can also be used, but the firmware ensures that the same > device is not enumerated via both ACPI and PCI. > > Rafael can you take these patches? Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] directly in addition to excluded_id_list[], so that duplicate entries don't have to be added to the both of them. Also, I wonder if you really don't want to use ACPI PM and if you don't, then why? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-22 21:24 ` Rafael J. Wysocki @ 2012-11-23 9:34 ` Mika Westerberg 2012-11-23 10:13 ` Adrian Hunter 2012-11-23 10:23 ` Adrian Hunter 1 sibling, 1 reply; 13+ messages in thread From: Mika Westerberg @ 2012-11-23 9:34 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Adrian Hunter, Chris Ball, Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel On Thu, Nov 22, 2012 at 10:24:33PM +0100, Rafael J. Wysocki wrote: > On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: > > On 22/11/12 15:55, Chris Ball wrote: > > > Hi, > > > > > > On Thu, Nov 22 2012, Adrian Hunter wrote: > > >> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support > > >> so I suggest Rafael takes the patches with Chris' Ack. > > >> > > >> Please note that I would prefer this to be queued for 3.8 > > > > > > Looks fine: > > > > > > Acked-by: Chris Ball <cjb@laptop.org> > > > > Thank you! > > > > > > > > I have some dumb questions, though -- what kind of platforms ship with > > > these devices? Do they ever have the controller on PCI too, and what > > > happens with sdhci-pci vs. sdhci-acpi in that case? > > > > Since the arrival of ACPI5, platform devices can be configured using ACPI > > tables. PCI can also be used, but the firmware ensures that the same > > device is not enumerated via both ACPI and PCI. > > > > Rafael can you take these patches? > > Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] > directly in addition to excluded_id_list[], so that duplicate entries don't > have to be added to the both of them. How about having pnpacpi to check if the ACPI device is already bound to a physical device and skip the device creation? Then we don't need to expose the acpi_platform_device_ids[] list, and this is what the ->find_device() code already does so why create the device in the first place? diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 5b17cc8..4dc2e64 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -243,6 +243,10 @@ static int __init pnpacpi_add_device(struct acpi_device *device) char *pnpid; struct acpi_hardware_id *id; + /* Skip devices that are already bound */ + if (device->physical_node_count) + return 0; + /* * If a PnPacpi device is not present , the device * driver should not be loaded. ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-23 9:34 ` Mika Westerberg @ 2012-11-23 10:13 ` Adrian Hunter 2012-11-23 10:50 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-11-23 10:13 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Mika Westerberg, Chris Ball, Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel On 23/11/12 11:34, Mika Westerberg wrote: > On Thu, Nov 22, 2012 at 10:24:33PM +0100, Rafael J. Wysocki wrote: >> On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: >>> On 22/11/12 15:55, Chris Ball wrote: >>>> Hi, >>>> >>>> On Thu, Nov 22 2012, Adrian Hunter wrote: >>>>> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support >>>>> so I suggest Rafael takes the patches with Chris' Ack. >>>>> >>>>> Please note that I would prefer this to be queued for 3.8 >>>> >>>> Looks fine: >>>> >>>> Acked-by: Chris Ball <cjb@laptop.org> >>> >>> Thank you! >>> >>>> >>>> I have some dumb questions, though -- what kind of platforms ship with >>>> these devices? Do they ever have the controller on PCI too, and what >>>> happens with sdhci-pci vs. sdhci-acpi in that case? >>> >>> Since the arrival of ACPI5, platform devices can be configured using ACPI >>> tables. PCI can also be used, but the firmware ensures that the same >>> device is not enumerated via both ACPI and PCI. >>> >>> Rafael can you take these patches? >> >> Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] >> directly in addition to excluded_id_list[], so that duplicate entries don't >> have to be added to the both of them. > > How about having pnpacpi to check if the ACPI device is already bound to a > physical device and skip the device creation? Then we don't need to expose > the acpi_platform_device_ids[] list, and this is what the ->find_device() > code already does so why create the device in the first place? Yes, I was going to suggest that too. AFAICS pnpacpi has no concept of multiple physical nodes. Any objections? > > diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c > index 5b17cc8..4dc2e64 100644 > --- a/drivers/pnp/pnpacpi/core.c > +++ b/drivers/pnp/pnpacpi/core.c > @@ -243,6 +243,10 @@ static int __init pnpacpi_add_device(struct acpi_device *device) > char *pnpid; > struct acpi_hardware_id *id; > > + /* Skip devices that are already bound */ > + if (device->physical_node_count) > + return 0; > + > /* > * If a PnPacpi device is not present , the device > * driver should not be loaded. > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-23 10:13 ` Adrian Hunter @ 2012-11-23 10:50 ` Rafael J. Wysocki 0 siblings, 0 replies; 13+ messages in thread From: Rafael J. Wysocki @ 2012-11-23 10:50 UTC (permalink / raw) To: Adrian Hunter Cc: Mika Westerberg, Chris Ball, Rafael J. Wysocki, linux-mmc, linux-acpi, linux-kernel On Friday, November 23, 2012 12:13:13 PM Adrian Hunter wrote: > On 23/11/12 11:34, Mika Westerberg wrote: > > On Thu, Nov 22, 2012 at 10:24:33PM +0100, Rafael J. Wysocki wrote: > >> On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: > >>> On 22/11/12 15:55, Chris Ball wrote: > >>>> Hi, > >>>> > >>>> On Thu, Nov 22 2012, Adrian Hunter wrote: > >>>>> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support > >>>>> so I suggest Rafael takes the patches with Chris' Ack. > >>>>> > >>>>> Please note that I would prefer this to be queued for 3.8 > >>>> > >>>> Looks fine: > >>>> > >>>> Acked-by: Chris Ball <cjb@laptop.org> > >>> > >>> Thank you! > >>> > >>>> > >>>> I have some dumb questions, though -- what kind of platforms ship with > >>>> these devices? Do they ever have the controller on PCI too, and what > >>>> happens with sdhci-pci vs. sdhci-acpi in that case? > >>> > >>> Since the arrival of ACPI5, platform devices can be configured using ACPI > >>> tables. PCI can also be used, but the firmware ensures that the same > >>> device is not enumerated via both ACPI and PCI. > >>> > >>> Rafael can you take these patches? > >> > >> Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] > >> directly in addition to excluded_id_list[], so that duplicate entries don't > >> have to be added to the both of them. > > > > How about having pnpacpi to check if the ACPI device is already bound to a > > physical device and skip the device creation? Then we don't need to expose > > the acpi_platform_device_ids[] list, and this is what the ->find_device() > > code already does so why create the device in the first place? > > Yes, I was going to suggest that too. AFAICS pnpacpi has no concept of > multiple physical nodes. Any objections? Not from me. Thanks, Rafael > > diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c > > index 5b17cc8..4dc2e64 100644 > > --- a/drivers/pnp/pnpacpi/core.c > > +++ b/drivers/pnp/pnpacpi/core.c > > @@ -243,6 +243,10 @@ static int __init pnpacpi_add_device(struct acpi_device *device) > > char *pnpid; > > struct acpi_hardware_id *id; > > > > + /* Skip devices that are already bound */ > > + if (device->physical_node_count) > > + return 0; > > + > > /* > > * If a PnPacpi device is not present , the device > > * driver should not be loaded. > > > > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-22 21:24 ` Rafael J. Wysocki 2012-11-23 9:34 ` Mika Westerberg @ 2012-11-23 10:23 ` Adrian Hunter 2012-11-23 10:51 ` Rafael J. Wysocki 1 sibling, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-11-23 10:23 UTC (permalink / raw) To: Rafael J. Wysocki, Rafael J. Wysocki Cc: Chris Ball, linux-mmc, linux-acpi, linux-kernel On 22/11/12 23:24, Rafael J. Wysocki wrote: > On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: >> On 22/11/12 15:55, Chris Ball wrote: >>> Hi, >>> >>> On Thu, Nov 22 2012, Adrian Hunter wrote: >>>> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support >>>> so I suggest Rafael takes the patches with Chris' Ack. >>>> >>>> Please note that I would prefer this to be queued for 3.8 >>> >>> Looks fine: >>> >>> Acked-by: Chris Ball <cjb@laptop.org> >> >> Thank you! >> >>> >>> I have some dumb questions, though -- what kind of platforms ship with >>> these devices? Do they ever have the controller on PCI too, and what >>> happens with sdhci-pci vs. sdhci-acpi in that case? >> >> Since the arrival of ACPI5, platform devices can be configured using ACPI >> tables. PCI can also be used, but the firmware ensures that the same >> device is not enumerated via both ACPI and PCI. >> >> Rafael can you take these patches? > > Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] > directly in addition to excluded_id_list[], so that duplicate entries don't > have to be added to the both of them. > > Also, I wonder if you really don't want to use ACPI PM and if you don't, > then why? Mika and Lv Zheng are working on adding it to acpi_platform ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Add SDHCI ACPI driver 2012-11-23 10:23 ` Adrian Hunter @ 2012-11-23 10:51 ` Rafael J. Wysocki 0 siblings, 0 replies; 13+ messages in thread From: Rafael J. Wysocki @ 2012-11-23 10:51 UTC (permalink / raw) To: Adrian Hunter Cc: Rafael J. Wysocki, Chris Ball, linux-mmc, linux-acpi, linux-kernel On Friday, November 23, 2012 12:23:11 PM Adrian Hunter wrote: > On 22/11/12 23:24, Rafael J. Wysocki wrote: > > On Thursday, November 22, 2012 04:46:10 PM Adrian Hunter wrote: > >> On 22/11/12 15:55, Chris Ball wrote: > >>> Hi, > >>> > >>> On Thu, Nov 22 2012, Adrian Hunter wrote: > >>>> Here is SDHCI ACPI driver. It is dependent on new ACPI Platform support > >>>> so I suggest Rafael takes the patches with Chris' Ack. > >>>> > >>>> Please note that I would prefer this to be queued for 3.8 > >>> > >>> Looks fine: > >>> > >>> Acked-by: Chris Ball <cjb@laptop.org> > >> > >> Thank you! > >> > >>> > >>> I have some dumb questions, though -- what kind of platforms ship with > >>> these devices? Do they ever have the controller on PCI too, and what > >>> happens with sdhci-pci vs. sdhci-acpi in that case? > >> > >> Since the arrival of ACPI5, platform devices can be configured using ACPI > >> tables. PCI can also be used, but the firmware ensures that the same > >> device is not enumerated via both ACPI and PCI. > >> > >> Rafael can you take these patches? > > > > Well, I'd prefer pnpacpi/core.c to actually use acpi_platform_device_ids[] > > directly in addition to excluded_id_list[], so that duplicate entries don't > > have to be added to the both of them. > > > > Also, I wonder if you really don't want to use ACPI PM and if you don't, > > then why? > > Mika and Lv Zheng are working on adding it to acpi_platform OK Please address the Mika's comment for [3/3], though. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-11-23 10:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-11-22 8:43 [PATCH 0/3] Add SDHCI ACPI driver Adrian Hunter 2012-11-22 8:43 ` [PATCH 1/3] PNPACPI: exclude SDHCI devices Adrian Hunter 2012-11-22 8:43 ` [PATCH 2/3] ACPI: add SDHCI to ACPI platform devices Adrian Hunter 2012-11-22 8:43 ` [PATCH 3/3] mmc: sdhci-acpi: add SDHCI ACPI driver Adrian Hunter 2012-11-23 9:44 ` Mika Westerberg 2012-11-22 13:55 ` [PATCH 0/3] Add " Chris Ball 2012-11-22 14:46 ` Adrian Hunter 2012-11-22 21:24 ` Rafael J. Wysocki 2012-11-23 9:34 ` Mika Westerberg 2012-11-23 10:13 ` Adrian Hunter 2012-11-23 10:50 ` Rafael J. Wysocki 2012-11-23 10:23 ` Adrian Hunter 2012-11-23 10:51 ` Rafael J. Wysocki
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).