Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Tue, Sep 09, 2014 at 12:14:04AM +0200, Rafael J. Wysocki wrote:
> On Saturday, August 23, 2014 11:21:17 PM Zhang Rui wrote:
>
> [cut]
>
> Darren, would there be any problems if I took the patch below from Rui for
> 3.18?
>
No objection.
Acked-by: Darren Hart
> >
> > So I guess the following patch can be upstream candidate, right?
> >
> > From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001
> > From: Zhang Rui
> > Date: Fri, 22 Aug 2014 09:06:10 +0800
> > Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from
> > ACPI
> > PNP id list
> >
> > Fujitsu backlight and hotkey devices have ACPI drivers.
> > The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an
> > indicator for module autoloading. But this is wrong because what we need is
> > ACPI module device table instead because the driver is probing ACPI devices.
> >
> > Thus remove those ids from ACPI PNP scan handler list as we don't
> > have PNP driver for them, and convert the fujitsu-laptop PNP
> > MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE.
> >
> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971
> > Signed-off-by: Zhang Rui
> > Tested-by: Dirk Griesbach
> > ---
> > drivers/acpi/acpi_pnp.c | 4
> > drivers/platform/x86/fujitsu-laptop.c | 16 +++-
> > 2 files changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> > index 1f8b204..b193f84 100644
> > --- a/drivers/acpi/acpi_pnp.c
> > +++ b/drivers/acpi/acpi_pnp.c
> > @@ -130,10 +130,6 @@ static const struct acpi_device_id
> > acpi_pnp_device_ids[] = {
> > {"PNP0401"},/* ECP Printer Port */
> > /* apple-gmux */
> > {"APP000B"},
> > - /* fujitsu-laptop.c */
> > - {"FUJ02bf"},
> > - {"FUJ02B1"},
> > - {"FUJ02E3"},
> > /* system */
> > {"PNP0c02"},/* General ID for reserving resources */
> > {"PNP0c01"},/* memory controller */
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c
> > b/drivers/platform/x86/fujitsu-laptop.c
> > index 87aa28c..2655d4a 100644
> > --- a/drivers/platform/x86/fujitsu-laptop.c
> > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > @@ -1050,6 +1050,13 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver
> > = {
> > },
> > };
> >
> > +static const struct acpi_device_id fujitsu_ids[] __used = {
> > + {ACPI_FUJITSU_HID, 0},
> > + {ACPI_FUJITSU_HOTKEY_HID, 0},
> > + {"", 0}
> > +};
> > +MODULE_DEVICE_TABLE(acpi, fujitsu_ids);
> > +
> > static int __init fujitsu_init(void)
> > {
> > int ret, result, max_brightness;
> > @@ -1208,12 +1215,3 @@ MODULE_LICENSE("GPL");
> >
> > MODULE_ALIAS("dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1D3:*:cvrS6410:*");
> >
> > MODULE_ALIAS("dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1E6:*:cvrS6420:*");
> > MODULE_ALIAS("dmi:*:svnFUJITSU:*:pvr:rvnFUJITSU:rnFJNB19C:*:cvrS7020:*");
> > -
> > -static struct pnp_device_id pnp_ids[] __used = {
> > - {.id = "FUJ02bf"},
> > - {.id = "FUJ02B1"},
> > - {.id = "FUJ02E3"},
> > - {.id = ""}
> > -};
> > -
> > -MODULE_DEVICE_TABLE(pnp, pnp_ids);
> >
>
> --
> 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/
>
--
Darren Hart
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Saturday, August 23, 2014 11:21:17 PM Zhang Rui wrote:
[cut]
Darren, would there be any problems if I took the patch below from Rui for 3.18?
>
> So I guess the following patch can be upstream candidate, right?
>
> From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001
> From: Zhang Rui
> Date: Fri, 22 Aug 2014 09:06:10 +0800
> Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from ACPI
> PNP id list
>
> Fujitsu backlight and hotkey devices have ACPI drivers.
> The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an
> indicator for module autoloading. But this is wrong because what we need is
> ACPI module device table instead because the driver is probing ACPI devices.
>
> Thus remove those ids from ACPI PNP scan handler list as we don't
> have PNP driver for them, and convert the fujitsu-laptop PNP
> MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE.
>
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971
> Signed-off-by: Zhang Rui
> Tested-by: Dirk Griesbach
> ---
> drivers/acpi/acpi_pnp.c | 4
> drivers/platform/x86/fujitsu-laptop.c | 16 +++-
> 2 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> index 1f8b204..b193f84 100644
> --- a/drivers/acpi/acpi_pnp.c
> +++ b/drivers/acpi/acpi_pnp.c
> @@ -130,10 +130,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]
> = {
> {"PNP0401"},/* ECP Printer Port */
> /* apple-gmux */
> {"APP000B"},
> - /* fujitsu-laptop.c */
> - {"FUJ02bf"},
> - {"FUJ02B1"},
> - {"FUJ02E3"},
> /* system */
> {"PNP0c02"},/* General ID for reserving resources */
> {"PNP0c01"},/* memory controller */
> diff --git a/drivers/platform/x86/fujitsu-laptop.c
> b/drivers/platform/x86/fujitsu-laptop.c
> index 87aa28c..2655d4a 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -1050,6 +1050,13 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver =
> {
> },
> };
>
> +static const struct acpi_device_id fujitsu_ids[] __used = {
> + {ACPI_FUJITSU_HID, 0},
> + {ACPI_FUJITSU_HOTKEY_HID, 0},
> + {"", 0}
> +};
> +MODULE_DEVICE_TABLE(acpi, fujitsu_ids);
> +
> static int __init fujitsu_init(void)
> {
> int ret, result, max_brightness;
> @@ -1208,12 +1215,3 @@ MODULE_LICENSE("GPL");
>
> MODULE_ALIAS("dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1D3:*:cvrS6410:*");
>
> MODULE_ALIAS("dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1E6:*:cvrS6420:*");
> MODULE_ALIAS("dmi:*:svnFUJITSU:*:pvr:rvnFUJITSU:rnFJNB19C:*:cvrS7020:*");
> -
> -static struct pnp_device_id pnp_ids[] __used = {
> - {.id = "FUJ02bf"},
> - {.id = "FUJ02B1"},
> - {.id = "FUJ02E3"},
> - {.id = ""}
> -};
> -
> -MODULE_DEVICE_TABLE(pnp, pnp_ids);
>
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Saturday, August 23, 2014 11:21:17 PM Zhang Rui wrote:
[cut]
Darren, would there be any problems if I took the patch below from Rui for 3.18?
So I guess the following patch can be upstream candidate, right?
From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001
From: Zhang Rui rui.zh...@intel.com
Date: Fri, 22 Aug 2014 09:06:10 +0800
Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from ACPI
PNP id list
Fujitsu backlight and hotkey devices have ACPI drivers.
The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an
indicator for module autoloading. But this is wrong because what we need is
ACPI module device table instead because the driver is probing ACPI devices.
Thus remove those ids from ACPI PNP scan handler list as we don't
have PNP driver for them, and convert the fujitsu-laptop PNP
MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Signed-off-by: Zhang Rui rui.zh...@intel.com
Tested-by: Dirk Griesbach spamt...@freenet.de
---
drivers/acpi/acpi_pnp.c | 4
drivers/platform/x86/fujitsu-laptop.c | 16 +++-
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 1f8b204..b193f84 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -130,10 +130,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]
= {
{PNP0401},/* ECP Printer Port */
/* apple-gmux */
{APP000B},
- /* fujitsu-laptop.c */
- {FUJ02bf},
- {FUJ02B1},
- {FUJ02E3},
/* system */
{PNP0c02},/* General ID for reserving resources */
{PNP0c01},/* memory controller */
diff --git a/drivers/platform/x86/fujitsu-laptop.c
b/drivers/platform/x86/fujitsu-laptop.c
index 87aa28c..2655d4a 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -1050,6 +1050,13 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver =
{
},
};
+static const struct acpi_device_id fujitsu_ids[] __used = {
+ {ACPI_FUJITSU_HID, 0},
+ {ACPI_FUJITSU_HOTKEY_HID, 0},
+ {, 0}
+};
+MODULE_DEVICE_TABLE(acpi, fujitsu_ids);
+
static int __init fujitsu_init(void)
{
int ret, result, max_brightness;
@@ -1208,12 +1215,3 @@ MODULE_LICENSE(GPL);
MODULE_ALIAS(dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1D3:*:cvrS6410:*);
MODULE_ALIAS(dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1E6:*:cvrS6420:*);
MODULE_ALIAS(dmi:*:svnFUJITSU:*:pvr:rvnFUJITSU:rnFJNB19C:*:cvrS7020:*);
-
-static struct pnp_device_id pnp_ids[] __used = {
- {.id = FUJ02bf},
- {.id = FUJ02B1},
- {.id = FUJ02E3},
- {.id = }
-};
-
-MODULE_DEVICE_TABLE(pnp, pnp_ids);
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Tue, Sep 09, 2014 at 12:14:04AM +0200, Rafael J. Wysocki wrote:
On Saturday, August 23, 2014 11:21:17 PM Zhang Rui wrote:
[cut]
Darren, would there be any problems if I took the patch below from Rui for
3.18?
No objection.
Acked-by: Darren Hart dvh...@linux.intel.com
So I guess the following patch can be upstream candidate, right?
From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001
From: Zhang Rui rui.zh...@intel.com
Date: Fri, 22 Aug 2014 09:06:10 +0800
Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from
ACPI
PNP id list
Fujitsu backlight and hotkey devices have ACPI drivers.
The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an
indicator for module autoloading. But this is wrong because what we need is
ACPI module device table instead because the driver is probing ACPI devices.
Thus remove those ids from ACPI PNP scan handler list as we don't
have PNP driver for them, and convert the fujitsu-laptop PNP
MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Signed-off-by: Zhang Rui rui.zh...@intel.com
Tested-by: Dirk Griesbach spamt...@freenet.de
---
drivers/acpi/acpi_pnp.c | 4
drivers/platform/x86/fujitsu-laptop.c | 16 +++-
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 1f8b204..b193f84 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -130,10 +130,6 @@ static const struct acpi_device_id
acpi_pnp_device_ids[] = {
{PNP0401},/* ECP Printer Port */
/* apple-gmux */
{APP000B},
- /* fujitsu-laptop.c */
- {FUJ02bf},
- {FUJ02B1},
- {FUJ02E3},
/* system */
{PNP0c02},/* General ID for reserving resources */
{PNP0c01},/* memory controller */
diff --git a/drivers/platform/x86/fujitsu-laptop.c
b/drivers/platform/x86/fujitsu-laptop.c
index 87aa28c..2655d4a 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -1050,6 +1050,13 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver
= {
},
};
+static const struct acpi_device_id fujitsu_ids[] __used = {
+ {ACPI_FUJITSU_HID, 0},
+ {ACPI_FUJITSU_HOTKEY_HID, 0},
+ {, 0}
+};
+MODULE_DEVICE_TABLE(acpi, fujitsu_ids);
+
static int __init fujitsu_init(void)
{
int ret, result, max_brightness;
@@ -1208,12 +1215,3 @@ MODULE_LICENSE(GPL);
MODULE_ALIAS(dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1D3:*:cvrS6410:*);
MODULE_ALIAS(dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1E6:*:cvrS6420:*);
MODULE_ALIAS(dmi:*:svnFUJITSU:*:pvr:rvnFUJITSU:rnFJNB19C:*:cvrS7020:*);
-
-static struct pnp_device_id pnp_ids[] __used = {
- {.id = FUJ02bf},
- {.id = FUJ02B1},
- {.id = FUJ02E3},
- {.id = }
-};
-
-MODULE_DEVICE_TABLE(pnp, pnp_ids);
--
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/
--
Darren Hart
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Sunday, August 24, 2014 03:06:50 PM Zhang Rui wrote:
> On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki
[cut]
> > 3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/acpi/acpi_pnp.c
> > ===
> > --- linux-pm.orig/drivers/acpi/acpi_pnp.c
> > +++ linux-pm/drivers/acpi/acpi_pnp.c
> > @@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
> > {
> > acpi_scan_add_handler(_pnp_handler);
> > }
> > +
> > +bool is_acpi_pnp_device(struct acpi_device *adev)
> > +{
> > + return adev->handler == _pnp_handler;
> > +}
>
> can we reuse acpi_is_pnp_device()?
> The only difference is that acpi_is_pnp_device() returns true for
> RTC_CMOS devices, which is not a problem IMO because RTC CMOS devices
> have PNP driver only.
Yes, we can. Overlooked that.
Updated patch is appended.
Rafael
---
From: Rafael J. Wysocki
Subject: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion "physical" device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time. Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
For this reason, add a special check for PNP devices in
acpi_device_probe() so that ACPI drivers can bind to ACPI device
objects having existing PNP device companions as before.
Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
enumeration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Reported-by: Gabriele Mazzotta
Reported-by: Dirk Griesbach
Signed-off-by: Rafael J. Wysocki
---
drivers/acpi/scan.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
int ret;
- if (acpi_dev->handler)
+ if (acpi_dev->handler && !acpi_is_pnp_device(acpi_dev))
return -EINVAL;
if (!acpi_drv->ops.add)
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Sunday, August 24, 2014 03:06:50 PM Zhang Rui wrote:
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
[cut]
3 files changed, 7 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/acpi_pnp.c
===
--- linux-pm.orig/drivers/acpi/acpi_pnp.c
+++ linux-pm/drivers/acpi/acpi_pnp.c
@@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
{
acpi_scan_add_handler(acpi_pnp_handler);
}
+
+bool is_acpi_pnp_device(struct acpi_device *adev)
+{
+ return adev-handler == acpi_pnp_handler;
+}
can we reuse acpi_is_pnp_device()?
The only difference is that acpi_is_pnp_device() returns true for
RTC_CMOS devices, which is not a problem IMO because RTC CMOS devices
have PNP driver only.
Yes, we can. Overlooked that.
Updated patch is appended.
Rafael
---
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion physical device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time. Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
For this reason, add a special check for PNP devices in
acpi_device_probe() so that ACPI drivers can bind to ACPI device
objects having existing PNP device companions as before.
Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
enumeration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Reported-by: Gabriele Mazzotta gabriele@gmail.com
Reported-by: Dirk Griesbach spamt...@freenet.de
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
struct acpi_driver *acpi_drv = to_acpi_driver(dev-driver);
int ret;
- if (acpi_dev-handler)
+ if (acpi_dev-handler !acpi_is_pnp_device(acpi_dev))
return -EINVAL;
if (!acpi_drv-ops.add)
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> We generally don't allow ACPI drivers to bind to ACPI device objects
> that companion "physical" device objects are created for to avoid
> situations in which two different drivers may attempt to handle one
> device at the same time. Recent ACPI device enumeration rework
> extended that approach to ACPI PNP devices by starting to use a scan
> handler for enumerating them. However, we previously allowed ACPI
> drivers to bind to ACPI device objects with existing PNP device
> companions and changing that led to functional regressions on some
> systems.
>
> For this reason, add a special check for PNP devices in
> acpi_device_probe() so that ACPI drivers can bind to ACPI device
> objects having existing PNP device companions as before.
>
> Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
> enumeration)
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
> Reported-and-tested-by: Gabriele Mazzotta
> Reported-and-tested-by: Dirk Griesbach
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/acpi_pnp.c |5 +
> drivers/acpi/internal.h |1 +
> drivers/acpi/scan.c |2 +-
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/acpi_pnp.c
> ===
> --- linux-pm.orig/drivers/acpi/acpi_pnp.c
> +++ linux-pm/drivers/acpi/acpi_pnp.c
> @@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
> {
> acpi_scan_add_handler(_pnp_handler);
> }
> +
> +bool is_acpi_pnp_device(struct acpi_device *adev)
> +{
> + return adev->handler == _pnp_handler;
> +}
can we reuse acpi_is_pnp_device()?
The only difference is that acpi_is_pnp_device() returns true for
RTC_CMOS devices, which is not a problem IMO because RTC CMOS devices
have PNP driver only.
thanks,
rui
> Index: linux-pm/drivers/acpi/internal.h
> ===
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -86,6 +86,7 @@ void acpi_device_add_finalize(struct acp
> void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
> bool acpi_device_is_present(struct acpi_device *adev);
> bool acpi_device_is_battery(struct acpi_device *adev);
> +bool is_acpi_pnp_device(struct acpi_device *adev);
>
> /* --
>Power Resource
> Index: linux-pm/drivers/acpi/scan.c
> ===
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
> struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
> int ret;
>
> - if (acpi_dev->handler)
> + if (acpi_dev->handler && !is_acpi_pnp_device(acpi_dev))
> return -EINVAL;
>
> if (!acpi_drv->ops.add)
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion physical device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time. Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
For this reason, add a special check for PNP devices in
acpi_device_probe() so that ACPI drivers can bind to ACPI device
objects having existing PNP device companions as before.
Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
enumeration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Reported-and-tested-by: Gabriele Mazzotta gabriele@gmail.com
Reported-and-tested-by: Dirk Griesbach spamt...@freenet.de
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/acpi_pnp.c |5 +
drivers/acpi/internal.h |1 +
drivers/acpi/scan.c |2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/acpi_pnp.c
===
--- linux-pm.orig/drivers/acpi/acpi_pnp.c
+++ linux-pm/drivers/acpi/acpi_pnp.c
@@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
{
acpi_scan_add_handler(acpi_pnp_handler);
}
+
+bool is_acpi_pnp_device(struct acpi_device *adev)
+{
+ return adev-handler == acpi_pnp_handler;
+}
can we reuse acpi_is_pnp_device()?
The only difference is that acpi_is_pnp_device() returns true for
RTC_CMOS devices, which is not a problem IMO because RTC CMOS devices
have PNP driver only.
thanks,
rui
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -86,6 +86,7 @@ void acpi_device_add_finalize(struct acp
void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
bool acpi_device_is_present(struct acpi_device *adev);
bool acpi_device_is_battery(struct acpi_device *adev);
+bool is_acpi_pnp_device(struct acpi_device *adev);
/* --
Power Resource
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
struct acpi_driver *acpi_drv = to_acpi_driver(dev-driver);
int ret;
- if (acpi_dev-handler)
+ if (acpi_dev-handler !is_acpi_pnp_device(acpi_dev))
return -EINVAL;
if (!acpi_drv-ops.add)
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
On Fri, 2014-08-22 at 19:53 +0200, Rafael J. Wysocki wrote: > On Friday, August 22, 2014 10:00:31 AM Zhang Rui wrote: > > On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote: > > > On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: > > > > Hi, Rafael, > > > > > > > > On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: > > > > > From: Rafael J. Wysocki > > > > > > [cut] > > > > > > > Note that I've just tested on my machine and it works well. > > > > I still need the bug reporter to check if the patch fixes bug 81511 or > > > > not. > > > > > > The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and > > > they aren't motherboard devices. > > > > Right, but IMO, the rootcause of that bug is that > > 1. the PNP id table in fujitsu-laptop driver was introduced for some > > reason, probably it is used as an indicator for module auto-loading, and > > nowadays, this is redundant because fujitsu-laptop driver probes ACPI > > device only, and the driver will be loaded if the ACPI device objects > > for FUJ02B1 and FUJ02E3 is created. > > We may be able to drop the Fujitsu devices from the ACPI PNP list and all may > work. Still, does that fix all of the potential problems? > > > 2. This "redundant" PNP id table results in that those IDs are added to > > PNP ID list unnecessarily, and results in PNP device nodes for those > > devices are created unnecessarily. > > Yes, that may be the case, but the way to deal with that is not to break > thing and then see what's broken and fix it, but to get rid of ACPI drivers > one by one in which case we can control what's been changed and why. > > > > Yes, we need to convert that driver > > > to use a PNP driver structure or a platform device, but (1) we need a > > > -stable fix *first* > > > > I agree. > > > > > and (2) the cases we already know about may not be > > > the only broken ones. > > > > Agree. > > But the issue addressed in your patch is that PNP scan handler blocks > > ACPI driver from being probed, right? > > Yes. > > > So my question would be, > > 1. If the id in PNP scan handler does not have a PNP driver, like the > >FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler? > >In fact, I think this is a good chance for us to cleanup the ACPI PNP > >id list, as long as we can fix them in time. > > No. > > We need -stable to work and there's no way I will mark the motherboard > resource changes for -stable. I agree. And I would rather consider my patch as the start point of a long term solution. > Second, if we deal with it as I said (that is, > get rid of ACPI drivers gradually), we will clean up that list in the process > without breaking things for people in random ways. Agree. > > > 2. If the id in PNP scan handler has a PNP driver, should we allow both > >PNP driver and ACPI driver loaded? I think PNP system driver is the > >only case that makes us have to say yes, and what I'm trying to do > >is to fix this in the following patch. > > > > Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We > > still may get bug report complaining some *PLATFORM* driver stops to > > functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later. > > right? > > No. > > We never allowed ACPI drivers to bind to ACPI device objects having platform > device companions, No, I mean, a platform device is used to be created as the ACPI device object _HID was in the previous acpi_platform scan handler list, but if this device has _CID PNP0C01/PNP0C02, the platform device will not be created any more after the ACPI enumeration re-work patches. > wherease we *did* allow that for ACPI device objects having > PNP device companions. We simply need to go back to what we were doing and > fix > things *on* *top* of that. > > Any other approach pretty much guarantees leaving breakage in random places. > > So I'm fine with cleaning up the PNP list *if* you convert the drivers in > question from ACPI drivers to something else (platform drivers preferably) > at the same time. > yes, I see. So I guess the following patch can be upstream candidate, right? >From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Fri, 22 Aug 2014 09:06:10 +0800 Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from ACPI PNP id list Fujitsu backlight and hotkey devices have ACPI drivers. The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an indicator for module autoloading. But this is wrong because what we need is ACPI module device table instead because the driver is probing ACPI devices. Thus remove those ids from ACPI PNP scan handler list as we don't have PNP driver for them, and convert the fujitsu-laptop PNP MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE. Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971 Signed-off-by: Zhang Rui Tested-by: Dirk Griesbach --- drivers/acpi/acpi_pnp.c
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Fri, 2014-08-22 at 19:53 +0200, Rafael J. Wysocki wrote: On Friday, August 22, 2014 10:00:31 AM Zhang Rui wrote: On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote: On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: Hi, Rafael, On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com [cut] Note that I've just tested on my machine and it works well. I still need the bug reporter to check if the patch fixes bug 81511 or not. The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and they aren't motherboard devices. Right, but IMO, the rootcause of that bug is that 1. the PNP id table in fujitsu-laptop driver was introduced for some reason, probably it is used as an indicator for module auto-loading, and nowadays, this is redundant because fujitsu-laptop driver probes ACPI device only, and the driver will be loaded if the ACPI device objects for FUJ02B1 and FUJ02E3 is created. We may be able to drop the Fujitsu devices from the ACPI PNP list and all may work. Still, does that fix all of the potential problems? 2. This redundant PNP id table results in that those IDs are added to PNP ID list unnecessarily, and results in PNP device nodes for those devices are created unnecessarily. Yes, that may be the case, but the way to deal with that is not to break thing and then see what's broken and fix it, but to get rid of ACPI drivers one by one in which case we can control what's been changed and why. Yes, we need to convert that driver to use a PNP driver structure or a platform device, but (1) we need a -stable fix *first* I agree. and (2) the cases we already know about may not be the only broken ones. Agree. But the issue addressed in your patch is that PNP scan handler blocks ACPI driver from being probed, right? Yes. So my question would be, 1. If the id in PNP scan handler does not have a PNP driver, like the FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler? In fact, I think this is a good chance for us to cleanup the ACPI PNP id list, as long as we can fix them in time. No. We need -stable to work and there's no way I will mark the motherboard resource changes for -stable. I agree. And I would rather consider my patch as the start point of a long term solution. Second, if we deal with it as I said (that is, get rid of ACPI drivers gradually), we will clean up that list in the process without breaking things for people in random ways. Agree. 2. If the id in PNP scan handler has a PNP driver, should we allow both PNP driver and ACPI driver loaded? I think PNP system driver is the only case that makes us have to say yes, and what I'm trying to do is to fix this in the following patch. Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We still may get bug report complaining some *PLATFORM* driver stops to functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later. right? No. We never allowed ACPI drivers to bind to ACPI device objects having platform device companions, No, I mean, a platform device is used to be created as the ACPI device object _HID was in the previous acpi_platform scan handler list, but if this device has _CID PNP0C01/PNP0C02, the platform device will not be created any more after the ACPI enumeration re-work patches. wherease we *did* allow that for ACPI device objects having PNP device companions. We simply need to go back to what we were doing and fix things *on* *top* of that. Any other approach pretty much guarantees leaving breakage in random places. So I'm fine with cleaning up the PNP list *if* you convert the drivers in question from ACPI drivers to something else (platform drivers preferably) at the same time. yes, I see. So I guess the following patch can be upstream candidate, right? From e32c2de37750d622dae6ef9d2f5c448a528a7edb Mon Sep 17 00:00:00 2001 From: Zhang Rui rui.zh...@intel.com Date: Fri, 22 Aug 2014 09:06:10 +0800 Subject: [PATCH] ACPI: remove Fujitsu backlight and hotkey device ID from ACPI PNP id list Fujitsu backlight and hotkey devices have ACPI drivers. The PNP MODULE_DEVICE_TABLE in fujitsu-laptop driver is just used as an indicator for module autoloading. But this is wrong because what we need is ACPI module device table instead because the driver is probing ACPI devices. Thus remove those ids from ACPI PNP scan handler list as we don't have PNP driver for them, and convert the fujitsu-laptop PNP MODULE_DEVICE_TABLE to ACPI MODULE_DEVICE_TABLE. Reference: https://bugzilla.kernel.org/show_bug.cgi?id=81971 Signed-off-by: Zhang Rui rui.zh...@intel.com Tested-by: Dirk Griesbach spamt...@freenet.de --- drivers/acpi/acpi_pnp.c | 4 drivers/platform/x86/fujitsu-laptop.c | 16 +++- 2
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Friday, August 22, 2014 10:00:31 AM Zhang Rui wrote: > On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote: > > On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: > > > Hi, Rafael, > > > > > > On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki > > > > [cut] > > > > > Note that I've just tested on my machine and it works well. > > > I still need the bug reporter to check if the patch fixes bug 81511 or > > > not. > > > > The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and > > they aren't motherboard devices. > > Right, but IMO, the rootcause of that bug is that > 1. the PNP id table in fujitsu-laptop driver was introduced for some > reason, probably it is used as an indicator for module auto-loading, and > nowadays, this is redundant because fujitsu-laptop driver probes ACPI > device only, and the driver will be loaded if the ACPI device objects > for FUJ02B1 and FUJ02E3 is created. We may be able to drop the Fujitsu devices from the ACPI PNP list and all may work. Still, does that fix all of the potential problems? > 2. This "redundant" PNP id table results in that those IDs are added to > PNP ID list unnecessarily, and results in PNP device nodes for those > devices are created unnecessarily. Yes, that may be the case, but the way to deal with that is not to break thing and then see what's broken and fix it, but to get rid of ACPI drivers one by one in which case we can control what's been changed and why. > > Yes, we need to convert that driver > > to use a PNP driver structure or a platform device, but (1) we need a > > -stable fix *first* > > I agree. > > > and (2) the cases we already know about may not be > > the only broken ones. > > Agree. > But the issue addressed in your patch is that PNP scan handler blocks > ACPI driver from being probed, right? Yes. > So my question would be, > 1. If the id in PNP scan handler does not have a PNP driver, like the >FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler? >In fact, I think this is a good chance for us to cleanup the ACPI PNP >id list, as long as we can fix them in time. No. We need -stable to work and there's no way I will mark the motherboard resource changes for -stable. Second, if we deal with it as I said (that is, get rid of ACPI drivers gradually), we will clean up that list in the process without breaking things for people in random ways. > 2. If the id in PNP scan handler has a PNP driver, should we allow both >PNP driver and ACPI driver loaded? I think PNP system driver is the >only case that makes us have to say yes, and what I'm trying to do >is to fix this in the following patch. > > Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We > still may get bug report complaining some *PLATFORM* driver stops to > functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later. > right? No. We never allowed ACPI drivers to bind to ACPI device objects having platform device companions, wherease we *did* allow that for ACPI device objects having PNP device companions. We simply need to go back to what we were doing and fix things *on* *top* of that. Any other approach pretty much guarantees leaving breakage in random places. So I'm fine with cleaning up the PNP list *if* you convert the drivers in question from ACPI drivers to something else (platform drivers preferably) at the same time. Rafael -- 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 / scan: Allow ACPI drivers to bind to PNP device objects
On Friday, August 22, 2014 10:00:31 AM Zhang Rui wrote: On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote: On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: Hi, Rafael, On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com [cut] Note that I've just tested on my machine and it works well. I still need the bug reporter to check if the patch fixes bug 81511 or not. The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and they aren't motherboard devices. Right, but IMO, the rootcause of that bug is that 1. the PNP id table in fujitsu-laptop driver was introduced for some reason, probably it is used as an indicator for module auto-loading, and nowadays, this is redundant because fujitsu-laptop driver probes ACPI device only, and the driver will be loaded if the ACPI device objects for FUJ02B1 and FUJ02E3 is created. We may be able to drop the Fujitsu devices from the ACPI PNP list and all may work. Still, does that fix all of the potential problems? 2. This redundant PNP id table results in that those IDs are added to PNP ID list unnecessarily, and results in PNP device nodes for those devices are created unnecessarily. Yes, that may be the case, but the way to deal with that is not to break thing and then see what's broken and fix it, but to get rid of ACPI drivers one by one in which case we can control what's been changed and why. Yes, we need to convert that driver to use a PNP driver structure or a platform device, but (1) we need a -stable fix *first* I agree. and (2) the cases we already know about may not be the only broken ones. Agree. But the issue addressed in your patch is that PNP scan handler blocks ACPI driver from being probed, right? Yes. So my question would be, 1. If the id in PNP scan handler does not have a PNP driver, like the FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler? In fact, I think this is a good chance for us to cleanup the ACPI PNP id list, as long as we can fix them in time. No. We need -stable to work and there's no way I will mark the motherboard resource changes for -stable. Second, if we deal with it as I said (that is, get rid of ACPI drivers gradually), we will clean up that list in the process without breaking things for people in random ways. 2. If the id in PNP scan handler has a PNP driver, should we allow both PNP driver and ACPI driver loaded? I think PNP system driver is the only case that makes us have to say yes, and what I'm trying to do is to fix this in the following patch. Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We still may get bug report complaining some *PLATFORM* driver stops to functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later. right? No. We never allowed ACPI drivers to bind to ACPI device objects having platform device companions, wherease we *did* allow that for ACPI device objects having PNP device companions. We simply need to go back to what we were doing and fix things *on* *top* of that. Any other approach pretty much guarantees leaving breakage in random places. So I'm fine with cleaning up the PNP list *if* you convert the drivers in question from ACPI drivers to something else (platform drivers preferably) at the same time. Rafael -- 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 / scan: Allow ACPI drivers to bind to PNP device objects
On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote:
> On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote:
> > Hi, Rafael,
> >
> > On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki
>
> [cut]
>
> > Note that I've just tested on my machine and it works well.
> > I still need the bug reporter to check if the patch fixes bug 81511 or not.
>
> The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and
> they aren't motherboard devices.
Right, but IMO, the rootcause of that bug is that
1. the PNP id table in fujitsu-laptop driver was introduced for some
reason, probably it is used as an indicator for module auto-loading, and
nowadays, this is redundant because fujitsu-laptop driver probes ACPI
device only, and the driver will be loaded if the ACPI device objects
for FUJ02B1 and FUJ02E3 is created.
2. This "redundant" PNP id table results in that those IDs are added to
PNP ID list unnecessarily, and results in PNP device nodes for those
devices are created unnecessarily.
> Yes, we need to convert that driver
> to use a PNP driver structure or a platform device, but (1) we need a
> -stable fix *first*
I agree.
> and (2) the cases we already know about may not be
> the only broken ones.
Agree.
But the issue addressed in your patch is that PNP scan handler blocks
ACPI driver from being probed, right?
So my question would be,
1. If the id in PNP scan handler does not have a PNP driver, like the
FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler?
In fact, I think this is a good chance for us to cleanup the ACPI PNP
id list, as long as we can fix them in time.
2. If the id in PNP scan handler has a PNP driver, should we allow both
PNP driver and ACPI driver loaded? I think PNP system driver is the
only case that makes us have to say yes, and what I'm trying to do
is to fix this in the following patch.
Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We
still may get bug report complaining some *PLATFORM* driver stops to
functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later.
right?
thanks,
rui
>
> > From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
> > From: Zhang Rui
> > Date: Thu, 21 Aug 2014 13:39:47 +0800
> > Subject: [RFC PATCH] ACPI: introduce motherboard resource management
> >
> > ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
> > they have some motherboard resources that needs to be reserved.
> >
> > We used to enumerated those devices to PNP bus and rely on
> > PNP system driver to do resource reservation.
> > But this mechanism does not work well nowadays as many devices
> > not only represent motherboard resources, but also represent
> > physical devices that need native drivers other than PNP system
> > driver for the device to work. For example,
> > 1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
> > Device (NIPM)
> > {
> > Name (_HID, EisaId ("IPI0001")) // _HID: Hardware ID
> > Name (_CID, EisaId ("PNP0C01")) // _CID: Compatible ID
> >the NIPM device has _CID PNP0C01 but it is an IPMI device.
> >PNP system driver blocks the PNP IPMI driver to probe.
>
> That is a good reason for PNP0C01 to be dropped from acpi_pnp_device_ids[].
>
> > 2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
> > Device (IFFS)
> > {
> > Name (_HID, EisaId ("INT3392")) // _HID: Hardware ID
> > Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
> >the IFFS device has _CID PNP0C02, but it is an intel rapid start
> >device, which already has an ACPI driver at
> >drivers/platform/x86/intel-rst.c
>
> And which should be a platform driver really.
>
> > 3) a couple of machines, including the on in
> >https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
> >like following
> > Device (PTID)
> > {
> > Name (_HID, EisaId ("INT340E")) // _HID: Hardware ID
> > Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
> >the PTID device has _CID PNP0C02, but it is also represents an
> >INT340E device, there is a platform bus driver for this device
> >which will be introduced by myself soon.
>
> Again, that's a good reason for dropping PNP0C02 from acpi_pnp_device_ids[].
>
> > In any of the above cases, the current code for managing PNP0C01/PNP0C02
> > resources in Linux kernel is broken, because it either blocks the physical
> > device driver on the same bus, or results in multiple drivers loaded for
> > the same ACPI device node, which may also has some potential risks.
> >
> > Thus, IMO, we need a clean way to handle those motherboard resources.
> > Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
> > resources, this patch
> > 1. does the resource reservation in ACPI code directly, with the same logic
> >and time point in drivers/pnp/quirks.c and
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote:
> Hi, Rafael,
>
> On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki
[cut]
> Note that I've just tested on my machine and it works well.
> I still need the bug reporter to check if the patch fixes bug 81511 or not.
The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and
they aren't motherboard devices. Yes, we need to convert that driver
to use a PNP driver structure or a platform device, but (1) we need a
-stable fix *first* and (2) the cases we already know about may not be
the only broken ones.
> From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
> From: Zhang Rui
> Date: Thu, 21 Aug 2014 13:39:47 +0800
> Subject: [RFC PATCH] ACPI: introduce motherboard resource management
>
> ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
> they have some motherboard resources that needs to be reserved.
>
> We used to enumerated those devices to PNP bus and rely on
> PNP system driver to do resource reservation.
> But this mechanism does not work well nowadays as many devices
> not only represent motherboard resources, but also represent
> physical devices that need native drivers other than PNP system
> driver for the device to work. For example,
> 1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
> Device (NIPM)
> {
> Name (_HID, EisaId ("IPI0001")) // _HID: Hardware ID
> Name (_CID, EisaId ("PNP0C01")) // _CID: Compatible ID
>the NIPM device has _CID PNP0C01 but it is an IPMI device.
>PNP system driver blocks the PNP IPMI driver to probe.
That is a good reason for PNP0C01 to be dropped from acpi_pnp_device_ids[].
> 2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
> Device (IFFS)
> {
> Name (_HID, EisaId ("INT3392")) // _HID: Hardware ID
> Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
>the IFFS device has _CID PNP0C02, but it is an intel rapid start
>device, which already has an ACPI driver at
>drivers/platform/x86/intel-rst.c
And which should be a platform driver really.
> 3) a couple of machines, including the on in
>https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
>like following
> Device (PTID)
> {
> Name (_HID, EisaId ("INT340E")) // _HID: Hardware ID
> Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
>the PTID device has _CID PNP0C02, but it is also represents an
>INT340E device, there is a platform bus driver for this device
>which will be introduced by myself soon.
Again, that's a good reason for dropping PNP0C02 from acpi_pnp_device_ids[].
> In any of the above cases, the current code for managing PNP0C01/PNP0C02
> resources in Linux kernel is broken, because it either blocks the physical
> device driver on the same bus, or results in multiple drivers loaded for
> the same ACPI device node, which may also has some potential risks.
>
> Thus, IMO, we need a clean way to handle those motherboard resources.
> Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
> resources, this patch
> 1. does the resource reservation in ACPI code directly, with the same logic
>and time point in drivers/pnp/quirks.c and drivers/pnp/system.c.
> 2. makes PNP0C01/PNP0C02 PNP id transparent to Linux devices and drivers,
>thus PNP system driver becomes a no-op for ACPI enumerated devices.
>
> This is just a draft patch, and I'd like to see if this is
> the right direction to go. Any comments are welcome.
>
> Signed-off-by: Zhang Rui
> ---
> drivers/acpi/acpi_pnp.c | 3 -
> drivers/acpi/scan.c | 208
> +++-
> 2 files changed, 204 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> index 1f8b204..a7deae5 100644
> --- a/drivers/acpi/acpi_pnp.c
> +++ b/drivers/acpi/acpi_pnp.c
> @@ -134,9 +134,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]
> = {
> {"FUJ02bf"},
> {"FUJ02B1"},
> {"FUJ02E3"},
> - /* system */
> - {"PNP0c02"},/* General ID for reserving resources */
> - {"PNP0c01"},/* memory controller */
> /* rtc_cmos */
> {"PNP0b00"},
> {"PNP0b01"},
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0a817ad..674518b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -1781,12 +1782,201 @@ static bool acpi_object_is_system_bus(acpi_handle
> handle)
> return false;
> }
>
> +static bool acpi_is_motherboard_resource(char *id)
> +{
> + return !(strncmp(id, "PNP0C01", sizeof("PNP0C01")) &&
> + strncmp(id, "PNP0C02", sizeof("PNP0C02")));
> +}
Can we use __acpi_match_device() for that?
> +
> +static
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: > Hi, Rafael, > > On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > We generally don't allow ACPI drivers to bind to ACPI device objects > > that companion "physical" device objects are created for to avoid > > situations in which two different drivers may attempt to handle one > > device at the same time. > > Yes, and I think we should not break this rule. No, we broke the way the code worked previously. We need to restore it first and *then* try to fix it up, not the other way around. > > Recent ACPI device enumeration rework > > extended that approach to ACPI PNP devices by starting to use a scan > > handler for enumerating them. However, we previously allowed ACPI > > drivers to bind to ACPI device objects with existing PNP device > > companions and changing that led to functional regressions on some > > systems. > > > Question: except the PNP0C01/PNP0C02 case, if we have an device have two > ids that matches two different drivers, should we allow both drivers > probe successfully? No, we shouldn't, but in the cases in question we have only one driver (an ACPI one). > I think the answer is no. > In the PNP0C01/PNP0C02 case, I think we can fix the issue by the > following patch instead. The right answer to me is to get rid of ACPI drviers entirely. The thing below adds complexity to the resources management which I'm not sure is necessary. In any case, please let's do that on top of the $subject patch not instead of it, because there are more cases we need to cover. And we need a fix for -stable. Rafael -- 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 / scan: Allow ACPI drivers to bind to PNP device objects
Hi, Rafael,
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> We generally don't allow ACPI drivers to bind to ACPI device objects
> that companion "physical" device objects are created for to avoid
> situations in which two different drivers may attempt to handle one
> device at the same time.
Yes, and I think we should not break this rule.
> Recent ACPI device enumeration rework
> extended that approach to ACPI PNP devices by starting to use a scan
> handler for enumerating them. However, we previously allowed ACPI
> drivers to bind to ACPI device objects with existing PNP device
> companions and changing that led to functional regressions on some
> systems.
>
Question: except the PNP0C01/PNP0C02 case, if we have an device have two
ids that matches two different drivers, should we allow both drivers
probe successfully?
I think the answer is no.
In the PNP0C01/PNP0C02 case, I think we can fix the issue by the
following patch instead.
Note that I've just tested on my machine and it works well.
I still need the bug reporter to check if the patch fixes bug 81511 or not.
>From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
From: Zhang Rui
Date: Thu, 21 Aug 2014 13:39:47 +0800
Subject: [RFC PATCH] ACPI: introduce motherboard resource management
ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
they have some motherboard resources that needs to be reserved.
We used to enumerated those devices to PNP bus and rely on
PNP system driver to do resource reservation.
But this mechanism does not work well nowadays as many devices
not only represent motherboard resources, but also represent
physical devices that need native drivers other than PNP system
driver for the device to work. For example,
1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
Device (NIPM)
{
Name (_HID, EisaId ("IPI0001")) // _HID: Hardware ID
Name (_CID, EisaId ("PNP0C01")) // _CID: Compatible ID
the NIPM device has _CID PNP0C01 but it is an IPMI device.
PNP system driver blocks the PNP IPMI driver to probe.
2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
Device (IFFS)
{
Name (_HID, EisaId ("INT3392")) // _HID: Hardware ID
Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
the IFFS device has _CID PNP0C02, but it is an intel rapid start
device, which already has an ACPI driver at
drivers/platform/x86/intel-rst.c
3) a couple of machines, including the on in
https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
like following
Device (PTID)
{
Name (_HID, EisaId ("INT340E")) // _HID: Hardware ID
Name (_CID, EisaId ("PNP0C02")) // _CID: Compatible ID
the PTID device has _CID PNP0C02, but it is also represents an
INT340E device, there is a platform bus driver for this device
which will be introduced by myself soon.
In any of the above cases, the current code for managing PNP0C01/PNP0C02
resources in Linux kernel is broken, because it either blocks the physical
device driver on the same bus, or results in multiple drivers loaded for
the same ACPI device node, which may also has some potential risks.
Thus, IMO, we need a clean way to handle those motherboard resources.
Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
resources, this patch
1. does the resource reservation in ACPI code directly, with the same logic
and time point in drivers/pnp/quirks.c and drivers/pnp/system.c.
2. makes PNP0C01/PNP0C02 PNP id transparent to Linux devices and drivers,
thus PNP system driver becomes a no-op for ACPI enumerated devices.
This is just a draft patch, and I'd like to see if this is
the right direction to go. Any comments are welcome.
Signed-off-by: Zhang Rui
---
drivers/acpi/acpi_pnp.c | 3 -
drivers/acpi/scan.c | 208 +++-
2 files changed, 204 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 1f8b204..a7deae5 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -134,9 +134,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[] = {
{"FUJ02bf"},
{"FUJ02B1"},
{"FUJ02E3"},
- /* system */
- {"PNP0c02"},/* General ID for reserving resources */
- {"PNP0c01"},/* memory controller */
/* rtc_cmos */
{"PNP0b00"},
{"PNP0b01"},
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0a817ad..674518b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
@@ -1781,12 +1782,201 @@ static bool acpi_object_is_system_bus(acpi_handle
handle)
return false;
}
+static bool acpi_is_motherboard_resource(char *id)
+{
+ return !(strncmp(id, "PNP0C01", sizeof("PNP0C01")) &&
+
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
Hi, Rafael,
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion physical device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time.
Yes, and I think we should not break this rule.
Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
Question: except the PNP0C01/PNP0C02 case, if we have an device have two
ids that matches two different drivers, should we allow both drivers
probe successfully?
I think the answer is no.
In the PNP0C01/PNP0C02 case, I think we can fix the issue by the
following patch instead.
Note that I've just tested on my machine and it works well.
I still need the bug reporter to check if the patch fixes bug 81511 or not.
From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
From: Zhang Rui rui.zh...@intel.com
Date: Thu, 21 Aug 2014 13:39:47 +0800
Subject: [RFC PATCH] ACPI: introduce motherboard resource management
ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
they have some motherboard resources that needs to be reserved.
We used to enumerated those devices to PNP bus and rely on
PNP system driver to do resource reservation.
But this mechanism does not work well nowadays as many devices
not only represent motherboard resources, but also represent
physical devices that need native drivers other than PNP system
driver for the device to work. For example,
1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
Device (NIPM)
{
Name (_HID, EisaId (IPI0001)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C01)) // _CID: Compatible ID
the NIPM device has _CID PNP0C01 but it is an IPMI device.
PNP system driver blocks the PNP IPMI driver to probe.
2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
Device (IFFS)
{
Name (_HID, EisaId (INT3392)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the IFFS device has _CID PNP0C02, but it is an intel rapid start
device, which already has an ACPI driver at
drivers/platform/x86/intel-rst.c
3) a couple of machines, including the on in
https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
like following
Device (PTID)
{
Name (_HID, EisaId (INT340E)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the PTID device has _CID PNP0C02, but it is also represents an
INT340E device, there is a platform bus driver for this device
which will be introduced by myself soon.
In any of the above cases, the current code for managing PNP0C01/PNP0C02
resources in Linux kernel is broken, because it either blocks the physical
device driver on the same bus, or results in multiple drivers loaded for
the same ACPI device node, which may also has some potential risks.
Thus, IMO, we need a clean way to handle those motherboard resources.
Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
resources, this patch
1. does the resource reservation in ACPI code directly, with the same logic
and time point in drivers/pnp/quirks.c and drivers/pnp/system.c.
2. makes PNP0C01/PNP0C02 PNP id transparent to Linux devices and drivers,
thus PNP system driver becomes a no-op for ACPI enumerated devices.
This is just a draft patch, and I'd like to see if this is
the right direction to go. Any comments are welcome.
Signed-off-by: Zhang Rui rui.zh...@intel.com
---
drivers/acpi/acpi_pnp.c | 3 -
drivers/acpi/scan.c | 208 +++-
2 files changed, 204 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 1f8b204..a7deae5 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -134,9 +134,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[] = {
{FUJ02bf},
{FUJ02B1},
{FUJ02E3},
- /* system */
- {PNP0c02},/* General ID for reserving resources */
- {PNP0c01},/* memory controller */
/* rtc_cmos */
{PNP0b00},
{PNP0b01},
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0a817ad..674518b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -11,6 +11,7 @@
#include linux/kthread.h
#include linux/dmi.h
#include linux/nls.h
+#include linux/pci.h
#include asm/pgtable.h
@@ -1781,12 +1782,201 @@ static bool acpi_object_is_system_bus(acpi_handle
handle)
return false;
}
+static bool acpi_is_motherboard_resource(char *id)
+{
+
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote: Hi, Rafael, On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com We generally don't allow ACPI drivers to bind to ACPI device objects that companion physical device objects are created for to avoid situations in which two different drivers may attempt to handle one device at the same time. Yes, and I think we should not break this rule. No, we broke the way the code worked previously. We need to restore it first and *then* try to fix it up, not the other way around. Recent ACPI device enumeration rework extended that approach to ACPI PNP devices by starting to use a scan handler for enumerating them. However, we previously allowed ACPI drivers to bind to ACPI device objects with existing PNP device companions and changing that led to functional regressions on some systems. Question: except the PNP0C01/PNP0C02 case, if we have an device have two ids that matches two different drivers, should we allow both drivers probe successfully? No, we shouldn't, but in the cases in question we have only one driver (an ACPI one). I think the answer is no. In the PNP0C01/PNP0C02 case, I think we can fix the issue by the following patch instead. The right answer to me is to get rid of ACPI drviers entirely. The thing below adds complexity to the resources management which I'm not sure is necessary. In any case, please let's do that on top of the $subject patch not instead of it, because there are more cases we need to cover. And we need a fix for -stable. Rafael -- 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 / scan: Allow ACPI drivers to bind to PNP device objects
On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote:
Hi, Rafael,
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
[cut]
Note that I've just tested on my machine and it works well.
I still need the bug reporter to check if the patch fixes bug 81511 or not.
The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and
they aren't motherboard devices. Yes, we need to convert that driver
to use a PNP driver structure or a platform device, but (1) we need a
-stable fix *first* and (2) the cases we already know about may not be
the only broken ones.
From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
From: Zhang Rui rui.zh...@intel.com
Date: Thu, 21 Aug 2014 13:39:47 +0800
Subject: [RFC PATCH] ACPI: introduce motherboard resource management
ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
they have some motherboard resources that needs to be reserved.
We used to enumerated those devices to PNP bus and rely on
PNP system driver to do resource reservation.
But this mechanism does not work well nowadays as many devices
not only represent motherboard resources, but also represent
physical devices that need native drivers other than PNP system
driver for the device to work. For example,
1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
Device (NIPM)
{
Name (_HID, EisaId (IPI0001)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C01)) // _CID: Compatible ID
the NIPM device has _CID PNP0C01 but it is an IPMI device.
PNP system driver blocks the PNP IPMI driver to probe.
That is a good reason for PNP0C01 to be dropped from acpi_pnp_device_ids[].
2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
Device (IFFS)
{
Name (_HID, EisaId (INT3392)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the IFFS device has _CID PNP0C02, but it is an intel rapid start
device, which already has an ACPI driver at
drivers/platform/x86/intel-rst.c
And which should be a platform driver really.
3) a couple of machines, including the on in
https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
like following
Device (PTID)
{
Name (_HID, EisaId (INT340E)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the PTID device has _CID PNP0C02, but it is also represents an
INT340E device, there is a platform bus driver for this device
which will be introduced by myself soon.
Again, that's a good reason for dropping PNP0C02 from acpi_pnp_device_ids[].
In any of the above cases, the current code for managing PNP0C01/PNP0C02
resources in Linux kernel is broken, because it either blocks the physical
device driver on the same bus, or results in multiple drivers loaded for
the same ACPI device node, which may also has some potential risks.
Thus, IMO, we need a clean way to handle those motherboard resources.
Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
resources, this patch
1. does the resource reservation in ACPI code directly, with the same logic
and time point in drivers/pnp/quirks.c and drivers/pnp/system.c.
2. makes PNP0C01/PNP0C02 PNP id transparent to Linux devices and drivers,
thus PNP system driver becomes a no-op for ACPI enumerated devices.
This is just a draft patch, and I'd like to see if this is
the right direction to go. Any comments are welcome.
Signed-off-by: Zhang Rui rui.zh...@intel.com
---
drivers/acpi/acpi_pnp.c | 3 -
drivers/acpi/scan.c | 208
+++-
2 files changed, 204 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 1f8b204..a7deae5 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -134,9 +134,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]
= {
{FUJ02bf},
{FUJ02B1},
{FUJ02E3},
- /* system */
- {PNP0c02},/* General ID for reserving resources */
- {PNP0c01},/* memory controller */
/* rtc_cmos */
{PNP0b00},
{PNP0b01},
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0a817ad..674518b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -11,6 +11,7 @@
#include linux/kthread.h
#include linux/dmi.h
#include linux/nls.h
+#include linux/pci.h
#include asm/pgtable.h
@@ -1781,12 +1782,201 @@ static bool acpi_object_is_system_bus(acpi_handle
handle)
return false;
}
+static bool acpi_is_motherboard_resource(char *id)
+{
+ return !(strncmp(id, PNP0C01, sizeof(PNP0C01))
+ strncmp(id, PNP0C02, sizeof(PNP0C02)));
+}
Can we use __acpi_match_device() for that?
+
+static LIST_HEAD(acpi_motherboard_resource_list);
+
+struct
Re: [PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
On Thu, 2014-08-21 at 19:10 +0200, Rafael J. Wysocki wrote:
On Thursday, August 21, 2014 08:08:54 PM Zhang Rui wrote:
Hi, Rafael,
On Thu, 2014-08-21 at 06:04 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
[cut]
Note that I've just tested on my machine and it works well.
I still need the bug reporter to check if the patch fixes bug 81511 or not.
The FUJ02B1 and FUJ02E3 devices in bug 81971 have the same problem and
they aren't motherboard devices.
Right, but IMO, the rootcause of that bug is that
1. the PNP id table in fujitsu-laptop driver was introduced for some
reason, probably it is used as an indicator for module auto-loading, and
nowadays, this is redundant because fujitsu-laptop driver probes ACPI
device only, and the driver will be loaded if the ACPI device objects
for FUJ02B1 and FUJ02E3 is created.
2. This redundant PNP id table results in that those IDs are added to
PNP ID list unnecessarily, and results in PNP device nodes for those
devices are created unnecessarily.
Yes, we need to convert that driver
to use a PNP driver structure or a platform device, but (1) we need a
-stable fix *first*
I agree.
and (2) the cases we already know about may not be
the only broken ones.
Agree.
But the issue addressed in your patch is that PNP scan handler blocks
ACPI driver from being probed, right?
So my question would be,
1. If the id in PNP scan handler does not have a PNP driver, like the
FUJ02B1/FUJ02E3 issue, what do we need the id in PNP scan handler?
In fact, I think this is a good chance for us to cleanup the ACPI PNP
id list, as long as we can fix them in time.
2. If the id in PNP scan handler has a PNP driver, should we allow both
PNP driver and ACPI driver loaded? I think PNP system driver is the
only case that makes us have to say yes, and what I'm trying to do
is to fix this in the following patch.
Plus, IMO, your patch only fixes the PNP bus vs. ACPI bus issue. We
still may get bug report complaining some *PLATFORM* driver stops to
functional if the ACPI node has _CID PNP0C01/PNP0C02, sooner or later.
right?
thanks,
rui
From c6c388728d08a6368f21dab61d6f0a940e0ea13a Mon Sep 17 00:00:00 2001
From: Zhang Rui rui.zh...@intel.com
Date: Thu, 21 Aug 2014 13:39:47 +0800
Subject: [RFC PATCH] ACPI: introduce motherboard resource management
ACPI Devices with _HID/_CID PNP0C01/PNP0C02 represents that
they have some motherboard resources that needs to be reserved.
We used to enumerated those devices to PNP bus and rely on
PNP system driver to do resource reservation.
But this mechanism does not work well nowadays as many devices
not only represent motherboard resources, but also represent
physical devices that need native drivers other than PNP system
driver for the device to work. For example,
1) https://bugzilla.kernel.org/show_bug.cgi?id=46741,
Device (NIPM)
{
Name (_HID, EisaId (IPI0001)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C01)) // _CID: Compatible ID
the NIPM device has _CID PNP0C01 but it is an IPMI device.
PNP system driver blocks the PNP IPMI driver to probe.
That is a good reason for PNP0C01 to be dropped from acpi_pnp_device_ids[].
2) https://bugzilla.kernel.org/show_bug.cgi?id=81511
Device (IFFS)
{
Name (_HID, EisaId (INT3392)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the IFFS device has _CID PNP0C02, but it is an intel rapid start
device, which already has an ACPI driver at
drivers/platform/x86/intel-rst.c
And which should be a platform driver really.
3) a couple of machines, including the on in
https://bugzilla.kernel.org/show_bug.cgi?id=81511, has the AML code
like following
Device (PTID)
{
Name (_HID, EisaId (INT340E)) // _HID: Hardware ID
Name (_CID, EisaId (PNP0C02)) // _CID: Compatible ID
the PTID device has _CID PNP0C02, but it is also represents an
INT340E device, there is a platform bus driver for this device
which will be introduced by myself soon.
Again, that's a good reason for dropping PNP0C02 from acpi_pnp_device_ids[].
In any of the above cases, the current code for managing PNP0C01/PNP0C02
resources in Linux kernel is broken, because it either blocks the physical
device driver on the same bus, or results in multiple drivers loaded for
the same ACPI device node, which may also has some potential risks.
Thus, IMO, we need a clean way to handle those motherboard resources.
Given that PNP0C01/PNP0C02 is more like an indicator for reserving the
resources, this patch
1. does the resource reservation in ACPI code directly, with the same logic
and time point in drivers/pnp/quirks.c and drivers/pnp/system.c.
2. makes PNP0C01/PNP0C02 PNP id transparent to Linux devices and drivers,
thus PNP
[PATCH] ACPI / scan: Allow ACPI drivers to bind to PNP device objects
From: Rafael J. Wysocki
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion "physical" device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time. Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
For this reason, add a special check for PNP devices in
acpi_device_probe() so that ACPI drivers can bind to ACPI device
objects having existing PNP device companions as before.
Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
enumeration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Reported-and-tested-by: Gabriele Mazzotta
Reported-and-tested-by: Dirk Griesbach
Signed-off-by: Rafael J. Wysocki
---
drivers/acpi/acpi_pnp.c |5 +
drivers/acpi/internal.h |1 +
drivers/acpi/scan.c |2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/acpi_pnp.c
===
--- linux-pm.orig/drivers/acpi/acpi_pnp.c
+++ linux-pm/drivers/acpi/acpi_pnp.c
@@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
{
acpi_scan_add_handler(_pnp_handler);
}
+
+bool is_acpi_pnp_device(struct acpi_device *adev)
+{
+ return adev->handler == _pnp_handler;
+}
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -86,6 +86,7 @@ void acpi_device_add_finalize(struct acp
void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
bool acpi_device_is_present(struct acpi_device *adev);
bool acpi_device_is_battery(struct acpi_device *adev);
+bool is_acpi_pnp_device(struct acpi_device *adev);
/* --
Power Resource
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
int ret;
- if (acpi_dev->handler)
+ if (acpi_dev->handler && !is_acpi_pnp_device(acpi_dev))
return -EINVAL;
if (!acpi_drv->ops.add)
--
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 / scan: Allow ACPI drivers to bind to PNP device objects
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
We generally don't allow ACPI drivers to bind to ACPI device objects
that companion physical device objects are created for to avoid
situations in which two different drivers may attempt to handle one
device at the same time. Recent ACPI device enumeration rework
extended that approach to ACPI PNP devices by starting to use a scan
handler for enumerating them. However, we previously allowed ACPI
drivers to bind to ACPI device objects with existing PNP device
companions and changing that led to functional regressions on some
systems.
For this reason, add a special check for PNP devices in
acpi_device_probe() so that ACPI drivers can bind to ACPI device
objects having existing PNP device companions as before.
Fixes: eec15edbb0e1 (ACPI / PNP: use device ID list for PNPACPI device
enumeration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81511
Link: https://bugzilla.kernel.org/show_bug.cgi?id=81971
Reported-and-tested-by: Gabriele Mazzotta gabriele@gmail.com
Reported-and-tested-by: Dirk Griesbach spamt...@freenet.de
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/acpi_pnp.c |5 +
drivers/acpi/internal.h |1 +
drivers/acpi/scan.c |2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/acpi_pnp.c
===
--- linux-pm.orig/drivers/acpi/acpi_pnp.c
+++ linux-pm/drivers/acpi/acpi_pnp.c
@@ -396,3 +396,8 @@ void __init acpi_pnp_init(void)
{
acpi_scan_add_handler(acpi_pnp_handler);
}
+
+bool is_acpi_pnp_device(struct acpi_device *adev)
+{
+ return adev-handler == acpi_pnp_handler;
+}
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -86,6 +86,7 @@ void acpi_device_add_finalize(struct acp
void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
bool acpi_device_is_present(struct acpi_device *adev);
bool acpi_device_is_battery(struct acpi_device *adev);
+bool is_acpi_pnp_device(struct acpi_device *adev);
/* --
Power Resource
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -975,7 +975,7 @@ static int acpi_device_probe(struct devi
struct acpi_driver *acpi_drv = to_acpi_driver(dev-driver);
int ret;
- if (acpi_dev-handler)
+ if (acpi_dev-handler !is_acpi_pnp_device(acpi_dev))
return -EINVAL;
if (!acpi_drv-ops.add)
--
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/
