From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932347AbcISN7E (ORCPT ); Mon, 19 Sep 2016 09:59:04 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34571 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756640AbcISN64 (ORCPT ); Mon, 19 Sep 2016 09:58:56 -0400 From: Nicolai Stange To: Mika Westerberg Cc: Nicolai Stange , Wolfram Sang , Octavian Purdila , "Rafael J. Wysocki" , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [REGRESSION? v4.8] i2c-core: acpi_i2c_get_info() touches non-existent devices References: <87a8f4kfhe.fsf@gmail.com> <20160919084807.GC1811@lahna.fi.intel.com> <20160919130324.GI1811@lahna.fi.intel.com> Date: Mon, 19 Sep 2016 15:58:52 +0200 In-Reply-To: <20160919130324.GI1811@lahna.fi.intel.com> (Mika Westerberg's message of "Mon, 19 Sep 2016 16:03:24 +0300") Message-ID: <87fuowf0tf.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mika Westerberg writes: > On Mon, Sep 19, 2016 at 11:48:07AM +0300, Mika Westerberg wrote: >> On Mon, Sep 19, 2016 at 12:30:53AM +0200, Nicolai Stange wrote: >> > I'm encountering the following: >> > >> > [ 10.409490] ERROR: Unable to locate IOAPIC for GSI 37 >> > >> > Note that the system works fine, so it's a "cosmetic" regression, I think. >> > >> > >> > I added a dump_stack() right below the printk() in question and it reads >> > as >> > >> > [ 10.410290] CPU: 6 PID: 710 Comm: systemd-udevd Not tainted 4.7.0-rc4+ #348 >> > [ 10.410962] Hardware name: Dell Inc. Latitude E6540/0725FP, BIOS A10 06/26/2014 >> > [ 10.411772] 0000000000000286 00000000b9050627 ffff8800c2e5f590 ffffffffa54161e7 >> > [ 10.412569] 0000000000000025 0000000000000001 ffff8800c2e5f5a0 ffffffffa50465df >> > [ 10.413292] ffff8800c2e5f5d0 ffffffffa5046ffd 0000000000000000 0000000000000025 >> > [ 10.414016] Call Trace: >> > [ 10.414713] [] dump_stack+0x68/0xa1 >> > [ 10.415406] [] mp_find_ioapic+0x4f/0x60 >> > [ 10.416131] [] mp_map_gsi_to_irq+0x1d/0xc0 >> > [ 10.416806] [] acpi_register_gsi_ioapic+0x7b/0x170 >> > [ 10.417494] [] acpi_register_gsi+0xf/0x20 >> > [ 10.418217] [] acpi_dev_get_irqresource.part.3+0xd7/0x11d >> > [ 10.418871] [] ? acpi_dev_resource_address_space+0x31/0x67 >> > [ 10.419655] [] acpi_dev_resource_interrupt+0x9b/0xab >> > [ 10.420408] [] acpi_dev_process_resource+0xbc/0xf7 >> > [ 10.421070] [] ? acpi_dev_resource_memory+0x7c/0x7c >> > [ 10.421732] [] acpi_walk_resource_buffer+0x4d/0x85 >> > [ 10.422399] [] ? acpi_dev_resource_memory+0x7c/0x7c >> > [ 10.423158] [] acpi_walk_resources+0x83/0xb6 >> > [ 10.423831] [] acpi_dev_get_resources+0x96/0xd7 >> > [ 10.424505] [] acpi_i2c_get_info+0xe4/0x1a0 >> > [ 10.425181] [] acpi_i2c_add_device+0x56/0xa0 >> > [ 10.425856] [] acpi_ns_walk_namespace+0xe8/0x19d >> > [ 10.426564] [] ? acpi_i2c_register_device+0x70/0x70 >> > [ 10.427418] [] ? acpi_i2c_register_device+0x70/0x70 >> > [ 10.428179] [] acpi_walk_namespace+0xa0/0xd5 >> > [ 10.428858] [] i2c_register_adapter+0x369/0x500 >> > [ 10.429499] [] i2c_add_adapter+0x5c/0x70 >> > [ 10.430125] [] i801_probe+0x2bd/0x6a0 [i2c_i801] >> > I bisected this to commit 525e6fabeae2 ("i2c / ACPI: add support for >> > ACPI reconfigure notifications"). >> > >> > The reason for the above message seems to be that acpi_i2c_get_info() >> > configures the IRQs for any ACPI devices that have got some >> > I2cSerialBus() resource, regardless of the actual adapter those are >> > attached to. This behaviour is different from before that commit. >> > >> > My ACPI DSDT has got a PCI I2C adapter that isn't physically present, it >> > seems. No clue why. >> > >> > That non-existent PCI I2C adapter is in turn I2cSerialBus()-referenced >> > by some ACPI device that has got exactly this interrupt 37 assigned. >> > >> > So it looks like an attempt is made to configure this non-existent, >> > ACPI-listed I2C slave's IRQs when an actually existing I2C adapter (i801 >> > SMBus) gets probed. > Can you try if the following patch cures the problem? Unfortunately not. That patch installs the check after the acpi_i2c_get_info() invocation which is part of the backtrace above. I moved your check into i2c_get_info(), right in front of the IRQ handling and this works. So, Tested-by: Nicolai Stange for this: diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 74e5aea..3f2b3cf 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -141,6 +141,7 @@ static int acpi_i2c_get_info(struct acpi_device *adev, struct list_head resource_list; struct resource_entry *entry; struct acpi_i2c_lookup lookup; + struct acpi_device *adapter_adev; int ret; if (acpi_bus_get_status(adev) || !adev->status.present || @@ -163,6 +164,12 @@ static int acpi_i2c_get_info(struct acpi_device *adev, if (ret < 0 || !info->addr) return -EINVAL; + /* The adapter must be present */ + if (acpi_bus_get_device(lookup.adapter_handle, &adapter_adev)) + return -EINVAL; + if (acpi_bus_get_status(adapter_adev) || !adapter_adev->status.present) + return -EINVAL;; + *adapter_handle = lookup.adapter_handle; /* Then fill IRQ number if any */ But it is still true that acpi_i2c_register_devices() configures the interrupts for all ACPI I2C slaves attached to an available adapter, independent of whether their adapter is the one given as an argument or not. I can't tell whether this is desired, just a note...