* ACPI: Can I use I2cSerialBus with a PCI I2C controller? @ 2015-10-20 19:47 Ben Gardner 2015-10-21 8:50 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-20 19:47 UTC (permalink / raw) To: linux-acpi Hi everyone, I am building a custom board based on the Bayley Bay reference design, using the Intel E3845 CPU. I have a few I2C devices on the board that are connected to I2C buses 1-3. The I2C controllers use the i2c-designware-pci driver. I am currently using i2c_register_board_info() in a driver to describe what is on the I2C buses. That seems to work just fine. However, I would like to use the ACPI I2cSerialBus() macro to specify the devices on those buses and have Linux automatically create the devices. I am defining the device as shown below (in scope \_SB). That creates an ACPI device under /sys/bus/acpi/devices/. // M24C02 EEPROM on I2C-3 addr 0x57 Device (EEP0) { Name (_ADR, 1) Name (_CID, Package() { "24c02" }) Method (_CRS, 0, NotSerialized) { Name (RBUF, ResourceTemplate () { I2cSerialBus (0x0057, ControllerInitiated, 400000, AddressingMode7Bit, "\\_SB.I2C3", 0x00, ResourceConsumer,,) }) Return (RBUF) } } The problem is that the I2C controller is a PCI device. The logic that scans for attached devices in acpi_i2c_register_devices() in i2c-core.c gives up because the adapter is not an ACPI device. handle = ACPI_HANDLE(adap->dev.parent); if (!handle) return; I am using coreboot/SeaBIOS as the BIOS, so I have full control over the ACPI tables. I tried enabling the I2C ACPI devices and using the i2c-designware-platform driver, but that didn't work because Linux changes the resource allocation for the PCI I2C devices if I specify the resources for the I2C device in ACPI. The relevent kernel logs are: [ 0.105628] pci 0000:00:18.3: [8086:0f43] type 00 class 0x0c8000 [ 0.105656] pci 0000:00:18.3: reg 0x10: [mem 0xd0723000-0xd0723fff] [ 0.105670] pci 0000:00:18.3: reg 0x14: [mem 0xd0724000-0xd0724fff] ... [ 0.122970] pci 0000:00:18.3: can't claim BAR 0 [mem 0xd0723000-0xd0723fff]: address conflict with 80860F43:00 [mem 0xd0723000-0xd0723fff] ... [ 0.136530] pci 0000:00:18.3: BAR 0: assigned [mem 0x80602000-0x80602fff] The i2c-designware-platform driver tries to use the old BAR0 address (0xd0723000), which obviously doesn't work. So my questions are: Can I use I2cSerialBus with a PCI I2C controller? If so, what am I doing incorrectly? For reference, here are the interesting parts of the ACPI device definition. The variable \S3B0 holds the BAR0 value that coreboot sets up. 8086:0f43 is the PCI device ID. 0:18.3 is the PCI bus, device, and function. Device (I2C3) { Name (_HID, "80860F43") Name (_UID, 3) Name (_DDN, "I2C Controller #3") Name (_ADR, 0x00180003) /* Standard Mode: HCNT, LCNT, SDA Hold Time */ Name (SSCN, Package () { 0x200, 0x200, 0x6 }) /* Fast Mode: HCNT, LCNT, SDA Hold Time */ Name (FMCN, Package () { 0x55, 0x99, 0x6 }) Name (RBUF, ResourceTemplate() { /* BAR0._BAS is replaced in _CRS */ Memory32Fixed (ReadWrite, 0, 0x1000, BAR0) Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive,,,) { LPSS_I2C3_IRQ } FixedDMA (0x10, 0x0, Width32Bit, ) FixedDMA (0x11, 0x1, Width32Bit, ) }) Method (_CRS) { CreateDwordField (^RBUF, ^BAR0._BAS, RBAS) Store (\S3B0, RBAS) Return (^RBUF) } ... } Thanks, Ben Gardner ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-20 19:47 ACPI: Can I use I2cSerialBus with a PCI I2C controller? Ben Gardner @ 2015-10-21 8:50 ` Mika Westerberg 2015-10-21 23:14 ` Ben Gardner 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-10-21 8:50 UTC (permalink / raw) To: Ben Gardner; +Cc: linux-acpi On Tue, Oct 20, 2015 at 02:47:43PM -0500, Ben Gardner wrote: > Hi everyone, > > I am building a custom board based on the Bayley Bay reference design, using > the Intel E3845 CPU. > I have a few I2C devices on the board that are connected to I2C buses 1-3. > The I2C controllers use the i2c-designware-pci driver. > > I am currently using i2c_register_board_info() in a driver to describe what > is on the I2C buses. That seems to work just fine. > > However, I would like to use the ACPI I2cSerialBus() macro to specify the > devices on those buses and have Linux automatically create the devices. > > I am defining the device as shown below (in scope \_SB). > That creates an ACPI device under /sys/bus/acpi/devices/. > > // M24C02 EEPROM on I2C-3 addr 0x57 > Device (EEP0) { > Name (_ADR, 1) > Name (_CID, Package() { "24c02" }) > Method (_CRS, 0, NotSerialized) { > Name (RBUF, ResourceTemplate () { > I2cSerialBus (0x0057, ControllerInitiated, > 400000, AddressingMode7Bit, "\\_SB.I2C3", > 0x00, ResourceConsumer,,) > }) > Return (RBUF) > } > } > > The problem is that the I2C controller is a PCI device. The logic that scans > for attached devices in acpi_i2c_register_devices() in i2c-core.c gives up > because the adapter is not an ACPI device. > > handle = ACPI_HANDLE(adap->dev.parent); > if (!handle) > return; > > I am using coreboot/SeaBIOS as the BIOS, so I have full control over > the ACPI tables. > > I tried enabling the I2C ACPI devices and using the i2c-designware-platform > driver, but that didn't work because Linux changes the resource allocation > for the PCI I2C devices if I specify the resources for the I2C device in ACPI. > > The relevent kernel logs are: > [ 0.105628] pci 0000:00:18.3: [8086:0f43] type 00 class 0x0c8000 > [ 0.105656] pci 0000:00:18.3: reg 0x10: [mem 0xd0723000-0xd0723fff] > [ 0.105670] pci 0000:00:18.3: reg 0x14: [mem 0xd0724000-0xd0724fff] > ... > [ 0.122970] pci 0000:00:18.3: can't claim BAR 0 [mem > 0xd0723000-0xd0723fff]: address conflict with 80860F43:00 [mem > 0xd0723000-0xd0723fff] > ... > [ 0.136530] pci 0000:00:18.3: BAR 0: assigned [mem 0x80602000-0x80602fff] > > The i2c-designware-platform driver tries to use the old BAR0 address > (0xd0723000), which obviously doesn't work. > > So my questions are: > Can I use I2cSerialBus with a PCI I2C controller? Yes you can. That's what we do all the time for Intel hardware. > If so, what am I doing incorrectly? Let me see.. > For reference, here are the interesting parts of the ACPI device definition. > The variable \S3B0 holds the BAR0 value that coreboot sets up. > 8086:0f43 is the PCI device ID. 0:18.3 is the PCI bus, device, and function. > > Device (I2C3) > { > Name (_HID, "80860F43") > Name (_UID, 3) Drop the _HID and _UID. > Name (_DDN, "I2C Controller #3") > Name (_ADR, 0x00180003) This will bind the PCI device to this ACPI device if the address is correct. > /* Standard Mode: HCNT, LCNT, SDA Hold Time */ > Name (SSCN, Package () { 0x200, 0x200, 0x6 }) > > /* Fast Mode: HCNT, LCNT, SDA Hold Time */ > Name (FMCN, Package () { 0x55, 0x99, 0x6 }) > > Name (RBUF, ResourceTemplate() > { > /* BAR0._BAS is replaced in _CRS */ > Memory32Fixed (ReadWrite, 0, 0x1000, BAR0) > Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive,,,) > { > LPSS_I2C3_IRQ > } You should not populate BAR and IRQ. > FixedDMA (0x10, 0x0, Width32Bit, ) > FixedDMA (0x11, 0x1, Width32Bit, ) These you don't need as the designware I2C does not currently support DMA. So you can just drop the whole _CRS and make your I2C host controller device to look like: Device (I2C3) { Name (_ADR, 0x00180003) /* Standard Mode: HCNT, LCNT, SDA Hold Time */ Name (SSCN, Package () { 0x200, 0x200, 0x6 }) /* Fast Mode: HCNT, LCNT, SDA Hold Time */ Name (FMCN, Package () { 0x55, 0x99, 0x6 }) Device (EEP0) { Name (_CID, Package() { "24c02" }) Name (_CRS, ResourceTemplate () { I2cSerialBus (0x0057, ControllerInitiated, 400000, AddressingMode7Bit, "\\_SB.I2C3", 0x00, ResourceConsumer,,) }) } } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-21 8:50 ` Mika Westerberg @ 2015-10-21 23:14 ` Ben Gardner 2015-10-22 8:01 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-21 23:14 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi Thanks for the help! >> So my questions are: >> Can I use I2cSerialBus with a PCI I2C controller? > > Yes you can. > > That's what we do all the time for Intel hardware. That is good to hear. ... > So you can just drop the whole _CRS and make your I2C host controller > device to look like: > > Device (I2C3) > { > Name (_ADR, 0x00180003) > > /* Standard Mode: HCNT, LCNT, SDA Hold Time */ > Name (SSCN, Package () { 0x200, 0x200, 0x6 }) > > /* Fast Mode: HCNT, LCNT, SDA Hold Time */ > Name (FMCN, Package () { 0x55, 0x99, 0x6 }) > > Device (EEP0) > { > Name (_CID, Package() { "24c02" }) > Name (_CRS, ResourceTemplate () { > I2cSerialBus (0x0057, ControllerInitiated, 400000, > AddressingMode7Bit, "\\_SB.I2C3", 0x00, > ResourceConsumer,,) > }) > } > } OK. I tried that. It looks like the device is nested properly in the ACPI tree: I see this symlink: /sys/bus/acpi/devices/24C02:00 -> ../../../devices/LNXSYSTM:00/LNXSYBUS:00/device:16/24C02:00 # for x in /sys/bus/acpi/devices/* ; do if [ -e $x/path ] ; then printf "%-30.30s %s\n" "$(cat $x/path)" "$x" ; fi ; done | sort | grep I2C3 \_SB_.I2C3 device:16 \_SB_.I2C3.EEP0 24C02:00 But no I2C device are created under /sys/bus/i2c/devices/. # ll /sys/bus/i2c/devices/ lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-1 -> ../../../devices/pci0000:00/0000:00:18.1/i2c-1 lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-2 -> ../../../devices/pci0000:00/0000:00:18.2/i2c-2 lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-3 -> ../../../devices/pci0000:00/0000:00:18.3/i2c-3 I'll have to dig further to find where it is failing in i2c_core.c. I assume it will be in the same spot. Is there a way to see PCI to ACPI associations? BTW - I am using Linux kernel version 4.2.3. Thanks, Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-21 23:14 ` Ben Gardner @ 2015-10-22 8:01 ` Mika Westerberg 2015-10-22 16:19 ` Ben Gardner 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-10-22 8:01 UTC (permalink / raw) To: Ben Gardner; +Cc: linux-acpi On Wed, Oct 21, 2015 at 06:14:27PM -0500, Ben Gardner wrote: > Thanks for the help! > > >> So my questions are: > >> Can I use I2cSerialBus with a PCI I2C controller? > > > > Yes you can. > > > > That's what we do all the time for Intel hardware. > > That is good to hear. > ... > > > So you can just drop the whole _CRS and make your I2C host controller > > device to look like: > > > > Device (I2C3) > > { > > Name (_ADR, 0x00180003) > > > > /* Standard Mode: HCNT, LCNT, SDA Hold Time */ > > Name (SSCN, Package () { 0x200, 0x200, 0x6 }) > > > > /* Fast Mode: HCNT, LCNT, SDA Hold Time */ > > Name (FMCN, Package () { 0x55, 0x99, 0x6 }) > > > > Device (EEP0) > > { > > Name (_CID, Package() { "24c02" }) > > Name (_CRS, ResourceTemplate () { > > I2cSerialBus (0x0057, ControllerInitiated, 400000, > > AddressingMode7Bit, "\\_SB.I2C3", 0x00, > > ResourceConsumer,,) > > }) > > } > > } > > OK. I tried that. > It looks like the device is nested properly in the ACPI tree: > > I see this symlink: > /sys/bus/acpi/devices/24C02:00 -> > ../../../devices/LNXSYSTM:00/LNXSYBUS:00/device:16/24C02:00 > > # for x in /sys/bus/acpi/devices/* ; do if [ -e $x/path ] ; then > printf "%-30.30s %s\n" "$(cat $x/path)" "$x" ; fi ; done | sort | > grep I2C3 > \_SB_.I2C3 device:16 > \_SB_.I2C3.EEP0 24C02:00 > > But no I2C device are created under /sys/bus/i2c/devices/. > # ll /sys/bus/i2c/devices/ > lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-1 -> > ../../../devices/pci0000:00/0000:00:18.1/i2c-1 > lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-2 -> > ../../../devices/pci0000:00/0000:00:18.2/i2c-2 > lrwxrwxrwx 1 root root 0 Oct 21 11:30 i2c-3 -> > ../../../devices/pci0000:00/0000:00:18.3/i2c-3 > > I'll have to dig further to find where it is failing in i2c_core.c. I > assume it will be in the same spot. > Is there a way to see PCI to ACPI associations? Yes, the PCI device has a symlink "firmware_node" which points to the ACPI device. Here an example from Skylake system (which uses PCI mode for LPSS devices): # ls -l /sys/bus/pci/devices/0000\:00\:15.1/firmware_node lrwxrwxrwx 1 root root 0 Oct 22 08:00 /sys/bus/pci/devices/0000:00:15.1/firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:6f # cat /sys/bus/pci/devices/0000\:00\:15.1/firmware_node/path \_SB_.PCI0.I2C1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-22 8:01 ` Mika Westerberg @ 2015-10-22 16:19 ` Ben Gardner 2015-10-22 17:17 ` Ben Gardner 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-22 16:19 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi >> Is there a way to see PCI to ACPI associations? > > Yes, the PCI device has a symlink "firmware_node" which points to the > ACPI device. > > Here an example from Skylake system (which uses PCI mode for LPSS > devices): > > # ls -l /sys/bus/pci/devices/0000\:00\:15.1/firmware_node > lrwxrwxrwx 1 root root 0 Oct 22 08:00 /sys/bus/pci/devices/0000:00:15.1/firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:6f > > # cat /sys/bus/pci/devices/0000\:00\:15.1/firmware_node/path > \_SB_.PCI0.I2C1 I see the problem now. Your I2C controller is under the PCI0 device on your Skylake system, mine was not. So, I moved the definition of all the PCI devices that are on PCI bus 0 to under PCI0. After that, all the PCI devices found their ACPI companion and the firmware_node symlink is present and correct. I2C-core creates a symlink for the attached device on the right controller. /sys/bus/i2c/devices/i2c-24C02:00 -> ../../../devices/pci0000:00/0000:00:18.3/i2c-3/i2c-24C02:00 So that part is solved. (And I'll have to send a patch to coreboot...) The next issue is that the I2C-core isn't matching the device to the "at24" driver, which has the alias "24c02". I used to instantiate the device from user space with: # echo 24c02 0x57 > /sys/bus/i2c/devices/i2c-3/new_device That now shows the address is busy, so it is creating the device, just not linking it to the driver. I'll dig further and see what I can see. Thanks for your help. Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-22 16:19 ` Ben Gardner @ 2015-10-22 17:17 ` Ben Gardner 2015-10-23 8:20 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-22 17:17 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi > The next issue is that the I2C-core isn't matching the device to the > "at24" driver, which has the alias "24c02". Here is what I found. i2c-core is creating the device with the ACPI name "24C02:00". The at24 driver uses "24c02" as the alias. i2c-code is matching devices to drivers using strcmp(). Result: no match. ("24c02" != "24C02:00"). If I modify acpi_i2c_add_device() to cut off the name at the ':' and covert to lowercase when populating info.type, it matches and works. I must be missing something here, because this would have never worked as-is. I'll ask on the I2C mailing list. Thanks again, Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-22 17:17 ` Ben Gardner @ 2015-10-23 8:20 ` Mika Westerberg 2015-10-23 9:43 ` Mika Westerberg ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Mika Westerberg @ 2015-10-23 8:20 UTC (permalink / raw) To: Ben Gardner; +Cc: linux-acpi [-- Attachment #1: Type: text/plain, Size: 1802 bytes --] On Thu, Oct 22, 2015 at 12:17:58PM -0500, Ben Gardner wrote: > > The next issue is that the I2C-core isn't matching the device to the > > "at24" driver, which has the alias "24c02". > > Here is what I found. > i2c-core is creating the device with the ACPI name "24C02:00". > The at24 driver uses "24c02" as the alias. > i2c-code is matching devices to drivers using strcmp(). > > Result: no match. ("24c02" != "24C02:00"). > > If I modify acpi_i2c_add_device() to cut off the name at the ':' and > covert to lowercase when populating info.type, it matches and works. > I must be missing something here, because this would have never worked as-is. > I'll ask on the I2C mailing list. You should either use proper _HID/_CID for the device or put "PRP0001" to the _HID and let the match happen with DT .compatible strings. I've attached a hack that I use locally. The corresponding ASL fragment would look like: Device (AT24) { Name (_HID, "PRP0001") Method (_CRS, 0, Serialized) { Name (UBUF, ResourceTemplate () { I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\\_SB.I2C6", 0x00, ResourceConsumer) }) Return (UBUF) } Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { Package () {"compatible", "atmel,24c02"}, Package () {"size", 256}, Package () {"pagesize", 32}, Package () {"abs-value", 1}, }, }) Method (_STA, 0, NotSerialized) { Return (0xF) } } [-- Attachment #2: 0001-misc-at24-Make-use-of-device-property-API.patch --] [-- Type: text/plain, Size: 3164 bytes --] >From 78792d4e759f023975700222caffa7a20f77fcf9 Mon Sep 17 00:00:00 2001 From: Mika Westerberg <mika.westerberg@linux.intel.com> Date: Mon, 29 Sep 2014 13:11:07 +0300 Subject: [PATCH] misc/at24: Make use of device property API Not-Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/misc/eeprom/at24.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index c6cb7f8f325e..d2963b5632d4 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -20,9 +20,9 @@ #include <linux/log2.h> #include <linux/bitops.h> #include <linux/jiffies.h> -#include <linux/of.h> #include <linux/i2c.h> #include <linux/platform_data/at24.h> +#include <linux/property.h> /* * I2C EEPROMs from most vendors are inexpensive and mostly interchangeable. @@ -443,26 +443,24 @@ static ssize_t at24_macc_write(struct memory_accessor *macc, const char *buf, /*-------------------------------------------------------------------------*/ -#ifdef CONFIG_OF -static void at24_get_ofdata(struct i2c_client *client, +static int at24_get_fwdata(struct i2c_client *client, struct at24_platform_data *chip) { - const __be32 *val; - struct device_node *node = client->dev.of_node; - - if (node) { - if (of_get_property(node, "read-only", NULL)) - chip->flags |= AT24_FLAG_READONLY; - val = of_get_property(node, "pagesize", NULL); - if (val) - chip->page_size = be32_to_cpup(val); - } + if (device_property_present(&client->dev, "read-only")) + chip->flags |= AT24_FLAG_READONLY; + if (device_property_read_u16(&client->dev, "pagesize", &chip->page_size)) + return -ENODEV; + if (device_property_read_u32(&client->dev, "size", &chip->byte_len)) + return -ENODEV; + + return 0; } -#else -static void at24_get_ofdata(struct i2c_client *client, - struct at24_platform_data *chip) -{ } -#endif /* CONFIG_OF */ + +static const struct of_device_id at24_of_match[] = { + { .compatible = "atmel,24c02" }, + { }, +}; +MODULE_DEVICE_TABLE(of, at24_of_match); static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -477,7 +475,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) if (client->dev.platform_data) { chip = *(struct at24_platform_data *)client->dev.platform_data; - } else { + } else if (id) { if (!id->driver_data) return -ENODEV; @@ -493,10 +491,15 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) chip.page_size = 1; /* update chipdata if OF is present */ - at24_get_ofdata(client, &chip); + at24_get_fwdata(client, &chip); chip.setup = NULL; chip.context = NULL; + } else { + memset(&chip, 0, sizeof(chip)); + err = at24_get_fwdata(client, &chip); + if (err) + return err; } if (!is_power_of_2(chip.byte_len)) @@ -661,6 +664,7 @@ static int at24_remove(struct i2c_client *client) static struct i2c_driver at24_driver = { .driver = { .name = "at24", + .of_match_table = at24_of_match, }, .probe = at24_probe, .remove = at24_remove, -- 2.6.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-23 8:20 ` Mika Westerberg @ 2015-10-23 9:43 ` Mika Westerberg 2015-10-23 17:24 ` Ben Gardner 2015-10-27 21:11 ` Dustin Byford 2 siblings, 0 replies; 15+ messages in thread From: Mika Westerberg @ 2015-10-23 9:43 UTC (permalink / raw) To: Ben Gardner; +Cc: linux-acpi On Fri, Oct 23, 2015 at 11:20:54AM +0300, Mika Westerberg wrote: > On Thu, Oct 22, 2015 at 12:17:58PM -0500, Ben Gardner wrote: > > > The next issue is that the I2C-core isn't matching the device to the > > > "at24" driver, which has the alias "24c02". > > > > Here is what I found. > > i2c-core is creating the device with the ACPI name "24C02:00". > > The at24 driver uses "24c02" as the alias. > > i2c-code is matching devices to drivers using strcmp(). > > > > Result: no match. ("24c02" != "24C02:00"). > > > > If I modify acpi_i2c_add_device() to cut off the name at the ':' and > > covert to lowercase when populating info.type, it matches and works. > > I must be missing something here, because this would have never worked as-is. > > I'll ask on the I2C mailing list. > > You should either use proper _HID/_CID for the device or put "PRP0001" > to the _HID and let the match happen with DT .compatible strings. I've > attached a hack that I use locally. Alternatively you can use patch by Andy here: https://patchwork.ozlabs.org/patch/524923 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-23 8:20 ` Mika Westerberg 2015-10-23 9:43 ` Mika Westerberg @ 2015-10-23 17:24 ` Ben Gardner 2015-10-26 19:56 ` Ben Gardner 2015-10-27 21:11 ` Dustin Byford 2 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-23 17:24 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi On Fri, Oct 23, 2015 at 3:20 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Thu, Oct 22, 2015 at 12:17:58PM -0500, Ben Gardner wrote: >> > The next issue is that the I2C-core isn't matching the device to the >> > "at24" driver, which has the alias "24c02". >> >> Here is what I found. >> i2c-core is creating the device with the ACPI name "24C02:00". >> The at24 driver uses "24c02" as the alias. >> i2c-code is matching devices to drivers using strcmp(). >> >> Result: no match. ("24c02" != "24C02:00"). >> >> If I modify acpi_i2c_add_device() to cut off the name at the ':' and >> covert to lowercase when populating info.type, it matches and works. >> I must be missing something here, because this would have never worked as-is. >> I'll ask on the I2C mailing list. > > You should either use proper _HID/_CID for the device or put "PRP0001" > to the _HID and let the match happen with DT .compatible strings. I've > attached a hack that I use locally. > > The corresponding ASL fragment would look like: > > > Device (AT24) > { > Name (_HID, "PRP0001") > > Method (_CRS, 0, Serialized) { > Name (UBUF, ResourceTemplate () { > I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.I2C6", > 0x00, ResourceConsumer) > }) > Return (UBUF) > } > > Name (_DSD, Package () { > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > Package () { > Package () {"compatible", "atmel,24c02"}, > Package () {"size", 256}, > Package () {"pagesize", 32}, > Package () {"abs-value", 1}, > }, > }) > > Method (_STA, 0, NotSerialized) > { > Return (0xF) > } > } I like this approach, as one change to the driver will support all ACPI devices. I'll give it a try. Thanks, Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-23 17:24 ` Ben Gardner @ 2015-10-26 19:56 ` Ben Gardner 2015-10-27 10:49 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-26 19:56 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi On Fri, Oct 23, 2015 at 12:24 PM, Ben Gardner <gardner.ben@gmail.com> wrote: > On Fri, Oct 23, 2015 at 3:20 AM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: >> On Thu, Oct 22, 2015 at 12:17:58PM -0500, Ben Gardner wrote: >>> > The next issue is that the I2C-core isn't matching the device to the >>> > "at24" driver, which has the alias "24c02". >>> >>> Here is what I found. >>> i2c-core is creating the device with the ACPI name "24C02:00". >>> The at24 driver uses "24c02" as the alias. >>> i2c-code is matching devices to drivers using strcmp(). >>> >>> Result: no match. ("24c02" != "24C02:00"). >>> >>> If I modify acpi_i2c_add_device() to cut off the name at the ':' and >>> covert to lowercase when populating info.type, it matches and works. >>> I must be missing something here, because this would have never worked as-is. >>> I'll ask on the I2C mailing list. >> >> You should either use proper _HID/_CID for the device or put "PRP0001" >> to the _HID and let the match happen with DT .compatible strings. I've >> attached a hack that I use locally. >> >> The corresponding ASL fragment would look like: >> >> >> Device (AT24) >> { >> Name (_HID, "PRP0001") >> >> Method (_CRS, 0, Serialized) { >> Name (UBUF, ResourceTemplate () { >> I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, >> AddressingMode7Bit, "\\_SB.I2C6", >> 0x00, ResourceConsumer) >> }) >> Return (UBUF) >> } >> >> Name (_DSD, Package () { >> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), >> Package () { >> Package () {"compatible", "atmel,24c02"}, >> Package () {"size", 256}, >> Package () {"pagesize", 32}, >> Package () {"abs-value", 1}, >> }, >> }) >> >> Method (_STA, 0, NotSerialized) >> { >> Return (0xF) >> } >> } > > I like this approach, as one change to the driver will support all ACPI devices. > I'll give it a try. This approach works well enough. Would you mind if I cleaned up the patch and submitted it for inclusion? Or is there a reason why this isn't upstream? Thanks, Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-26 19:56 ` Ben Gardner @ 2015-10-27 10:49 ` Mika Westerberg 0 siblings, 0 replies; 15+ messages in thread From: Mika Westerberg @ 2015-10-27 10:49 UTC (permalink / raw) To: Ben Gardner; +Cc: linux-acpi On Mon, Oct 26, 2015 at 02:56:09PM -0500, Ben Gardner wrote: > This approach works well enough. Great. > Would you mind if I cleaned up the patch and submitted it for inclusion? Not at all. > Or is there a reason why this isn't upstream? I just haven't had time to clean it up ;-) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-23 8:20 ` Mika Westerberg 2015-10-23 9:43 ` Mika Westerberg 2015-10-23 17:24 ` Ben Gardner @ 2015-10-27 21:11 ` Dustin Byford 2015-10-28 9:01 ` Mika Westerberg 2 siblings, 1 reply; 15+ messages in thread From: Dustin Byford @ 2015-10-27 21:11 UTC (permalink / raw) To: Mika Westerberg; +Cc: Ben Gardner, linux-acpi Hi Mika, On Fri Oct 23 11:20, Mika Westerberg wrote: > You should either use proper _HID/_CID for the device or put "PRP0001" > to the _HID and let the match happen with DT .compatible strings. I've > attached a hack that I use locally. I have a similar hack over here. I have a question though: > The corresponding ASL fragment would look like: > > > Device (AT24) > { > Name (_HID, "PRP0001") > > Method (_CRS, 0, Serialized) { > Name (UBUF, ResourceTemplate () { > I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.I2C6", > 0x00, ResourceConsumer) > }) > Return (UBUF) > } > > Name (_DSD, Package () { > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > Package () { > Package () {"compatible", "atmel,24c02"}, The "c02" in 24c02 also indicates the size. I've always found it a little awkward when you could have a compatible string that disagrees with firmware properties. Should we do something about that? For example, is the more generic string, "atmel,at24" better? I'm not sure I like that approach in general, but it works well for the at24 devices. at25 does it the same way. > Package () {"size", 256}, > Package () {"pagesize", 32}, > Package () {"abs-value", 1}, > }, > }) > > Method (_STA, 0, NotSerialized) > { > Return (0xF) > } > } --Dustin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-27 21:11 ` Dustin Byford @ 2015-10-28 9:01 ` Mika Westerberg 2015-10-30 16:51 ` Ben Gardner 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-10-28 9:01 UTC (permalink / raw) To: Dustin Byford; +Cc: Ben Gardner, linux-acpi On Tue, Oct 27, 2015 at 02:11:11PM -0700, Dustin Byford wrote: > Hi Mika, > > On Fri Oct 23 11:20, Mika Westerberg wrote: > > > You should either use proper _HID/_CID for the device or put "PRP0001" > > to the _HID and let the match happen with DT .compatible strings. I've > > attached a hack that I use locally. > > I have a similar hack over here. I have a question though: > > > The corresponding ASL fragment would look like: > > > > > > Device (AT24) > > { > > Name (_HID, "PRP0001") > > > > Method (_CRS, 0, Serialized) { > > Name (UBUF, ResourceTemplate () { > > I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, > > AddressingMode7Bit, "\\_SB.I2C6", > > 0x00, ResourceConsumer) > > }) > > Return (UBUF) > > } > > > > Name (_DSD, Package () { > > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > > Package () { > > Package () {"compatible", "atmel,24c02"}, > > The "c02" in 24c02 also indicates the size. I've always found it a > little awkward when you could have a compatible string that disagrees > with firmware properties. Should we do something about that? > > For example, is the more generic string, "atmel,at24" better? I'm not > sure I like that approach in general, but it works well for the at24 > devices. at25 does it the same way. I think DT version actually identifies this if the device node is called "at24". I'm not sure if we can do that with ACPI though. I agree that we could use "atmel,at24" here but that requires corresponding binding to be added to Documentation/devicetree/bindings/i2c/trivial-devices.txt and review from DT folks. Definitely worth doing. Alternatively we may use the same _HID "INT3499" as Andy is doing in his series with the below properties. > > Package () {"size", 256}, > > Package () {"pagesize", 32}, > > Package () {"abs-value", 1}, > > }, > > }) > > > > Method (_STA, 0, NotSerialized) > > { > > Return (0xF) > > } > > } > > --Dustin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-28 9:01 ` Mika Westerberg @ 2015-10-30 16:51 ` Ben Gardner 2015-11-02 10:25 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Ben Gardner @ 2015-10-30 16:51 UTC (permalink / raw) To: Mika Westerberg; +Cc: Dustin Byford, linux-acpi Hi, On Wed, Oct 28, 2015 at 4:01 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Tue, Oct 27, 2015 at 02:11:11PM -0700, Dustin Byford wrote: >> Hi Mika, >> >> On Fri Oct 23 11:20, Mika Westerberg wrote: >> >> > You should either use proper _HID/_CID for the device or put "PRP0001" >> > to the _HID and let the match happen with DT .compatible strings. I've >> > attached a hack that I use locally. >> >> I have a similar hack over here. I have a question though: >> >> > The corresponding ASL fragment would look like: >> > >> > >> > Device (AT24) >> > { >> > Name (_HID, "PRP0001") >> > >> > Method (_CRS, 0, Serialized) { >> > Name (UBUF, ResourceTemplate () { >> > I2cSerialBus (0x50, ControllerInitiated, 0x00061A80, >> > AddressingMode7Bit, "\\_SB.I2C6", >> > 0x00, ResourceConsumer) >> > }) >> > Return (UBUF) >> > } >> > >> > Name (_DSD, Package () { >> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), >> > Package () { >> > Package () {"compatible", "atmel,24c02"}, >> >> The "c02" in 24c02 also indicates the size. I've always found it a >> little awkward when you could have a compatible string that disagrees >> with firmware properties. Should we do something about that? >> >> For example, is the more generic string, "atmel,at24" better? I'm not >> sure I like that approach in general, but it works well for the at24 >> devices. at25 does it the same way. > > I think DT version actually identifies this if the device node is called > "at24". I'm not sure if we can do that with ACPI though. > > I agree that we could use "atmel,at24" here but that requires > corresponding binding to be added to > Documentation/devicetree/bindings/i2c/trivial-devices.txt and review > from DT folks. Definitely worth doing. It looks like you are supposed to have the first compatible entry be the exact manufacturer,model. http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property I am considering using a generic compatible entry ("linux,at24") to match the Linux at24 driver. In my case, I'm using a M24C02 from ST, so the compatible line would be: Package () {"compatible", Package () {"st,m24c02", "linux,at24"}}, Package () {"size", 256}, Package () {"pagesize", 16}, The "linux,at24" would require the other properties (size and pagesize) to be present. I've seen something similar done with a few other devices. > Alternatively we may use the same _HID "INT3499" as Andy is doing in his > series with the below properties. The only issue with reusing INT3499 is that it is configured to be a 24C08 in the at24 driver. Reusing the 24C08 HID for a 24C02 or other chip might be a bit confusing. Ben ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ACPI: Can I use I2cSerialBus with a PCI I2C controller? 2015-10-30 16:51 ` Ben Gardner @ 2015-11-02 10:25 ` Mika Westerberg 0 siblings, 0 replies; 15+ messages in thread From: Mika Westerberg @ 2015-11-02 10:25 UTC (permalink / raw) To: Ben Gardner; +Cc: Dustin Byford, linux-acpi On Fri, Oct 30, 2015 at 11:51:01AM -0500, Ben Gardner wrote: > It looks like you are supposed to have the first compatible entry be > the exact manufacturer,model. > http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property > > I am considering using a generic compatible entry ("linux,at24") to > match the Linux at24 driver. > In my case, I'm using a M24C02 from ST, so the compatible line would be: > > Package () {"compatible", Package () {"st,m24c02", "linux,at24"}}, > Package () {"size", 256}, > Package () {"pagesize", 16}, > > The "linux,at24" would require the other properties (size and > pagesize) to be present. > I've seen something similar done with a few other devices. Using "linux,foo" is not a good idea and I'm pretty sure DT maintainers will not take such patches. The compatible (and other properties as well) should describe hardware not software. > > Alternatively we may use the same _HID "INT3499" as Andy is doing in his > > series with the below properties. > > The only issue with reusing INT3499 is that it is configured to be a > 24C08 in the at24 driver. > Reusing the 24C08 HID for a 24C02 or other chip might be a bit confusing. It could default to 24c08 but if it has properties the driver should use those to determine size etc. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-11-02 10:25 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-20 19:47 ACPI: Can I use I2cSerialBus with a PCI I2C controller? Ben Gardner 2015-10-21 8:50 ` Mika Westerberg 2015-10-21 23:14 ` Ben Gardner 2015-10-22 8:01 ` Mika Westerberg 2015-10-22 16:19 ` Ben Gardner 2015-10-22 17:17 ` Ben Gardner 2015-10-23 8:20 ` Mika Westerberg 2015-10-23 9:43 ` Mika Westerberg 2015-10-23 17:24 ` Ben Gardner 2015-10-26 19:56 ` Ben Gardner 2015-10-27 10:49 ` Mika Westerberg 2015-10-27 21:11 ` Dustin Byford 2015-10-28 9:01 ` Mika Westerberg 2015-10-30 16:51 ` Ben Gardner 2015-11-02 10:25 ` Mika Westerberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.