When registering a serdev controller, ACPI needs to be checked for devices attached to it. Currently, all immediate children of the ACPI node of the controller are assumed to be UART client devices for this controller. Furthermore, these devices are not searched elsewhere. This is incorrect: Similar to SPI and I2C devices, the UART client device definition (via UARTSerialBusV2) can reside anywhere in the ACPI namespace as resource definition inside the _CRS method and points to the controller via its ResourceSource field. This field may either contain a fully qualified or relative path, indicating the controller device. To address this, we need to walk over the whole ACPI namespace, looking at each resource definition, and match the client device to the controller via this field. Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx> --- This patch is similar to the the implementations in drivers/spi/spi.c (see commit 4c3c59544f33e97cf8557f27e05a9904ead16363) and drivers/i2c/i2c-core-acpi.c. However, I think that there may be an issues with these two implementations: Both walk over the whole ACPI namespace, but only match the first SPI or I2C resource (respectively), so I think there may be problems when multiple SPI or I2C resources are defined under the same ACPI device node (as in second or third SPI/I2C resource definitions being ignored). Please note, however, that I am by no means qualified with regards to this, and this might be totally fine. Nevertheless I'd appreciate if anyone with more knowledge on the subject could have a look at it. This patch would avoid this problem (for UART) by simply walking all resource definitions via acpi_walk_resources. There is a further issue in the serdev ACPI implementation that this patch does not address: ACPI UART resource definitions contain things like the initial baud-rate, parity, flow-control, etc. As far as I know, these things can currently only be set once the device is opened. Furthermore, some option values, such as ParityTypeMark, are not (yet) supported. I'd be willing to try and implement setting the currently supported values based on ACPI for a future patch, if anyone can provide me with some pointers on how to do that. I have personally tested this patch on a Microsoft Surface Book 2, which like all newer MS Surface devices has a UART EC, and it has been in use (in some form or another) for a couple of months on other Surface devices via a patched kernel [1, 2, 3]. I can, however, not speak for any non-Microsoft devices or potential Apple ACPI quirks. [1]: https://github.com/jakeday/linux-surface/ [2]: https://github.com/qzed/linux-surface/ [3]: https://github.com/qzed/linux-surfacegen5-acpi/ drivers/tty/serdev/core.c | 64 ++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index a0ac16ee6575..1c8360deea77 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -582,18 +582,64 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl, return AE_OK; } -static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level, - void *data, void **return_value) +struct acpi_serdev_resource_context { + struct serdev_controller *controller; + struct acpi_device *device; +}; + +static acpi_status +acpi_serdev_add_device_from_resource(struct acpi_resource *resource, void *data) { - struct serdev_controller *ctrl = data; - struct acpi_device *adev; + struct acpi_serdev_resource_context *ctx + = (struct acpi_serdev_resource_context *)data; + struct acpi_resource_source *ctrl_name; + acpi_handle ctrl_handle; + + if (resource->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) + return AE_OK; - if (acpi_bus_get_device(handle, &adev)) + if (resource->data.common_serial_bus.type + != ACPI_RESOURCE_SERIAL_TYPE_UART) return AE_OK; - return acpi_serdev_register_device(ctrl, adev); + ctrl_name = &resource->data.common_serial_bus.resource_source; + if (ctrl_name->string_length == 0 || !ctrl_name->string_ptr) + return AE_OK; + + if (acpi_get_handle(ctx->device->handle, ctrl_name->string_ptr, + &ctrl_handle)) + return AE_OK; + + if (ctrl_handle == ACPI_HANDLE(ctx->controller->dev.parent)) + return acpi_serdev_register_device(ctx->controller, + ctx->device); + + return AE_OK; } +static acpi_status +acpi_serdev_add_devices_from_resources(acpi_handle handle, u32 level, + void *data, void **return_value) +{ + struct acpi_serdev_resource_context ctx; + acpi_status status; + + ctx.controller = (struct serdev_controller *)data; + status = acpi_bus_get_device(handle, &ctx.device); + if (status) + return AE_OK; // ignore device if not present + + status = acpi_walk_resources(handle, METHOD_NAME__CRS, + acpi_serdev_add_device_from_resource, + &ctx); + if (status == AE_NOT_FOUND) + return AE_OK; // ignore if _CRS is not found + else + return status; +} + +#define SERDEV_ACPI_ENUMERATE_MAX_DEPTH 32 + static int acpi_serdev_register_devices(struct serdev_controller *ctrl) { acpi_status status; @@ -603,8 +649,10 @@ static int acpi_serdev_register_devices(struct serdev_controller *ctrl) if (!handle) return -ENODEV; - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1, - acpi_serdev_add_device, NULL, ctrl, NULL); + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, + SERDEV_ACPI_ENUMERATE_MAX_DEPTH, + acpi_serdev_add_devices_from_resources, + NULL, ctrl, NULL); if (ACPI_FAILURE(status)) dev_dbg(&ctrl->dev, "failed to enumerate serdev slaves\n"); -- 2.23.0
