ACPI / scan: Fix regression related to X-Gene UARTs
PATCH 1/1 ACPI / scan: Fix regression related to X-Gene UARTs
Commit e361d1f85855 ("ACPI / scan: Fix enumeration for special UART devices") caused a regression with some X-Gene based platforms (Mustang and M400) with invalid DSDT. The DSDT makes it appear that the UART device is also a slave device attached to itself. With the above commit the UART won't be enumerated by ACPI scan (slave serial devices shouldn't be). So check for X-Gene UART device and skip slace device check on it.
--- drivers/acpi/scan.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/acpi/scan.c +8 -0
@@ -1551,6 +1551,14 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) fwnode_property_present(&device->fwnode, "baud")))return true; +/* +* Firmware on some arm64 X-Gene platforms will make the UART +* device appear as both a UART and a slave of that UART. Just +* bail out here for X-Gene UARTs. +*/ +if (!strcmp (acpi_device_hid (device),"APMC0D08" )) +return false; +INIT_LIST_HEAD (&resource_list); acpi_dev_get_resources(device, &resource_list, acpi_check_serial_bus_slave,
Signature · 1 line
2.14.3
Rafael J. Wysocki
maintainerQuoted 3 lines · Mark Salter
Commit
e361d1f85855("ACPI / scan: Fix enumeration for special UART devices") caused a regression with some X-Gene based platforms (Mustang and M400) with invalid DSDT.
I'm not convinced that making changes to the core ACPI device enumeration code in order to cover up for firmware bugs is the right approach.
Quoted 15 lines
The DSDT makes it appear that the UART
device is also a slave device attached to itself. With the above commit the UART won't be enumerated by ACPI scan (slave serial devices shouldn't be). So check for X-Gene UART device and skip slace device check on it.Signed-off-by Mark Salter <[email protected]>--- drivers/acpi/scan.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/acpi/scan.c +0
-0
drivers/acpi/scan.c static bool acpi_device_enumeration_by_parent(struct acpi_device *device) · line 1560
Quoted 10 lines
drivers/acpi/scan.c +7
-0@@ -1551,6 +1551,14 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) fwnode_property_present(&device->fwnode, "baud")))return true; +/* +* Firmware on some arm64 X-Gene platforms will make the UART +* device appear as both a UART and a slave of that UART. Just +* bail out here for X-Gene UARTs. +*/ +if (!strcmp (acpi_device_hid (device),"APMC0D08" )) +return false;
Is the device ID never to be used outside of the broken configurations?
Even if that's the plan, how are you going to guarantee that anyway?
Quoted 5 lines
drivers/acpi/scan.c +1
-1+INIT_LIST_HEAD (&resource_list); acpi_dev_get_resources(device, &resource_list, acpi_check_serial_bus_slave, --
Quoted 8 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Salter
Commit
e361d1f85855("ACPI / scan: Fix enumeration for special UART devices") caused a regression with some X-Gene based platforms (Mustang and M400) with invalid DSDT.I'm not convinced that making changes to the core ACPI device enumeration code in order to cover up for firmware bugs is the right approach.
It is unfortunate but the firmware bug predates the change which uncovered it, so previously working systems no longer work.
Quoted 29 lines
Quoted 16 lines
The DSDT makes it appear that the UART
device is also a slave device attached to itself. With the above commit the UART won't be enumerated by ACPI scan (slave serial devices shouldn't be). So check for X-Gene UART device and skip slace device check on it.Signed-off-by Mark Salter <[email protected]>--- drivers/acpi/scan.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/acpi/scan.c +0
-0drivers/acpi/scan.c static bool acpi_device_enumeration_by_parent(struct acpi_device *device) · line 1560
Quoted 10 lines
drivers/acpi/scan.c +7
-0@@ -1551,6 +1551,14 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) fwnode_property_present(&device->fwnode, "baud")))return true; +/* +* Firmware on some arm64 X-Gene platforms will make the UART +* device appear as both a UART and a slave of that UART. Just +* bail out here for X-Gene UARTs. +*/ +if (!strcmp (acpi_device_hid (device),"APMC0D08" )) +return false;Is the device ID never to be used outside of the broken configurations?
Quoted 1 line
Even if that's the plan, how are you going to guarantee that anyway?
The device ID will always be used for X-Gene UARTs. Whether the DSDT is broken or not wouldn't matter because the end result would be the same (the UART being treated as master rather than a serial bus slave). The broken firmware looks like this:
Device (URT0)
{
Name (_HID, "APMC0D08") // _HID: Hardware ID
...
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
Memory32Fixed (ReadWrite,
0x1C020000, // Address Base
0x00000100, // Address Length
)
UartSerialBusV2 (0x0001C200, DataBitsEight, StopBitsOne,
0x00, LittleEndian, ParityTypeNone, FlowControlNone,
0x0010, 0x0010, "URT0",
0x00, ResourceConsumer, , Exclusive,
)
Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
{
0x0000006D,
}
})
...
}So "URT0" has a UartSerialBusV2 resource which references itself as the bus master.
Quoted 6 lines
Quoted 6 lines
drivers/acpi/scan.c +1
-1+INIT_LIST_HEAD (&resource_list); acpi_dev_get_resources(device, &resource_list, acpi_check_serial_bus_slave, --