[PATCH v2 0/3] soundwire: add DMI quirks for overridind addr

[PATCH v2 0/3] soundwire: add DMI quirks for overridind addr

[Bard Liao]

Platform firmware may have incorrect _ADR values causing the driver
probes to fail. Adding the override_ops allows people to override the
addr values.

v2:
 - Add the override_adr ops
 - Move DMI quirks to a new file

Pierre-Louis Bossart (2):
  soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible
  soundwire: Intel: add DMI quirk for Dell SKU 0A3E

Vinod Koul (1):
  soundwire: add override addr ops

 drivers/soundwire/Makefile     |  2 +-
 drivers/soundwire/bus.h        |  2 +
 drivers/soundwire/dmi-quirks.c | 96 ++++++++++++++++++++++++++++++++++
 drivers/soundwire/intel.c      |  1 +
 drivers/soundwire/slave.c      |  8 ++-
 include/linux/soundwire/sdw.h  |  4 +-
 6 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 drivers/soundwire/dmi-quirks.c

-- 
2.17.1

[PATCH v2 1/3] soundwire: add override addr ops

[Bard Liao]

From: Vinod Koul <vkoul@kernel.org>

Platform firmware may have incorrect _ADR values causing the driver
probes to fail. Add the override_ops, which when configured will allow
for quirks based on DMI etc to override the addr values.

Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 drivers/soundwire/slave.c     | 8 +++++++-
 include/linux/soundwire/sdw.h | 4 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 180f38bd003b..112b21967c7a 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -95,7 +95,7 @@ static bool find_slave(struct sdw_bus *bus,
 		       struct acpi_device *adev,
 		       struct sdw_slave_id *id)
 {
-	unsigned long long addr;
+	u64 addr;
 	unsigned int link_id;
 	acpi_status status;
 
@@ -108,6 +108,12 @@ static bool find_slave(struct sdw_bus *bus,
 		return false;
 	}
 
+	if (bus->ops->override_adr)
+		addr = bus->ops->override_adr(bus, addr);
+
+	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 d08039d65825..f0a3895e8faf 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -804,6 +804,7 @@ struct sdw_defer {
 /**
  * struct sdw_master_ops - Master driver ops
  * @read_prop: Read Master properties
+ * @override_adr: Override value read from firmware (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,7 +814,8 @@ struct sdw_defer {
  */
 struct sdw_master_ops {
 	int (*read_prop)(struct sdw_bus *bus);
-
+	u64 (*override_adr)
+			(struct sdw_bus *bus, u64 addr);
 	enum sdw_command_response (*xfer_msg)
 			(struct sdw_bus *bus, struct sdw_msg *msg);
 	enum sdw_command_response (*xfer_msg_defer)
-- 
2.17.1

[PATCH v2 2/3] soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

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.

BugLink: https://github.com/thesofproject/linux/issues/2700
Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 drivers/soundwire/Makefile     |  2 +-
 drivers/soundwire/bus.h        |  2 ++
 drivers/soundwire/dmi-quirks.c | 66 ++++++++++++++++++++++++++++++++++
 drivers/soundwire/intel.c      |  1 +
 4 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soundwire/dmi-quirks.c

diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
index bf1e250d50dd..986776787b9e 100644
--- a/drivers/soundwire/Makefile
+++ b/drivers/soundwire/Makefile
@@ -20,7 +20,7 @@ soundwire-cadence-y := cadence_master.o
 obj-$(CONFIG_SOUNDWIRE_CADENCE) += soundwire-cadence.o
 
 #Intel driver
-soundwire-intel-y :=	intel.o intel_init.o
+soundwire-intel-y :=	intel.o intel_init.o dmi-quirks.o
 obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel.o
 
 #Qualcomm driver
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 2e049d39c6e5..40354469860a 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -7,6 +7,8 @@
 #define DEFAULT_BANK_SWITCH_TIMEOUT 3000
 #define DEFAULT_PROBE_TIMEOUT       2000
 
+u64 sdw_dmi_override_adr(struct sdw_bus *bus, u64 addr);
+
 #if IS_ENABLED(CONFIG_ACPI)
 int sdw_acpi_find_slaves(struct sdw_bus *bus);
 #else
diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c
new file mode 100644
index 000000000000..249e476e49ea
--- /dev/null
+++ b/drivers/soundwire/dmi-quirks.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2021 Intel Corporation.
+
+/*
+ * Soundwire DMI quirks
+ */
+
+#include <linux/device.h>
+#include <linux/dmi.h>
+#include <linux/soundwire/sdw.h>
+#include "bus.h"
+
+struct adr_remap {
+	u64 adr;
+	u64 remapped_adr;
+};
+
+/*
+ * HP Spectre 360 Convertible devices do not expose the correct _ADR
+ * in the DSDT.
+ * Remap the bad _ADR values to the ones reported by hardware
+ */
+static const struct adr_remap hp_spectre_360[] = {
+	{
+		0x000010025D070100,
+		0x000020025D071100
+	},
+	{
+		0x000110025d070100,
+		0x000120025D130800
+	},
+	{}
+};
+
+static const struct dmi_system_id adr_remap_quirk_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x360 Convertible"),
+		},
+		.driver_data = (void *)hp_spectre_360,
+	},
+	{}
+};
+
+u64 sdw_dmi_override_adr(struct sdw_bus *bus, u64 addr)
+{
+	const struct dmi_system_id *dmi_id;
+
+	/* check if any address remap quirk applies */
+	dmi_id = dmi_first_match(adr_remap_quirk_table);
+	if (dmi_id) {
+		struct adr_remap *map = dmi_id->driver_data;
+
+		for (map = dmi_id->driver_data; map->adr; map++) {
+			if (map->adr == addr) {
+				dev_dbg(bus->dev, "remapped _ADR 0x%llx as 0x%llx\n",
+					addr, map->remapped_adr);
+				addr = map->remapped_adr;
+				break;
+			}
+		}
+	}
+
+	return addr;
+}
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index a2d5cdaa9998..a401e270a47f 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1302,6 +1302,7 @@ static int intel_prop_read(struct sdw_bus *bus)
 
 static struct sdw_master_ops sdw_intel_ops = {
 	.read_prop = sdw_master_read_prop,
+	.override_adr = sdw_dmi_override_adr,
 	.xfer_msg = cdns_xfer_msg,
 	.xfer_msg_defer = cdns_xfer_msg_defer,
 	.reset_page_addr = cdns_reset_page_addr,
-- 
2.17.1

Re: [PATCH v2 2/3] soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible

[Dave Hansen]

On 3/1/21 11:51 PM, Bard Liao wrote:
> +++ b/drivers/soundwire/dmi-quirks.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +// Copyright(c) 2021 Intel Corporation.

It looks like this is already in intel-next, so this may be moot.  But,
is there a specific reason this is dual licensed?  If so, can you please
include information about the license choice in the cover letter of any
future version?

If there is no specific reason for this contribution to be dual
licensed, please make it GPL-2.0 only.

Re: [PATCH v2 2/3] soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible

[Vinod Koul]

On 12-04-21, 14:37, Dave Hansen wrote:
> On 3/1/21 11:51 PM, Bard Liao wrote:
> > +++ b/drivers/soundwire/dmi-quirks.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > +// Copyright(c) 2021 Intel Corporation.
> 
> It looks like this is already in intel-next, so this may be moot.  But,
> is there a specific reason this is dual licensed?  If so, can you please
> include information about the license choice in the cover letter of any
> future version?

The soundwire module from Intel and core soundwire core was always dual
licensed, so it kind of followed that..

> If there is no specific reason for this contribution to be dual
> licensed, please make it GPL-2.0 only.

This module, I would say NO. Unless someone from Intel disagree..
Pierre/Bard..?

If all agree I dont see a reason why this cant be updated to GPL only.

Thanks
-- 
~Vinod

Re: [PATCH v2 2/3] soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible

[Pierre-Louis Bossart]

On 4/13/21 11:08 PM, Vinod Koul wrote:
> On 12-04-21, 14:37, Dave Hansen wrote:
>> On 3/1/21 11:51 PM, Bard Liao wrote:
>>> +++ b/drivers/soundwire/dmi-quirks.c
>>> @@ -0,0 +1,66 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
>>> +// Copyright(c) 2021 Intel Corporation.
>>
>> It looks like this is already in intel-next, so this may be moot.  But,
>> is there a specific reason this is dual licensed?  If so, can you please
>> include information about the license choice in the cover letter of any
>> future version?
> 
> The soundwire module from Intel and core soundwire core was always dual
> licensed, so it kind of followed that..
> 
>> If there is no specific reason for this contribution to be dual
>> licensed, please make it GPL-2.0 only.
> 
> This module, I would say NO. Unless someone from Intel disagree..
> Pierre/Bard..?
> 
> If all agree I dont see a reason why this cant be updated to GPL only.

The initial version of those quirks was contributed as a change to 
drivers/soundwire/slave.c, which is dual-licensed. the code was split to 
a different file and the dual-license followed.

I am personally favorable to keeping the code as is, the quirks are just 
referring to low-level hardware descriptors that are not aligned with 
DevID hardware registers in external SoundWire devices. If enumeration 
was handled at a lower level, e.g. in DSP firmware the same information 
would be quite useful.

That said, it's been agreed with Dave that moving forward all new 
contributions from Intel with a dual-license would include an explicit 
statement in the commit message as to why it was selected over plain 
vanilla GPL-2.0-only.

[PATCH v2 3/3] soundwire: Intel: add DMI quirk for Dell SKU 0A3E

[Bard Liao]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

We've been handling ACPI issues on early versions of the product with
a local ACPI initrd override but now that we have the possibility of a
kernel quirk let's get rid of the initrd override. This helps make
sure that the kernel will support all versions of the BIOS, with or
without updates.

Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 drivers/soundwire/dmi-quirks.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c
index 249e476e49ea..82061c1d9835 100644
--- a/drivers/soundwire/dmi-quirks.c
+++ b/drivers/soundwire/dmi-quirks.c
@@ -32,6 +32,29 @@ static const struct adr_remap hp_spectre_360[] = {
 	{}
 };
 
+/*
+ * The initial version of the Dell SKU 0A3E did not expose the devices
+ * on the correct links.
+ */
+static const struct adr_remap dell_sku_0A3E[] = {
+	/* rt715 on link0 */
+	{
+		0x00020025d071100,
+		0x00021025d071500
+	},
+	/* rt711 on link1 */
+	{
+		0x000120025d130800,
+		0x000120025d071100,
+	},
+	/* rt1308 on link2 */
+	{
+		0x000220025d071500,
+		0x000220025d130800
+	},
+	{}
+};
+
 static const struct dmi_system_id adr_remap_quirk_table[] = {
 	{
 		.matches = {
@@ -40,6 +63,13 @@ static const struct dmi_system_id adr_remap_quirk_table[] = {
 		},
 		.driver_data = (void *)hp_spectre_360,
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A3E")
+		},
+		.driver_data = (void *)dell_sku_0A3E,
+	},
 	{}
 };
 
-- 
2.17.1

Re: [PATCH v2 0/3] soundwire: add DMI quirks for overridind addr

[Vinod Koul]

On 02-03-21, 15:51, Bard Liao wrote:
> Platform firmware may have incorrect _ADR values causing the driver
> probes to fail. Adding the override_ops allows people to override the
> addr values.

Applied, thanks

-- 
~Vinod