| Message ID | 1393405874-3266-9-git-send-email-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
Hi, Bjorn, do you have comments for this particular patch? thanks, rui On Wed, 2014-02-26 at 17:11 +0800, Zhang Rui wrote: > ACPI devices with id "PNP0C01/PNP0C02" means that we need to > protect their resources from being allocated by others. > > Currently, this is done in drivers/pnp/system.c. > > But the problem is that, there are some devices with extra ids besides > PNP0C01/PNP0C02, and for these devices, > 1) PNP0C01/PNP0C02 suggest that resource reservation is still needed. > 2) the other ids suggest that we should enumerate them to platform bus > > To reserve resources for those devices, we should either use the current code > by exporting the device to PNP bus, or introduce resource reservation support > in platform bus/ACPI. > > This patch follows the first way by enumerating an ACPI device to platform bus > AND pnp bus at the same time. > Its PNP device node will be probed by drivers/pnp/system.c and do > everything as we do today. > Its platform device node will also be created so that a platform bus > driver can be probed. > > The advantage is that, it brings little change to the current code, > the patch itself looks safe and clear. > The disadvantage is that > 1) we create two physical device nodes for the same ACPI node, > this is against our effort that has been doing recently. > 2) we still depend on PNP bus to do this (resouce reservation) for us, > which is still a problem we need to fix sooner or later. > > An alternative proposal is to remove the depedency of PNP bus and > do resource management in ACPI for all PNP0C01/PNP0C02 devices instead, > no matter what bus they are enumerated to. > To do this, we need to > 1) introduce a fs_initcall() in ACPI, to reserve all PNP0C01/PNP0C02 resources > in ACPI, something like we did via drivers/acpi/motherboard.c before > (but the code needs to follow drivers/pnp/quirks.c and system.c strictly). > This initcall will be run after PCI claiming BARs and before PCI assigning > resources for uninitialized devices. > 2) skip drivers/pnp/quirks.c and drivers/pnp/system.c for ACPI > enumerted PNP devices, by checking pnp_device->protocal. > 3) remove PNP0C01/PNP0C02 from PNPACPI white list. > > By doing this, we can remove the depedency of PNP bus, but this requires > a lot of code duplication(need to copy quirks.c and system.c logic into ACPI), > which does not look good neither. > > Any comments will be appreciated. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/pnp/pnpacpi/core.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c > index 739fa24..5b13600 100644 > --- a/drivers/pnp/pnpacpi/core.c > +++ b/drivers/pnp/pnpacpi/core.c > @@ -173,9 +173,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]= { > {"FUJ02bf"}, > {"FUJ02B1"}, > {"FUJ02E3"}, > - /* system */ > - {"PNP0c02"}, /* General ID for reserving resources */ > - {"PNP0c01"}, /* memory controller */ > /* c6xdigio */ > {"PNP0400"}, /* Standard LPT Printer Port */ > {"PNP0401"}, /* ECP Printer Port */ > @@ -677,6 +674,20 @@ static int is_cmos_rtc_device(struct acpi_device *adev) > return !acpi_match_device_ids(adev, ids); > } > > +/* > + * For devices with id "PNP0C01"/"PNP0C02", they will be enumerated > + * to PNP bus anyway to do resource reservation. > + */ > +static int is_system_device(struct acpi_device *adev) > +{ > + struct acpi_device_id ids[] = { > + {"PNP0C02"}, > + {"PNP0C01"}, > + {""}, > + }; > + return !acpi_match_device_ids(adev, ids); > +} > + > static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, > u32 lvl, void *context, > void **rv) > @@ -685,7 +696,8 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, > > if (acpi_bus_get_device(handle, &device)) > return AE_CTRL_DEPTH; > - if (device->handler == &pnpacpi_handler || is_cmos_rtc_device(device)) > + if (device->handler == &pnpacpi_handler || is_system_device(device) || > + is_cmos_rtc_device(device)) > pnpacpi_add_device(device); > return AE_OK; > } -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 3, 2014 at 7:17 AM, Zhang Rui <rui.zhang@intel.com> wrote: > Hi, Bjorn, > > do you have comments for this particular patch? Nope, I'm not paying attention to this area any more. > On Wed, 2014-02-26 at 17:11 +0800, Zhang Rui wrote: >> ACPI devices with id "PNP0C01/PNP0C02" means that we need to >> protect their resources from being allocated by others. >> >> Currently, this is done in drivers/pnp/system.c. >> >> But the problem is that, there are some devices with extra ids besides >> PNP0C01/PNP0C02, and for these devices, >> 1) PNP0C01/PNP0C02 suggest that resource reservation is still needed. >> 2) the other ids suggest that we should enumerate them to platform bus >> >> To reserve resources for those devices, we should either use the current code >> by exporting the device to PNP bus, or introduce resource reservation support >> in platform bus/ACPI. >> >> This patch follows the first way by enumerating an ACPI device to platform bus >> AND pnp bus at the same time. >> Its PNP device node will be probed by drivers/pnp/system.c and do >> everything as we do today. >> Its platform device node will also be created so that a platform bus >> driver can be probed. >> >> The advantage is that, it brings little change to the current code, >> the patch itself looks safe and clear. >> The disadvantage is that >> 1) we create two physical device nodes for the same ACPI node, >> this is against our effort that has been doing recently. >> 2) we still depend on PNP bus to do this (resouce reservation) for us, >> which is still a problem we need to fix sooner or later. >> >> An alternative proposal is to remove the depedency of PNP bus and >> do resource management in ACPI for all PNP0C01/PNP0C02 devices instead, >> no matter what bus they are enumerated to. >> To do this, we need to >> 1) introduce a fs_initcall() in ACPI, to reserve all PNP0C01/PNP0C02 resources >> in ACPI, something like we did via drivers/acpi/motherboard.c before >> (but the code needs to follow drivers/pnp/quirks.c and system.c strictly). >> This initcall will be run after PCI claiming BARs and before PCI assigning >> resources for uninitialized devices. >> 2) skip drivers/pnp/quirks.c and drivers/pnp/system.c for ACPI >> enumerted PNP devices, by checking pnp_device->protocal. >> 3) remove PNP0C01/PNP0C02 from PNPACPI white list. >> >> By doing this, we can remove the depedency of PNP bus, but this requires >> a lot of code duplication(need to copy quirks.c and system.c logic into ACPI), >> which does not look good neither. >> >> Any comments will be appreciated. >> >> Signed-off-by: Zhang Rui <rui.zhang@intel.com> >> --- >> drivers/pnp/pnpacpi/core.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c >> index 739fa24..5b13600 100644 >> --- a/drivers/pnp/pnpacpi/core.c >> +++ b/drivers/pnp/pnpacpi/core.c >> @@ -173,9 +173,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]= { >> {"FUJ02bf"}, >> {"FUJ02B1"}, >> {"FUJ02E3"}, >> - /* system */ >> - {"PNP0c02"}, /* General ID for reserving resources */ >> - {"PNP0c01"}, /* memory controller */ >> /* c6xdigio */ >> {"PNP0400"}, /* Standard LPT Printer Port */ >> {"PNP0401"}, /* ECP Printer Port */ >> @@ -677,6 +674,20 @@ static int is_cmos_rtc_device(struct acpi_device *adev) >> return !acpi_match_device_ids(adev, ids); >> } >> >> +/* >> + * For devices with id "PNP0C01"/"PNP0C02", they will be enumerated >> + * to PNP bus anyway to do resource reservation. >> + */ >> +static int is_system_device(struct acpi_device *adev) >> +{ >> + struct acpi_device_id ids[] = { >> + {"PNP0C02"}, >> + {"PNP0C01"}, >> + {""}, >> + }; >> + return !acpi_match_device_ids(adev, ids); >> +} >> + >> static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, >> u32 lvl, void *context, >> void **rv) >> @@ -685,7 +696,8 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, >> >> if (acpi_bus_get_device(handle, &device)) >> return AE_CTRL_DEPTH; >> - if (device->handler == &pnpacpi_handler || is_cmos_rtc_device(device)) >> + if (device->handler == &pnpacpi_handler || is_system_device(device) || >> + is_cmos_rtc_device(device)) >> pnpacpi_add_device(device); >> return AE_OK; >> } > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 739fa24..5b13600 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -173,9 +173,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[]= { {"FUJ02bf"}, {"FUJ02B1"}, {"FUJ02E3"}, - /* system */ - {"PNP0c02"}, /* General ID for reserving resources */ - {"PNP0c01"}, /* memory controller */ /* c6xdigio */ {"PNP0400"}, /* Standard LPT Printer Port */ {"PNP0401"}, /* ECP Printer Port */ @@ -677,6 +674,20 @@ static int is_cmos_rtc_device(struct acpi_device *adev) return !acpi_match_device_ids(adev, ids); } +/* + * For devices with id "PNP0C01"/"PNP0C02", they will be enumerated + * to PNP bus anyway to do resource reservation. + */ +static int is_system_device(struct acpi_device *adev) +{ + struct acpi_device_id ids[] = { + {"PNP0C02"}, + {"PNP0C01"}, + {""}, + }; + return !acpi_match_device_ids(adev, ids); +} + static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, u32 lvl, void *context, void **rv) @@ -685,7 +696,8 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle, if (acpi_bus_get_device(handle, &device)) return AE_CTRL_DEPTH; - if (device->handler == &pnpacpi_handler || is_cmos_rtc_device(device)) + if (device->handler == &pnpacpi_handler || is_system_device(device) || + is_cmos_rtc_device(device)) pnpacpi_add_device(device); return AE_OK; }
ACPI devices with id "PNP0C01/PNP0C02" means that we need to protect their resources from being allocated by others. Currently, this is done in drivers/pnp/system.c. But the problem is that, there are some devices with extra ids besides PNP0C01/PNP0C02, and for these devices, 1) PNP0C01/PNP0C02 suggest that resource reservation is still needed. 2) the other ids suggest that we should enumerate them to platform bus To reserve resources for those devices, we should either use the current code by exporting the device to PNP bus, or introduce resource reservation support in platform bus/ACPI. This patch follows the first way by enumerating an ACPI device to platform bus AND pnp bus at the same time. Its PNP device node will be probed by drivers/pnp/system.c and do everything as we do today. Its platform device node will also be created so that a platform bus driver can be probed. The advantage is that, it brings little change to the current code, the patch itself looks safe and clear. The disadvantage is that 1) we create two physical device nodes for the same ACPI node, this is against our effort that has been doing recently. 2) we still depend on PNP bus to do this (resouce reservation) for us, which is still a problem we need to fix sooner or later. An alternative proposal is to remove the depedency of PNP bus and do resource management in ACPI for all PNP0C01/PNP0C02 devices instead, no matter what bus they are enumerated to. To do this, we need to 1) introduce a fs_initcall() in ACPI, to reserve all PNP0C01/PNP0C02 resources in ACPI, something like we did via drivers/acpi/motherboard.c before (but the code needs to follow drivers/pnp/quirks.c and system.c strictly). This initcall will be run after PCI claiming BARs and before PCI assigning resources for uninitialized devices. 2) skip drivers/pnp/quirks.c and drivers/pnp/system.c for ACPI enumerted PNP devices, by checking pnp_device->protocal. 3) remove PNP0C01/PNP0C02 from PNPACPI white list. By doing this, we can remove the depedency of PNP bus, but this requires a lot of code duplication(need to copy quirks.c and system.c logic into ACPI), which does not look good neither. Any comments will be appreciated. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/pnp/pnpacpi/core.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)