Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On 11/11/2013 09:45 PM, Rafael J. Wysocki wrote: > On Monday, November 11, 2013 09:21:40 AM Lan Tianyu wrote: >> On 2013年11月10日 08:58, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki >>> >>> Modify struct acpi_dev_node to contain a pointer to struct device >>> ambedded in the struct acpi_device associated with the given device >>> object (that is, its ACPI companion device) instead of an ACPI handle >>> corresponding to that struct acpi_device. Introduce two new macros >>> for manipulating that pointer in a CONFIG_ACPI-safe way, >>> ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the >>> ACPI_HANDLE() macro to take the above changes into account. >>> Drop the ACPI_HANDLE_SET() macro entirely and rework its users to >>> use ACPI_COMPANION_SET() instead. For some of them who used to >>> pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() >>> introduce a helper routine acpi_preset_companion() doing an >>> equivalent thing. >>> >>> The rationale for using a struct device pointer instead of a >>> struct acpi_device one as the member of struct acpi_dev_node is >>> that it allows device.h to avoid including linux/acpi.h which would >>> introduce quite a bit of compilation overhead for stuff that doesn't >>> care about ACPI. >>> In turn, moving the macros to linux/acpi.h forces >>> the stuff that does care about ACPI to include that file as >>> appropriate anyway. >> >> How about declaring "struct acpi_device" in the device.h? This can help >> to use struct acpi_device without including linux/acpi.h. >> >> struct iommu_ops and struct iommu_group have been used by the same way >> in the device.h. > > Yes, they are. Well, that appears to work too. > > Updated patch is appended. It also contains some fixes for problems reported > by the auto build system and it's been tested on x86-64 now, so it should be > reasonably close to final. > > Thanks, > Rafael > > > --- > From: Rafael J. Wysocki > Subject: ACPI / driver core: Store an ACPI device pointer in struct > acpi_dev_node > > Modify struct acpi_dev_node to contain a pointer to struct acpi_device > associated with the given device object (that is, its ACPI companion > device) instead of an ACPI handle corresponding to it. Introduce two > new macros for manipulating that pointer in a CONFIG_ACPI-safe way, > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > ACPI_HANDLE() macro to take the above changes into account. > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > use ACPI_COMPANION_SET() instead. For some of them who used to > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > introduce a helper routine acpi_preset_companion() doing an > equivalent thing. > > The main motivation for doing this is that there are things > represented by struct acpi_device objects that don't have valid > ACPI handles (so called fixed ACPI hardware features, such as > power and sleep buttons) and we would like to create platform > device objects for them and "glue" them to their ACPI companions > in the usual way (which currently is impossible due to the > lack of valid ACPI handles). However, there are more reasons > why it may be useful. > > First, struct acpi_device pointers allow of much better type checking > than void pointers which are ACPI handles, so it should be more > difficult to write buggy code using modified struct acpi_dev_node > and the new macros. Second, the change should help to reduce (over > time) the number of places in which the result of ACPI_HANDLE() is > passed to acpi_bus_get_device() in order to obtain a pointer to the > struct acpi_device associated with the given "physical" device, > because now that pointer is returned by ACPI_COMPANION() directly. > Finally, the change should make it easier to write generic code that > will build both for CONFIG_ACPI set and unset without adding explicit > compiler directives to it. > > Signed-off-by: Rafael J. Wysocki Reviewed-by: Aaron Lu for ATA and SDIO part. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Tuesday, November 12, 2013 11:24:02 AM Mika Westerberg wrote: > On Mon, Nov 11, 2013 at 02:45:39PM +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > Subject: ACPI / driver core: Store an ACPI device pointer in struct > > acpi_dev_node > > > > Modify struct acpi_dev_node to contain a pointer to struct acpi_device > > associated with the given device object (that is, its ACPI companion > > device) instead of an ACPI handle corresponding to it. Introduce two > > new macros for manipulating that pointer in a CONFIG_ACPI-safe way, > > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > > ACPI_HANDLE() macro to take the above changes into account. > > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > > use ACPI_COMPANION_SET() instead. For some of them who used to > > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > > introduce a helper routine acpi_preset_companion() doing an > > equivalent thing. > > > > The main motivation for doing this is that there are things > > represented by struct acpi_device objects that don't have valid > > ACPI handles (so called fixed ACPI hardware features, such as > > power and sleep buttons) and we would like to create platform > > device objects for them and "glue" them to their ACPI companions > > in the usual way (which currently is impossible due to the > > lack of valid ACPI handles). However, there are more reasons > > why it may be useful. > > > > First, struct acpi_device pointers allow of much better type checking > > than void pointers which are ACPI handles, so it should be more > > difficult to write buggy code using modified struct acpi_dev_node > > and the new macros. Second, the change should help to reduce (over > > time) the number of places in which the result of ACPI_HANDLE() is > > passed to acpi_bus_get_device() in order to obtain a pointer to the > > struct acpi_device associated with the given "physical" device, > > because now that pointer is returned by ACPI_COMPANION() directly. > > Finally, the change should make it easier to write generic code that > > will build both for CONFIG_ACPI set and unset without adding explicit > > compiler directives to it. > > > > Signed-off-by: Rafael J. Wysocki > > I tested this on Haswell as well and it works fine with ACPI enumerated > platform, I2C and SPI devices. > > Tested-by: Mika Westerberg (on Haswell) > Reviewed-by: Mika Westerberg Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Mon, Nov 11, 2013 at 02:45:39PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > Subject: ACPI / driver core: Store an ACPI device pointer in struct > acpi_dev_node > > Modify struct acpi_dev_node to contain a pointer to struct acpi_device > associated with the given device object (that is, its ACPI companion > device) instead of an ACPI handle corresponding to it. Introduce two > new macros for manipulating that pointer in a CONFIG_ACPI-safe way, > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > ACPI_HANDLE() macro to take the above changes into account. > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > use ACPI_COMPANION_SET() instead. For some of them who used to > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > introduce a helper routine acpi_preset_companion() doing an > equivalent thing. > > The main motivation for doing this is that there are things > represented by struct acpi_device objects that don't have valid > ACPI handles (so called fixed ACPI hardware features, such as > power and sleep buttons) and we would like to create platform > device objects for them and "glue" them to their ACPI companions > in the usual way (which currently is impossible due to the > lack of valid ACPI handles). However, there are more reasons > why it may be useful. > > First, struct acpi_device pointers allow of much better type checking > than void pointers which are ACPI handles, so it should be more > difficult to write buggy code using modified struct acpi_dev_node > and the new macros. Second, the change should help to reduce (over > time) the number of places in which the result of ACPI_HANDLE() is > passed to acpi_bus_get_device() in order to obtain a pointer to the > struct acpi_device associated with the given "physical" device, > because now that pointer is returned by ACPI_COMPANION() directly. > Finally, the change should make it easier to write generic code that > will build both for CONFIG_ACPI set and unset without adding explicit > compiler directives to it. > > Signed-off-by: Rafael J. Wysocki I tested this on Haswell as well and it works fine with ACPI enumerated platform, I2C and SPI devices. Tested-by: Mika Westerberg (on Haswell) Reviewed-by: Mika Westerberg -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Monday, November 11, 2013 07:03:18 AM Greg Kroah-Hartman wrote: > On Mon, Nov 11, 2013 at 02:45:39PM +0100, Rafael J. Wysocki wrote: > > On Monday, November 11, 2013 09:21:40 AM Lan Tianyu wrote: > > > On 2013年11月10日 08:58, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki > > > > > > > > Modify struct acpi_dev_node to contain a pointer to struct device > > > > ambedded in the struct acpi_device associated with the given device > > > > object (that is, its ACPI companion device) instead of an ACPI handle > > > > corresponding to that struct acpi_device. Introduce two new macros > > > > for manipulating that pointer in a CONFIG_ACPI-safe way, > > > > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > > > > ACPI_HANDLE() macro to take the above changes into account. > > > > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > > > > use ACPI_COMPANION_SET() instead. For some of them who used to > > > > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > > > > introduce a helper routine acpi_preset_companion() doing an > > > > equivalent thing. > > > > > > > > The rationale for using a struct device pointer instead of a > > > > struct acpi_device one as the member of struct acpi_dev_node is > > > > that it allows device.h to avoid including linux/acpi.h which would > > > > introduce quite a bit of compilation overhead for stuff that doesn't > > > > care about ACPI. > > > > In turn, moving the macros to linux/acpi.h forces > > > > the stuff that does care about ACPI to include that file as > > > > appropriate anyway. > > > > > > How about declaring "struct acpi_device" in the device.h? This can help > > > to use struct acpi_device without including linux/acpi.h. > > > > > > struct iommu_ops and struct iommu_group have been used by the same way > > > in the device.h. > > > > Yes, they are. Well, that appears to work too. > > > > Updated patch is appended. It also contains some fixes for problems > > reported > > by the auto build system and it's been tested on x86-64 now, so it should be > > reasonably close to final. > > > > Thanks, > > Rafael > > > > > > --- > > From: Rafael J. Wysocki > > Subject: ACPI / driver core: Store an ACPI device pointer in struct > > acpi_dev_node > > > > Modify struct acpi_dev_node to contain a pointer to struct acpi_device > > associated with the given device object (that is, its ACPI companion > > device) instead of an ACPI handle corresponding to it. Introduce two > > new macros for manipulating that pointer in a CONFIG_ACPI-safe way, > > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > > ACPI_HANDLE() macro to take the above changes into account. > > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > > use ACPI_COMPANION_SET() instead. For some of them who used to > > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > > introduce a helper routine acpi_preset_companion() doing an > > equivalent thing. > > > > The main motivation for doing this is that there are things > > represented by struct acpi_device objects that don't have valid > > ACPI handles (so called fixed ACPI hardware features, such as > > power and sleep buttons) and we would like to create platform > > device objects for them and "glue" them to their ACPI companions > > in the usual way (which currently is impossible due to the > > lack of valid ACPI handles). However, there are more reasons > > why it may be useful. > > > > First, struct acpi_device pointers allow of much better type checking > > than void pointers which are ACPI handles, so it should be more > > difficult to write buggy code using modified struct acpi_dev_node > > and the new macros. Second, the change should help to reduce (over > > time) the number of places in which the result of ACPI_HANDLE() is > > passed to acpi_bus_get_device() in order to obtain a pointer to the > > struct acpi_device associated with the given "physical" device, > > because now that pointer is returned by ACPI_COMPANION() directly. > > Finally, the change should make it easier to write generic code that > > will build both for CONFIG_ACPI set and unset without adding explicit > > compiler directives to it. > > > > Signed-off-by: Rafael J. Wysocki > > Acked-by: Greg Kroah-Hartman Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Mon, Nov 11, 2013 at 02:45:39PM +0100, Rafael J. Wysocki wrote: > On Monday, November 11, 2013 09:21:40 AM Lan Tianyu wrote: > > On 2013年11月10日 08:58, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki > > > > > > Modify struct acpi_dev_node to contain a pointer to struct device > > > ambedded in the struct acpi_device associated with the given device > > > object (that is, its ACPI companion device) instead of an ACPI handle > > > corresponding to that struct acpi_device. Introduce two new macros > > > for manipulating that pointer in a CONFIG_ACPI-safe way, > > > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > > > ACPI_HANDLE() macro to take the above changes into account. > > > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > > > use ACPI_COMPANION_SET() instead. For some of them who used to > > > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > > > introduce a helper routine acpi_preset_companion() doing an > > > equivalent thing. > > > > > > The rationale for using a struct device pointer instead of a > > > struct acpi_device one as the member of struct acpi_dev_node is > > > that it allows device.h to avoid including linux/acpi.h which would > > > introduce quite a bit of compilation overhead for stuff that doesn't > > > care about ACPI. > > > In turn, moving the macros to linux/acpi.h forces > > > the stuff that does care about ACPI to include that file as > > > appropriate anyway. > > > > How about declaring "struct acpi_device" in the device.h? This can help > > to use struct acpi_device without including linux/acpi.h. > > > > struct iommu_ops and struct iommu_group have been used by the same way > > in the device.h. > > Yes, they are. Well, that appears to work too. > > Updated patch is appended. It also contains some fixes for problems reported > by the auto build system and it's been tested on x86-64 now, so it should be > reasonably close to final. > > Thanks, > Rafael > > > --- > From: Rafael J. Wysocki > Subject: ACPI / driver core: Store an ACPI device pointer in struct > acpi_dev_node > > Modify struct acpi_dev_node to contain a pointer to struct acpi_device > associated with the given device object (that is, its ACPI companion > device) instead of an ACPI handle corresponding to it. Introduce two > new macros for manipulating that pointer in a CONFIG_ACPI-safe way, > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > ACPI_HANDLE() macro to take the above changes into account. > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > use ACPI_COMPANION_SET() instead. For some of them who used to > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > introduce a helper routine acpi_preset_companion() doing an > equivalent thing. > > The main motivation for doing this is that there are things > represented by struct acpi_device objects that don't have valid > ACPI handles (so called fixed ACPI hardware features, such as > power and sleep buttons) and we would like to create platform > device objects for them and "glue" them to their ACPI companions > in the usual way (which currently is impossible due to the > lack of valid ACPI handles). However, there are more reasons > why it may be useful. > > First, struct acpi_device pointers allow of much better type checking > than void pointers which are ACPI handles, so it should be more > difficult to write buggy code using modified struct acpi_dev_node > and the new macros. Second, the change should help to reduce (over > time) the number of places in which the result of ACPI_HANDLE() is > passed to acpi_bus_get_device() in order to obtain a pointer to the > struct acpi_device associated with the given "physical" device, > because now that pointer is returned by ACPI_COMPANION() directly. > Finally, the change should make it easier to write generic code that > will build both for CONFIG_ACPI set and unset without adding explicit > compiler directives to it. > > Signed-off-by: Rafael J. Wysocki Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Monday, November 11, 2013 09:21:40 AM Lan Tianyu wrote: > On 2013年11月10日 08:58, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > Modify struct acpi_dev_node to contain a pointer to struct device > > ambedded in the struct acpi_device associated with the given device > > object (that is, its ACPI companion device) instead of an ACPI handle > > corresponding to that struct acpi_device. Introduce two new macros > > for manipulating that pointer in a CONFIG_ACPI-safe way, > > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > > ACPI_HANDLE() macro to take the above changes into account. > > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > > use ACPI_COMPANION_SET() instead. For some of them who used to > > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > > introduce a helper routine acpi_preset_companion() doing an > > equivalent thing. > > > > The rationale for using a struct device pointer instead of a > > struct acpi_device one as the member of struct acpi_dev_node is > > that it allows device.h to avoid including linux/acpi.h which would > > introduce quite a bit of compilation overhead for stuff that doesn't > > care about ACPI. > > In turn, moving the macros to linux/acpi.h forces > > the stuff that does care about ACPI to include that file as > > appropriate anyway. > > How about declaring "struct acpi_device" in the device.h? This can help > to use struct acpi_device without including linux/acpi.h. > > struct iommu_ops and struct iommu_group have been used by the same way > in the device.h. Yes, they are. Well, that appears to work too. Updated patch is appended. It also contains some fixes for problems reported by the auto build system and it's been tested on x86-64 now, so it should be reasonably close to final. Thanks, Rafael --- From: Rafael J. Wysocki Subject: ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node Modify struct acpi_dev_node to contain a pointer to struct acpi_device associated with the given device object (that is, its ACPI companion device) instead of an ACPI handle corresponding to it. Introduce two new macros for manipulating that pointer in a CONFIG_ACPI-safe way, ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the ACPI_HANDLE() macro to take the above changes into account. Drop the ACPI_HANDLE_SET() macro entirely and rework its users to use ACPI_COMPANION_SET() instead. For some of them who used to pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() introduce a helper routine acpi_preset_companion() doing an equivalent thing. The main motivation for doing this is that there are things represented by struct acpi_device objects that don't have valid ACPI handles (so called fixed ACPI hardware features, such as power and sleep buttons) and we would like to create platform device objects for them and "glue" them to their ACPI companions in the usual way (which currently is impossible due to the lack of valid ACPI handles). However, there are more reasons why it may be useful. First, struct acpi_device pointers allow of much better type checking than void pointers which are ACPI handles, so it should be more difficult to write buggy code using modified struct acpi_dev_node and the new macros. Second, the change should help to reduce (over time) the number of places in which the result of ACPI_HANDLE() is passed to acpi_bus_get_device() in order to obtain a pointer to the struct acpi_device associated with the given "physical" device, because now that pointer is returned by ACPI_COMPANION() directly. Finally, the change should make it easier to write generic code that will build both for CONFIG_ACPI set and unset without adding explicit compiler directives to it. Signed-off-by: Rafael J. Wysocki --- arch/ia64/hp/common/sba_iommu.c |2 - arch/ia64/include/asm/pci.h |2 - arch/ia64/pci/pci.c |6 +-- arch/ia64/sn/kernel/io_acpi_init.c |4 +- arch/x86/include/asm/pci.h |2 - arch/x86/pci/acpi.c |4 +- drivers/acpi/acpi_platform.c |2 - drivers/acpi/device_pm.c |6 --- drivers/acpi/glue.c | 47 +-- drivers/ata/libata-acpi.c|4 +- drivers/base/platform.c |4 +- drivers/gpu/drm/radeon/radeon_atpx_handler.c |3 - drivers/hid/i2c-hid/i2c-hid.c|2 - drivers/i2c/i2c-core.c |4 +- drivers/ide/ide-acpi.c |3 - drivers/mmc/core/sdio_bus.c |3 - drivers/pci/hotplug/sgi_hotplug.c|4 +- drivers/spi/spi.c|2 - include/acpi/acpi_bus.h |2 - include/linux/acpi.h | 15 include/linux/device.h
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
gt; dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
> @@ -1103,7 +1103,7 @@ static acpi_status acpi_i2c_add_device(a
> return AE_OK;
>
> memset(&info, 0, sizeof(info));
> - info.acpi_node.handle = handle;
> + info.acpi_node.dev = &adev->dev;
> info.irq = -1;
>
> INIT_LIST_HEAD(&resource_list);
> Index: linux-pm/drivers/mmc/core/sdio_bus.c
> ===
> --- linux-pm.orig/drivers/mmc/core/sdio_bus.c
> +++ linux-pm/drivers/mmc/core/sdio_bus.c
> @@ -305,8 +305,7 @@ static void sdio_acpi_set_handle(struct
> struct mmc_host *host = func->card->host;
> u64 addr = (host->slotno << 16) | func->num;
>
> - ACPI_HANDLE_SET(&func->dev,
> - acpi_get_child(ACPI_HANDLE(host->parent), addr));
> + acpi_preset_companion(&func->dev, ACPI_HANDLE(host->parent), addr);
> }
> #else
> static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
> Index: linux-pm/drivers/spi/spi.c
> ===
> --- linux-pm.orig/drivers/spi/spi.c
> +++ linux-pm/drivers/spi/spi.c
> @@ -1024,7 +1024,7 @@ static acpi_status acpi_spi_add_device(a
> return AE_NO_MEMORY;
> }
>
> - ACPI_HANDLE_SET(&spi->dev, handle);
> + ACPI_COMPANION_SET(&spi->dev, &adev->dev);
> spi->irq = -1;
>
> INIT_LIST_HEAD(&resource_list);
> Index: linux-pm/drivers/acpi/device_pm.c
> ===
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -22,16 +22,12 @@
> *
> ~~~
> */
>
> -#include
> +#include
> #include
> #include
> #include
> #include
>
> -#include
> -#include
> -#include
> -
> #include "internal.h"
>
> #define _COMPONENT ACPI_POWER_COMPONENT
>
>
> Received: from fmsmsx107.amr.corp.intel.com (10.19.9.54) by
> SHSMSX104.ccr.corp.intel.com (10.239.4.70) with Microsoft SMTP Server (TLS)
> id 14.3.123.3; Sun, 10 Nov 2013 08:46:23 +0800
> Received: from AZSMGA002.ch.intel.com (10.2.17.35) by FMSMSX107-1.fm.intel.com
> (10.19.9.54) with Microsoft SMTP Server id 14.3.123.3; Sat, 9 Nov 2013
> 16:46:21 -0800
> Received: from azsmga102.ch.intel.com ([10.2.16.52]) by
> AZSMGA002-1.ch.intel.com with ESMTP; 09 Nov 2013 16:46:20 -0800
> X-IronPort-Anti-Spam-Filtered: true
> X-IronPort-Anti-Spam-Result:
> AssSAOTWflJPYKqG/2dsb2JhbABZgz+DR4IvsluHG4E+dIIlASkEUjAFAgUMBw4CEQE3FhOIBQGqWZIWgSmNBoEtCxGCYYFFA5Qug2GBL4kVh0aDJw
> X-IronPort-AV: E=Sophos;i="4.93,669,1378882800";
>d="scan'208";a="109266908"
> Received: from v094114.home.net.pl ([79.96.170.134]) by mga14.intel.com with
> SMTP; 09 Nov 2013 16:46:19 -0800
> Received: from aayg161.neoplus.adsl.tpnet.pl [83.6.118.161] (HELO
> vostro.rjw.lan) by serwer1319399.home.pl [79.96.170.134] with SMTP
> (IdeaSmtpServer v0.80) id f863c1bc354b7571; Sun, 10 Nov 2013 01:46:16 +0100
> From: "Rafael J. Wysocki"
> To: ACPI Devel Maling List
> CC: LKML , Linux PCI
> , Greg Kroah-Hartman
> ,
> Bjorn Helgaas , Aaron Lu ,
> "Jarkko
> Nikula" , Lan Tianyu ,
> Mika Westerberg , "Luck, Tony"
>
> Subject: [PATCH] ACPI / driver core: Store a device pointer in struct
> acpi_dev_node
> Date: Sun, 10 Nov 2013 01:58:42 +0100
> Message-ID: <3268437.ysushvk...@vostro.rjw.lan>
> User-Agent: KMail/4.10.5 (Linux/3.12.0-rc6+; KDE/4.10.5; x86_64; ; )
> Content-Transfer-Encoding: 7Bit
> Content-Type: text/plain; charset="utf-8"
> Return-Path: r...@rjwysocki.net
> X-MS-Exchange-Organization-AVStamp-Mailbox: NAI;56073597;0;novirus
> X-MS-Exchange-Organization-AuthSource: FMSMSX107.amr.corp.intel.com
> X-MS-Exchange-Organization-AuthAs: Anonymous
> MIME-Version: 1.0
>
> From: Rafael J. Wysocki
>
> Modify struct acpi_dev_node to contain a pointer to struct device
> ambedded in the struct acpi_device associated with the given device
> object (that is, its ACPI companion device) instead of an ACPI handle
> corresponding to that struct acpi_device. Introduce two new macros
> for manipulating that pointer in a CONFIG_ACPI-safe way,
> ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the
> ACPI_HANDLE() macro to take the above changes into account.
> Drop the ACPI_HANDLE_SET() macro entirely and rework its users to
> use ACPI_COMPANION_SET() instead.
Re: [PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
On Sun, Nov 10, 2013 at 01:58:42AM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > Modify struct acpi_dev_node to contain a pointer to struct device > ambedded in the struct acpi_device associated with the given device > object (that is, its ACPI companion device) instead of an ACPI handle > corresponding to that struct acpi_device. Introduce two new macros > for manipulating that pointer in a CONFIG_ACPI-safe way, > ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the > ACPI_HANDLE() macro to take the above changes into account. > Drop the ACPI_HANDLE_SET() macro entirely and rework its users to > use ACPI_COMPANION_SET() instead. For some of them who used to > pass the result of acpi_get_child() directly to ACPI_HANDLE_SET() > introduce a helper routine acpi_preset_companion() doing an > equivalent thing. > > The rationale for using a struct device pointer instead of a > struct acpi_device one as the member of struct acpi_dev_node is > that it allows device.h to avoid including linux/acpi.h which would > introduce quite a bit of compilation overhead for stuff that doesn't > care about ACPI. In turn, moving the macros to linux/acpi.h forces > the stuff that does care about ACPI to include that file as > appropriate anyway. > > The main motivation for doing this is that there are things > represented by struct acpi_device objects that don't have valid > ACPI handles (so called fixed ACPI hardware features, such as > power and sleep buttons) and we would like to create platform > device objects for them and "glue" them to their ACPI companions > in the usual way (which currently is impossible due to the > lack of valid ACPI handles). However, there are more reasons > why it may be useful. > > First, struct device pointers allow of much better type checking > than void pointers which are ACPI handles, so it should be more > difficult to write buggy code using modified struct acpi_dev_node > and the new macros. Second, it should help to reduce the number > of places in which the result of ACPI_HANDLE() is passed to > acpi_bus_get_device() in order to obtain a pointer to the > struct acpi_device associated with the given "physical" device, > because now that pointer can be obtained directly by applying > to_acpi_device() to the result of the ACPI_COMPANION() macro. > Finally, it should make it easier to write generic code that will > build both for CONFIG_ACPI set and unset without adding explicit > compiler directives to it. > > Signed-off-by: Rafael J. Wysocki > --- > > Hi Everybody, > > First of all, I haven't tested this yet, so caveat emptor. I have compiled > it on x86-64 for CONFIG_ACPI set and unset and I'm going to feed it to the > auto build system shortly in case I overlooked something build-related. > > Please have a look and let me know if you have any problems with this in > principle. If not, I'd like to queue it up for inclusion by the end of > the merge window or in the -rc2 time frame (to avoid collisions with any > big merges), as I'd like to be able to work on top of it during the 3.14 > cycle if possible. At first glance, this looks good to me, thanks for removing that void *, I like this a lot better now. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ACPI / driver core: Store a device pointer in struct acpi_dev_node
From: Rafael J. Wysocki
Modify struct acpi_dev_node to contain a pointer to struct device
ambedded in the struct acpi_device associated with the given device
object (that is, its ACPI companion device) instead of an ACPI handle
corresponding to that struct acpi_device. Introduce two new macros
for manipulating that pointer in a CONFIG_ACPI-safe way,
ACPI_COMPANION() and ACPI_COMPANION_SET(), and rework the
ACPI_HANDLE() macro to take the above changes into account.
Drop the ACPI_HANDLE_SET() macro entirely and rework its users to
use ACPI_COMPANION_SET() instead. For some of them who used to
pass the result of acpi_get_child() directly to ACPI_HANDLE_SET()
introduce a helper routine acpi_preset_companion() doing an
equivalent thing.
The rationale for using a struct device pointer instead of a
struct acpi_device one as the member of struct acpi_dev_node is
that it allows device.h to avoid including linux/acpi.h which would
introduce quite a bit of compilation overhead for stuff that doesn't
care about ACPI. In turn, moving the macros to linux/acpi.h forces
the stuff that does care about ACPI to include that file as
appropriate anyway.
The main motivation for doing this is that there are things
represented by struct acpi_device objects that don't have valid
ACPI handles (so called fixed ACPI hardware features, such as
power and sleep buttons) and we would like to create platform
device objects for them and "glue" them to their ACPI companions
in the usual way (which currently is impossible due to the
lack of valid ACPI handles). However, there are more reasons
why it may be useful.
First, struct device pointers allow of much better type checking
than void pointers which are ACPI handles, so it should be more
difficult to write buggy code using modified struct acpi_dev_node
and the new macros. Second, it should help to reduce the number
of places in which the result of ACPI_HANDLE() is passed to
acpi_bus_get_device() in order to obtain a pointer to the
struct acpi_device associated with the given "physical" device,
because now that pointer can be obtained directly by applying
to_acpi_device() to the result of the ACPI_COMPANION() macro.
Finally, it should make it easier to write generic code that will
build both for CONFIG_ACPI set and unset without adding explicit
compiler directives to it.
Signed-off-by: Rafael J. Wysocki
---
Hi Everybody,
First of all, I haven't tested this yet, so caveat emptor. I have compiled
it on x86-64 for CONFIG_ACPI set and unset and I'm going to feed it to the
auto build system shortly in case I overlooked something build-related.
Please have a look and let me know if you have any problems with this in
principle. If not, I'd like to queue it up for inclusion by the end of
the merge window or in the -rc2 time frame (to avoid collisions with any
big merges), as I'd like to be able to work on top of it during the 3.14
cycle if possible.
Thanks,
Rafael
---
arch/ia64/include/asm/pci.h |2 -
arch/ia64/pci/pci.c |6 ++---
arch/x86/include/asm/pci.h|2 -
arch/x86/pci/acpi.c |4 +--
drivers/acpi/acpi_platform.c |2 -
drivers/acpi/device_pm.c |6 -
drivers/acpi/glue.c | 45 +-
drivers/ata/libata-acpi.c |4 +--
drivers/base/platform.c |4 +--
drivers/hid/i2c-hid/i2c-hid.c |2 -
drivers/i2c/i2c-core.c|4 +--
drivers/mmc/core/sdio_bus.c |3 --
drivers/spi/spi.c |2 -
include/acpi/acpi_bus.h |2 -
include/linux/acpi.h | 14 +
include/linux/device.h| 10 -
16 files changed, 57 insertions(+), 55 deletions(-)
Index: linux-pm/include/linux/device.h
===
--- linux-pm.orig/include/linux/device.h
+++ linux-pm/include/linux/device.h
@@ -625,7 +625,7 @@ struct device_dma_parameters {
struct acpi_dev_node {
#ifdef CONFIG_ACPI
- void*handle;
+ struct device *dev;
#endif
};
@@ -769,14 +769,6 @@ static inline struct device *kobj_to_dev
return container_of(kobj, struct device, kobj);
}
-#ifdef CONFIG_ACPI
-#define ACPI_HANDLE(dev) ((dev)->acpi_node.handle)
-#define ACPI_HANDLE_SET(dev, _handle_) (dev)->acpi_node.handle = (_handle_)
-#else
-#define ACPI_HANDLE(dev) (NULL)
-#define ACPI_HANDLE_SET(dev, _handle_) do { } while (0)
-#endif
-
/* Get the wakeup routines, which depend on struct device */
#include
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -431,9 +431,9 @@ static inline acpi_handle acpi_get_child
{
return acpi_find_child(handle, addr, false);
}
+void acpi_preset_companion(struct device *dev, acpi_handle parent, u64 addr);
int acpi_is_root_bridge(acpi_handle);
struct acpi_pci_root *acpi_pci_find_r
