Re: [PATCH] drivers/platform/x86: Add driver for INT0002 ACPI device

[Takashi Iwai <tiwai@suse.de>]

On Fri, 21 Apr 2017 09:41:57 +0200,
Hans de Goede wrote:
> 
> Some peripherals on Baytrail and Cherrytrail platforms signal PME to the
> PMC to wakeup the system. When this happens software needs to clear the
> PME_B0_STS bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
> 
> This is modelled in ACPI through the INT0002 ACPI device.
> 
> This commit adds a driver which will bind to that device, call the
> ACPI event handler for the wakeup and clear the interrupt source
> avoiding the irq storm.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This patch requires your previous "always present" patchset, right?

Some nitpicking:

> --- /dev/null
> +++ b/drivers/platform/x86/intel_int0002.c
...
> +struct int0002_data {
> +	struct spinlock lock;
> +	struct device *dev;
> +	const struct x86_cpu_id *cpu_id;
> +	acpi_handle handle;
> +	char ev_name[5];

Are 5 bytes enough?  I see the code below:

> +		snprintf(data->ev_name, sizeof(data->ev_name), "_%c%02X",
> +			res->data.gpio.triggering ? 'E' : 'L',
> +			res->data.gpio.pin_table[0]);

So it counts 6 including NUL.

> +static int int0002_probe(struct platform_device *pdev)
> +{
> +	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct device *dev = &pdev->dev;
> +	struct int0002_data *data;
> +	struct acpi_resource *res;
> +	acpi_status status;
> +	acpi_handle hdl;
> +	int irq, ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		dev_err(dev, "can't allocate memory for int0002\n");

The error message is mostly superfluous.

> +static int int0002_runtime_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int int0002_runtime_resume(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops int0002_pm_ops = {
> +	.runtime_suspend = int0002_runtime_suspend,
> +	.runtime_resume = int0002_runtime_resume,
> +};

Do we need these runtime PM?  If not, we can remove the header
inclusion, too.


Other than that, I confirmed to work on my ASUS E200H.
  Tested-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi