Re: [PATCH 1/2] platform/x86: wmi: Support reading/writing 16 bit EC values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 06.03.24 um 11:19 schrieb Ilpo Järvinen:

On Mon, 4 Mar 2024, Armin Wolf wrote:

The ACPI EC address space handler currently only supports
reading/writing 8 bit values. Some firmware implementations however
want to access for example 16 bit values, which is prefectly legal
according to the ACPI spec.

Add support for reading/writing such values.

Tested on a Dell Inspiron 3505 and a Asus Prime B650-Plus.

Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
---
  drivers/platform/x86/wmi.c | 44 +++++++++++++++++++++++++++++---------
  1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 1920e115da89..900e0e52a5fa 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1153,6 +1153,32 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev)
  	return 0;
  }

+static int ec_read_multiple(u8 address, u8 *buffer, size_t bytes)
+{
+	int i, ret;
+
+	for (i = 0; i < bytes; i++) {
+		ret = ec_read(address + i, &buffer[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ec_write_multiple(u8 address, u8 *buffer, size_t bytes)
+{
+	int i, ret;
+
+	for (i = 0; i < bytes; i++) {
+		ret = ec_write(address + i, buffer[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
  /*
   * WMI can have EmbeddedControl access regions. In which case, we just want to
   * hand these off to the EC driver.
@@ -1162,27 +1188,25 @@ acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address,
  			  u32 bits, u64 *value,
  			  void *handler_context, void *region_context)
  {
-	int result = 0;
-	u8 temp = 0;
+	int bytes = bits / 8;
I'm a quite hesitant about this. IMO, it should do DIV_ROUND_UP(bits,
BITS_PER_BYTE) or return AE_BAD_PARAMETER when bits is not divisable by 8.
And if you choose the round up approach, I'm not sure what the write
should do with the excess bits.

In any case, 8 -> BITS_PER_BYTE.

After taking a look at acpi_ex_access_region(), which invokes the address space handler,
i think the number of bits are always divisible by 8.

I CCed the maintainers of the ACPI EC driver, so that we can clarify if this is indeed
always the case.

Thanks,
Armin Wolf

+	int ret;

-	if ((address > 0xFF) || !value)
+	if (address > 0xFF || !value)
This should takes bytes into account to not overflow the u8 address?






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux