linux-kernel - Re: [RFC PATCH 8/8] PNPACPI: create both PNP and Platform device nodes for PNP0C01/PNP0C02

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 03 Mar 2014 22:17:46 +0800
From:	Zhang Rui <rui.zhang@...el.com>
To:	linux-acpi@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, bhelgaas@...gle.com,
	matthew.garrett@...ula.com, rafael.j.wysocki@...el.com,
	dmitry.torokhov@...il.com
Subject: Re: [RFC PATCH 8/8] PNPACPI: create both PNP and Platform device
 nodes for PNP0C01/PNP0C02

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@...el.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/