Hi Pierre,
On 04-02-21, 14:48, Pierre-Louis Bossart wrote:
HP Spectre x360 Convertible devices expose invalid _ADR fields in the DSDT, which prevents codec drivers from probing. A possible solution is to override the DSDT, but that's just too painful for users.
This patch suggests a simple DMI-based quirk to remap the existing invalid ADR information into valid ones.
While I agree with the approach, I do not like the implementation. The quirks in firmware should not reside in core code. Drivers are the right place, of course we need to add callbacks into driver for this.
So I did a quick hacking and added the below patch, I think you can add the quirks in Intel driver based on DMI for this.
-- >8 --
From 20af8100025637ea5e295877d28f3afb9dbd4814 Mon Sep 17 00:00:00 2001
From: Vinod Koul vkoul@kernel.org Date: Fri, 5 Feb 2021 12:38:21 +0530 Subject: [PATCH] soundwire: add override addr ops
Some firmware can have buggy _ADR values causing the scanning of devices to fail. Add the override_ops which if implemented by master driver would be invoked instead of reading _ADR. The drivers can implement quirks based on DMI etc to override the addr values.
Signed-off-by: Vinod Koul vkoul@kernel.org --- -compile tested only, pls validate
drivers/soundwire/slave.c | 25 +++++++++++++++++++++---- include/linux/soundwire/sdw.h | 3 +++ 2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index a08f4081c1c4..2b0d646c5c2f 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -91,12 +91,11 @@ int sdw_slave_add(struct sdw_bus *bus,
#if IS_ENABLED(CONFIG_ACPI)
-static bool find_slave(struct sdw_bus *bus, - struct acpi_device *adev, - struct sdw_slave_id *id) +static bool _find_slave(struct sdw_bus *bus, + struct acpi_device *adev, + struct sdw_slave_id *id) { unsigned long long addr; - unsigned int link_id; acpi_status status;
status = acpi_evaluate_integer(adev->handle, @@ -108,6 +107,24 @@ static bool find_slave(struct sdw_bus *bus, return false; }
+ return addr; +} + +static bool find_slave(struct sdw_bus *bus, + struct acpi_device *adev, + struct sdw_slave_id *id) +{ + unsigned long long addr; + unsigned int link_id; + + if (bus->ops->override_adr) + addr = bus->ops->override_adr(bus); + else + addr = _find_slave(bus, adev, id); + + if (!addr) + return false; + /* Extract link id from ADR, Bit 51 to 48 (included) */ link_id = SDW_DISCO_LINK_ID(addr);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index f0b01b728640..d0ad3404ca95 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -804,6 +804,8 @@ struct sdw_defer { /** * struct sdw_master_ops - Master driver ops * @read_prop: Read Master properties + * @override_adr: Override reading address from firmware and read from + * driver instead (quirk for buggy firmware) * @xfer_msg: Transfer message callback * @xfer_msg_defer: Defer version of transfer message callback * @reset_page_addr: Reset the SCP page address registers @@ -813,6 +815,7 @@ struct sdw_defer { */ struct sdw_master_ops { int (*read_prop)(struct sdw_bus *bus); + unsigned long long (*override_adr)(struct sdw_bus *bus);
enum sdw_command_response (*xfer_msg) (struct sdw_bus *bus, struct sdw_msg *msg);