| Message ID | 9f06a7ce1cc6dd4cb33004c7d37adc1a5c0f80e2.1498636759.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wed, Jun 28, 2017 at 07:20:19PM +0200, Lukas Wunner wrote: > SPI and I2C slaves are enumerated by their respective parents rather > than the ACPI core. They are recognized by presence of _CRS resources, > which however are missing on Macs. Check for presence of device > properties instead. > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Federico Lorenzi <florenzi@gmail.com> > Reported-and-tested-by: Ronald Tschalär <ronald@innovation.ch> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Changes v1 -> v2: > - Newly inserted patch in v2 to fix an enumeration issue. (Ronald) > > drivers/acpi/scan.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index d4ff2cd1f738..c565e74bd2dd 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1444,6 +1444,12 @@ static bool acpi_is_spi_i2c_slave(struct acpi_device *device) > struct list_head resource_list; > bool is_spi_i2c_slave = false; > > + /* Macs use device properties in lieu of _CRS resources */ > + if (IS_ENABLED(CONFIG_X86) && dmi_match(DMI_SYS_VENDOR, "Apple Inc.") && Do we really need these checks? > + (device_property_present(&device->dev, "spiSclkPeriod") || > + device_property_present(&device->dev, "i2cAddress"))) > + return true; > + > INIT_LIST_HEAD(&resource_list); > acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave, > &is_spi_i2c_slave); > -- > 2.11.0 -- 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 Thu, Jun 29, 2017 at 10:34:20AM +0300, Mika Westerberg wrote: > On Wed, Jun 28, 2017 at 07:20:19PM +0200, Lukas Wunner wrote: > > --- a/drivers/acpi/scan.c > > +++ b/drivers/acpi/scan.c > > @@ -1444,6 +1444,12 @@ static bool acpi_is_spi_i2c_slave(struct acpi_device *device) > > struct list_head resource_list; > > bool is_spi_i2c_slave = false; > > > > + /* Macs use device properties in lieu of _CRS resources */ > > + if (IS_ENABLED(CONFIG_X86) && dmi_match(DMI_SYS_VENDOR, "Apple Inc.") && > > Do we really need these checks? With these checks present, searching for the properties can be optimized away on ARM and skipped on x86 non-Macs. (Each property query requires decoding the _DSD Package and performing an O(n) search of the properties.) So I'd say yes? Thanks, Lukas > > > + (device_property_present(&device->dev, "spiSclkPeriod") || > > + device_property_present(&device->dev, "i2cAddress"))) > > + return true; > > + > > INIT_LIST_HEAD(&resource_list); > > acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave, > > &is_spi_i2c_slave); > > -- > > 2.11.0 -- 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 Thu, Jun 29, 2017 at 10:46:04AM +0200, Lukas Wunner wrote: > On Thu, Jun 29, 2017 at 10:34:20AM +0300, Mika Westerberg wrote: > > On Wed, Jun 28, 2017 at 07:20:19PM +0200, Lukas Wunner wrote: > > > --- a/drivers/acpi/scan.c > > > +++ b/drivers/acpi/scan.c > > > @@ -1444,6 +1444,12 @@ static bool acpi_is_spi_i2c_slave(struct acpi_device *device) > > > struct list_head resource_list; > > > bool is_spi_i2c_slave = false; > > > > > > + /* Macs use device properties in lieu of _CRS resources */ > > > + if (IS_ENABLED(CONFIG_X86) && dmi_match(DMI_SYS_VENDOR, "Apple Inc.") && > > > > Do we really need these checks? > > With these checks present, searching for the properties can be optimized > away on ARM and skipped on x86 non-Macs. (Each property query requires > decoding the _DSD Package and performing an O(n) search of the properties.) Well, you add dmi_match() that gets called every time and I was under the impression that we already performed _DSD decode when the property set was initially parsed. Those checks just uglify the code IMHO. No strong feelings though, so up to Rafael to decide :-) -- 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/acpi/scan.c b/drivers/acpi/scan.c index d4ff2cd1f738..c565e74bd2dd 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1444,6 +1444,12 @@ static bool acpi_is_spi_i2c_slave(struct acpi_device *device) struct list_head resource_list; bool is_spi_i2c_slave = false; + /* Macs use device properties in lieu of _CRS resources */ + if (IS_ENABLED(CONFIG_X86) && dmi_match(DMI_SYS_VENDOR, "Apple Inc.") && + (device_property_present(&device->dev, "spiSclkPeriod") || + device_property_present(&device->dev, "i2cAddress"))) + return true; + INIT_LIST_HEAD(&resource_list); acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave, &is_spi_i2c_slave);
SPI and I2C slaves are enumerated by their respective parents rather than the ACPI core. They are recognized by presence of _CRS resources, which however are missing on Macs. Check for presence of device properties instead. Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Federico Lorenzi <florenzi@gmail.com> Reported-and-tested-by: Ronald Tschalär <ronald@innovation.ch> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- Changes v1 -> v2: - Newly inserted patch in v2 to fix an enumeration issue. (Ronald) drivers/acpi/scan.c | 6 ++++++ 1 file changed, 6 insertions(+)