diff mbox series

[v4,34/35] dm: acpi: Enhance acpi_get_name()

Message ID 20200707131135.v4.34.I1a77e678d7c6973796cc651ac6d858e1122bb7ae@changeid
State Superseded
Headers show
Series dm: Add programmatic generation of ACPI tables (part B) | expand

Commit Message

Simon Glass July 7, 2020, 7:12 p.m. UTC
For many device types it is possible to figure out the name just by
looking at its uclass or parent. Add a function to handle this, since it
allows us to cover the vast majority of cases automatically.

However it is sometimes impossible to figure out an ACPI name for a device
just by looking at its uclass. For example a touch device may have a
vendor-specific name. Add a new "acpi,name" property to allow a custom
name to be created.

With this new feature we can drop the get_name() methods in the sandbox
I2C and SPI drivers. They were only added for testing purposes. Update the
tests to use the new values.

Signed-off-by: Simon Glass <sjg at chromium.org>
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
---

Changes in v4:
- Mention that acpi_device_infer_name() only supports x86
- Add a comment about only supporting USB3.0

Changes in v3:
- Fix 'of' typo

Changes in v1:
- Use acpi,ddn instead of acpi,desc
- Rename to acpi_device_infer_name()
- Update newly created sandbox tests

 arch/sandbox/dts/test.dts           |   1 +
 doc/device-tree-bindings/device.txt |  13 ++++
 drivers/core/acpi.c                 |  13 +++-
 drivers/i2c/sandbox_i2c.c           |  10 ---
 drivers/spi/sandbox_spi.c           |  10 ---
 include/acpi/acpi_device.h          |  18 +++++
 lib/acpi/acpi_device.c              | 106 ++++++++++++++++++++++++++++
 test/dm/acpi.c                      |  42 ++++++++++-
 8 files changed, 190 insertions(+), 23 deletions(-)

Comments

Bin Meng July 13, 2020, 3:18 a.m. UTC | #1
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass <sjg at chromium.org> wrote:
>
> For many device types it is possible to figure out the name just by
> looking at its uclass or parent. Add a function to handle this, since it
> allows us to cover the vast majority of cases automatically.
>
> However it is sometimes impossible to figure out an ACPI name for a device
> just by looking at its uclass. For example a touch device may have a
> vendor-specific name. Add a new "acpi,name" property to allow a custom
> name to be created.
>
> With this new feature we can drop the get_name() methods in the sandbox
> I2C and SPI drivers. They were only added for testing purposes. Update the
> tests to use the new values.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> ---
>
> Changes in v4:
> - Mention that acpi_device_infer_name() only supports x86
> - Add a comment about only supporting USB3.0
>
> Changes in v3:
> - Fix 'of' typo
>
> Changes in v1:
> - Use acpi,ddn instead of acpi,desc
> - Rename to acpi_device_infer_name()
> - Update newly created sandbox tests
>
>  arch/sandbox/dts/test.dts           |   1 +
>  doc/device-tree-bindings/device.txt |  13 ++++
>  drivers/core/acpi.c                 |  13 +++-
>  drivers/i2c/sandbox_i2c.c           |  10 ---
>  drivers/spi/sandbox_spi.c           |  10 ---
>  include/acpi/acpi_device.h          |  18 +++++
>  lib/acpi/acpi_device.c              | 106 ++++++++++++++++++++++++++++
>  test/dm/acpi.c                      |  42 ++++++++++-
>  8 files changed, 190 insertions(+), 23 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
diff mbox series

Patch

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fc6a6b1774..382df1f1e6 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -113,6 +113,7 @@ 
 		int-array = <5678 9123 4567>;
 		str-value = "test string";
 		interrupts-extended = <&irq 3 0>;
+		acpi,name = "GHIJ";
 	};
 
 	junk {
diff --git a/doc/device-tree-bindings/device.txt b/doc/device-tree-bindings/device.txt
index 27bd3978d9..7140339623 100644
--- a/doc/device-tree-bindings/device.txt
+++ b/doc/device-tree-bindings/device.txt
@@ -17,6 +17,8 @@  the acpi,compatible property.
     System) Device Name)
  - acpi,hid : Contains the string to use as the HID (Hardware ID)
     identifier _HID
