Re: [PATCH v2 34/35] dm: acpi: Enhance acpi_get_name()

Hi Simon,

-----"Simon Glass" sjg@chromium.org schrieb: -----

Betreff: [PATCH v2 34/35] dm: acpi: Enhance acpi_get_name()

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@chromium.org

Changes in v2: None 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 | 15 ++++ lib/acpi/acpi_device.c | 106 ++++++++++++++++++++++++++++ test/dm/acpi.c | 42 ++++++++++- test/dm/acpigen.c | 4 +- 9 files changed, 189 insertions(+), 25 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index aab3fb7471..750381bcac 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -111,6 +111,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@10 { interrupts-extended = <&acpi_gpe GPIO_21_IRQ IRQ_TYPE_EDGE_FALLING>; linux,probed; };

+pcie-a0@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 ebe9fde68e..ec70db3951 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 4264acc953..5aeb6bdf43 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(spi_sandbox) = { .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 adebd63c15..e99330ac6f 100644 --- a/include/acpi/acpi_device.h +++ b/include/acpi/acpi_device.h @@ -383,4 +383,19 @@ 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, of the uclass of their parent. These rules are implemented in

Typo: or the uclass

• • this function. It attempts to produce a name for a device based on these
• • rules.
• • @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 e5968bd3aa..b39ec8eb93 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> @@ -704,3 +706,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:

• 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 3fe0a0c9ff..bd4c66c471 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); diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c index afaccc7664..2922896c69 100644 --- a/test/dm/acpigen.c +++ b/test/dm/acpigen.c @@ -303,7 +303,7 @@ static int dm_test_acpi_i2c(struct unit_test_state *uts) ut_asserteq(6, get_unaligned((u16 *)(ptr + 10))); ut_asserteq(100000, get_unaligned((u32 *)(ptr + 12))); ut_asserteq(0x43, get_unaligned((u16 *)(ptr + 16)));

• ut_asserteq_str("\_SB.SI2C", (char *)ptr + 18);

• ut_asserteq_str("\_SB.I2C0", (char *)ptr + 18);

  free_context(&ctx);

@@ -339,7 +339,7 @@ static int dm_test_acpi_spi(struct unit_test_state *uts) ut_asserteq(0, ptr[17]); ut_asserteq(0, ptr[18]); ut_asserteq(0, get_unaligned((u16 *)(ptr + 19)));

• ut_asserteq_str("\_SB.SSPI", (char *)ptr + 21);

• ut_asserteq_str("\_SB.SPI0", (char *)ptr + 21);

  free_context(&ctx);

-- 2.26.2.645.ge9eca65c58-goog

Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com