From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: acpi: better _CID handling
To: Miod Vallat <miod@online.fr>
Cc: tech@openbsd.org
Date: Tue, 11 Feb 2025 10:26:06 +0100

> Date: Tue, 11 Feb 2025 08:22:44 +0000
> From: Miod Vallat <miod@online.fr>
> 
> There are a few recent machines where the acpi _CID node is a Package
> containing two EisaId. This is not supported by the existing acpi code,
> and because of this, matches on _CID fails on devices where it should
> succeed.

Right.  Not seen this a lot and never seen it in a place where it
mattered for us.  But it is allowed by the spec.

> In particular, on some chromebooks, we see:
> 
>                 Name (_CID, Package (0x02)  // _CID: Compatible ID
>                 {
>                     EisaId ("PNP0303") /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */, 
>                     EisaId ("PNP030B")
>                 })
> 
> which prevents the keyboard controller from matching the recent
> pckbc@acpi attachment, unless the particular _HID is added to the
> devices list in that attachment.
> 
> The following quick'n'crude diff recognizes this case and uses the first
> item of the Package, which is expected to be the most generic and thus
> the most important to match on.

ok kettenis@

> Index: acpi.c
> ===================================================================
> RCS file: /OpenBSD/src/sys/dev/acpi/acpi.c,v
> diff -u -p -r1.441 acpi.c
> --- acpi.c	10 Feb 2025 11:41:19 -0000	1.441
> +++ acpi.c	11 Feb 2025 08:14:55 -0000
> @@ -2969,17 +2969,22 @@ acpi_parsehid(struct aml_node *node, voi
>      size_t devlen)
>  {
>  	struct acpi_softc	*sc = (struct acpi_softc *)arg;
> -	struct aml_value	 res;
> +	struct aml_value	 res, *cid;
>  	const char		*dev;
>  
>  	/* NB aml_eisaid returns a static buffer, this must come first */
>  	if (aml_evalname(acpi_softc, node->parent, "_CID", 0, NULL, &res) == 0) {
> +		if (res.type == AML_OBJTYPE_PACKAGE && res.length >= 1) {
> +			cid = res.v_package[0];
> +		} else {
> +			cid = &res;
> +		}
>  		switch (res.type) {
>  		case AML_OBJTYPE_STRING:
> -			dev = res.v_string;
> +			dev = cid->v_string;
>  			break;
>  		case AML_OBJTYPE_INTEGER:
> -			dev = aml_eisaid(aml_val2int(&res));
> +			dev = aml_eisaid(aml_val2int(cid));
>  			break;
>  		default:
>  			dev = "unknown";
> 
>