+ - acpi,name : Provides the ACPI name for a device, which is a string consisting
+   of four alphanumeric character (upper case)
  - acpi,uid : _UID value for device
  - linux,probed : Tells U-Boot to add 'linux,probed' to the ACPI tables so that
     Linux will only load the driver if the device can be detected (e.g. on I2C
@@ -34,3 +36,14 @@  elan_touchscreen: elan-touchscreen at 10 {
 	interrupts-extended = <&acpi_gpe GPIO_21_IRQ IRQ_TYPE_EDGE_FALLING>;
 	linux,probed;
 };
+
+pcie-a0 at 14,0 {
+	reg = <0x0000a000 0 0 0 0>;
+	acpi,name = "RP01";
+	wifi: wifi {
+		compatible = "intel,generic-wifi";
+		acpi,ddn = "Intel WiFi";
+		acpi,name = "WF00";
+		interrupts-extended = <&acpi_gpe 0x3c 0>;
+	};
+};
diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
index 7b32694ad4..076fb4f1b4 100644
--- a/drivers/core/acpi.c
+++ b/drivers/core/acpi.c
@@ -9,9 +9,10 @@ 
 #define LOG_CATEOGRY	LOGC_ACPI
 
 #include <common.h>
-#include <malloc.h>
 #include <dm.h>
 #include <log.h>
+#include <malloc.h>
+#include <acpi/acpi_device.h>
 #include <dm/acpi.h>
 #include <dm/device-internal.h>
 #include <dm/root.h>
@@ -65,12 +66,20 @@  int acpi_copy_name(char *out_name, const char *name)
 int acpi_get_name(const struct udevice *dev, char *out_name)
 {
 	struct acpi_ops *aops;
+	const char *name;
+	int ret;
 
 	aops = device_get_acpi_ops(dev);
 	if (aops && aops->get_name)
 		return aops->get_name(dev, out_name);
+	name = dev_read_string(dev, "acpi,name");
+	if (name)
+		return acpi_copy_name(out_name, name);
+	ret = acpi_device_infer_name(dev, out_name);
+	if (ret)
+		return log_msg_ret("dev", ret);
 
-	return -ENOSYS;
+	return 0;
 }
 
 /**
diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c
index 125026da90..57b1c60fde 100644
--- a/drivers/i2c/sandbox_i2c.c
+++ b/drivers/i2c/sandbox_i2c.c
@@ -84,15 +84,6 @@  static int sandbox_i2c_xfer(struct udevice *bus, struct i2c_msg *msg,
 	return ops->xfer(emul, msg, nmsgs);
 }
 
-static int sandbox_i2c_get_name(const struct udevice *dev, char *out_name)
-{
-	return acpi_copy_name(out_name, "SI2C");
-}
-
-struct acpi_ops sandbox_i2c_acpi_ops = {
-	.get_name	= sandbox_i2c_get_name,
-};
-
 static const struct dm_i2c_ops sandbox_i2c_ops = {
 	.xfer		= sandbox_i2c_xfer,
 };
@@ -108,5 +99,4 @@  U_BOOT_DRIVER(i2c_sandbox) = {
 	.of_match = sandbox_i2c_ids,
 	.ops	= &sandbox_i2c_ops,
 	.priv_auto_alloc_size = sizeof(struct sandbox_i2c_priv),
-	ACPI_OPS_PTR(&sandbox_i2c_acpi_ops)
 };
diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 77797bf096..755f176861 100644
--- a/drivers/spi/sandbox_spi.c
+++ b/drivers/spi/sandbox_spi.c
@@ -134,15 +134,6 @@  static int sandbox_spi_get_mmap(struct udevice *dev, ulong *map_basep,
 	return 0;
 }
 
-static int sandbox_spi_get_name(const struct udevice *dev, char *out_name)
-{
-	return acpi_copy_name(out_name, "SSPI");
-}
-
-struct acpi_ops sandbox_spi_acpi_ops = {
-	.get_name	= sandbox_spi_get_name,
-};
-
 static const struct dm_spi_ops sandbox_spi_ops = {
 	.xfer		= sandbox_spi_xfer,
 	.set_speed	= sandbox_spi_set_speed,
@@ -161,5 +152,4 @@  U_BOOT_DRIVER(sandbox_spi) = {
 	.id	= UCLASS_SPI,
 	.of_match = sandbox_spi_ids,
 	.ops	= &sandbox_spi_ops,
-	ACPI_OPS_PTR(&sandbox_spi_acpi_ops)
 };
diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
index ffa11eceb7..0bb9a4eec1 100644
--- a/include/acpi/acpi_device.h
+++ b/include/acpi/acpi_device.h
@@ -384,4 +384,22 @@  int acpi_device_add_power_res(struct acpi_ctx *ctx, u32 tx_state_val,
 			      const struct gpio_desc *stop_gpio,
 			      uint stop_delay_ms, uint stop_off_delay_ms);
 
+/**
+ * acpi_device_infer_name() - Infer the name from its uclass or parent
+ *
+ * Many ACPI devices have a standard name that can be inferred from the uclass
+ * they are in, or the uclass of their parent. These rules are implemented in
+ * this function. It attempts to produce a name for a device based on these
+ * rules.
+ *
+ * NOTE: This currently supports only x86 devices. Feel free to enhance it for
+ * other architectures as needed.
+ *
+ * @dev: Device to check
+ * @out_name: Place to put the name (must hold ACPI_NAME_MAX bytes)
+ * @return 0 if a name was found, -ENOENT if not found, -ENXIO if the device
+ *	sequence number could not be determined
+ */
+int acpi_device_infer_name(const struct udevice *dev, char *out_name);
+
 #endif
diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
index c66cafcfee..3c75b6d962 100644
--- a/lib/acpi/acpi_device.c
+++ b/lib/acpi/acpi_device.c
@@ -10,6 +10,8 @@ 
 #include <dm.h>
 #include <irq.h>
 #include <log.h>
+#include <usb.h>
+#include <acpi/acpigen.h>
 #include <acpi/acpi_device.h>
 #include <acpi/acpigen.h>
 #include <asm-generic/gpio.h>
@@ -715,3 +717,107 @@  int acpi_device_write_spi_dev(struct acpi_ctx *ctx, const struct udevice *dev)
 	return 0;
 }
 #endif /* CONFIG_SPI */
+
+static const char *acpi_name_from_id(enum uclass_id id)
+{
+	switch (id) {
+	case UCLASS_USB_HUB:
+		/* Root Hub */
+		return "RHUB";
+	/* DSDT: acpi/northbridge.asl */
+	case UCLASS_NORTHBRIDGE:
+		return "MCHC";
+	/* DSDT: acpi/lpc.asl */
+	case UCLASS_LPC:
+		return "LPCB";
+	/* DSDT: acpi/xhci.asl */
+	case UCLASS_USB:
+		/* This only supports USB3.0 controllers at present */
+		return "XHCI";
+	case UCLASS_PWM:
+		return "PWM";
+	default:
+		return NULL;
+	}
+}
+
+static int acpi_check_seq(const struct udevice *dev)
+{
+	if (dev->req_seq == -1) {
+		log_warning("Device '%s' has no seq\n", dev->name);
+		return log_msg_ret("no seq", -ENXIO);
+	}
+
+	return dev->req_seq;
+}
+
+/* If you change this function, add test cases to dm_test_acpi_get_name() */
+int acpi_device_infer_name(const struct udevice *dev, char *out_name)
+{
+	enum uclass_id parent_id = UCLASS_INVALID;
+	enum uclass_id id;
+	const char *name = NULL;
+
+	id = device_get_uclass_id(dev);
+	if (dev_get_parent(dev))
+		parent_id = device_get_uclass_id(dev_get_parent(dev));
+
+	if (id == UCLASS_SOUND)
+		name = "HDAS";
+	else if (id == UCLASS_PCI)
+		name = "PCI0";
+	else if (device_is_on_pci_bus(dev))
+		name = acpi_name_from_id(id);
+	if (!name) {
+		switch (parent_id) {
+		case UCLASS_USB: {
+			struct usb_device *udev = dev_get_parent_priv(dev);
+
+			sprintf(out_name, udev->speed >= USB_SPEED_SUPER ?
+				"HS%02d" : "FS%02d", udev->portnr);
+			name = out_name;
+			break;
+		}
+		default:
+			break;
+		}
+	}
+	if (!name) {
+		int num;
+
+		switch (id) {
+		/* DSDT: acpi/lpss.asl */
+		case UCLASS_SERIAL:
+			num = acpi_check_seq(dev);
+			if (num < 0)
+				return num;
+			sprintf(out_name, "URT%d", num);
+			name = out_name;
+			break;
+		case UCLASS_I2C:
+			num = acpi_check_seq(dev);
+			if (num < 0)
+				return num;
+			sprintf(out_name, "I2C%d", num);
+			name = out_name;
+			break;
+		case UCLASS_SPI:
+			num = acpi_check_seq(dev);
+			if (num < 0)
+				return num;
+			sprintf(out_name, "SPI%d", num);
+			name = out_name;
+			break;
+		default:
+			break;
+		}
+	}
+	if (!name) {
+		log_warning("No name for device '%s'\n", dev->name);
+		return -ENOENT;
+	}
+	if (name != out_name)
+		acpi_copy_name(out_name, name);
+
+	return 0;
+}
diff --git a/test/dm/acpi.c b/test/dm/acpi.c
index 1abde65c8c..69ca0902aa 100644
--- a/test/dm/acpi.c
+++ b/test/dm/acpi.c
@@ -124,12 +124,52 @@  UCLASS_DRIVER(testacpi) = {
 static int dm_test_acpi_get_name(struct unit_test_state *uts)
 {
 	char name[ACPI_NAME_MAX];
-	struct udevice *dev;
+	struct udevice *dev, *dev2, *i2c, *spi, *serial, *timer, *sound;
+	struct udevice *pci, *root;
 
+	/* Test getting the name from the driver */
 	ut_assertok(uclass_first_device_err(UCLASS_TEST_ACPI, &dev));
 	ut_assertok(acpi_get_name(dev, name));
 	ut_asserteq_str(ACPI_TEST_DEV_NAME, name);
 
+	/* Test getting the name from the device tree */
+	ut_assertok(uclass_get_device_by_name(UCLASS_TEST_FDT, "a-test",
+					      &dev2));
+	ut_assertok(acpi_get_name(dev2, name));
+	ut_asserteq_str("GHIJ", name);
+
+	/* Test getting the name from acpi_device_get_name() */
+	ut_assertok(uclass_first_device(UCLASS_I2C, &i2c));
+	ut_assertok(acpi_get_name(i2c, name));
+	ut_asserteq_str("I2C0", name);
+
+	ut_assertok(uclass_first_device(UCLASS_SPI, &spi));
+	ut_assertok(acpi_get_name(spi, name));
+	ut_asserteq_str("SPI0", name);
+
+	/* The uart has no sequence number, so this should fail */
+	ut_assertok(uclass_first_device(UCLASS_SERIAL, &serial));
+	ut_asserteq(-ENXIO, acpi_get_name(serial, name));
+
+	/* ACPI doesn't know about the timer */
+	ut_assertok(uclass_first_device(UCLASS_TIMER, &timer));
+	ut_asserteq(-ENOENT, acpi_get_name(timer, name));
+
+	/* May as well test the rest of the cases */
+	ut_assertok(uclass_first_device(UCLASS_SOUND, &sound));
+	ut_assertok(acpi_get_name(sound, name));
+	ut_asserteq_str("HDAS", name);
+
+	ut_assertok(uclass_first_device(UCLASS_PCI, &pci));
+	ut_assertok(acpi_get_name(pci, name));
+	ut_asserteq_str("PCI0", name);
+
+	ut_assertok(uclass_first_device(UCLASS_ROOT, &root));
+	ut_assertok(acpi_get_name(root, name));
+	ut_asserteq_str("\\_SB", name);
+
+	/* Note that we don't have tests for acpi_name_from_id() */
+
 	return 0;
 }
 DM_TEST(dm_test_acpi_get_name, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);