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

Bin Meng bmeng.cn at gmail.com
Mon Jun 29 07:39:36 CEST 2020


Hi Simon,

On Sun, Jun 14, 2020 at 10:55 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 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          |  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 caf935cfdf..5d4d3daa28 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 c1fc364550..acc2e3f7e9 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 5019a79528..d076b452b5 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, 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.
> + *
> + * @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 912369498a..c6560e37bc 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>
> @@ -711,3 +713,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";

What about USB 2.0 controllers?

> +       case UCLASS_PWM:
> +               return "PWM";
> +       default:
> +               return NULL;
> +       }
> +}

The above mapping seems to only work with x86 PCI devices? What about
other architectures? The API name is too generic if this is only for
86,

> +
> +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);
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index 3999825953..ea2033a27c 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -307,7 +307,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);
>
> @@ -343,7 +343,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);
>
> --

Regards,
Bin


More information about the U-Boot mailing list