Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Wednesday, January 14, 2015 10:42:23 AM David Woodhouse wrote:
> 
> --=-3dIl43yXcWwu/nzOqQWw
> Content-Type: multipart/mixed; boundary="=-ca0AFM5hvqL+pJIndiHh"
> 
> 
> --=-ca0AFM5hvqL+pJIndiHh
> Content-Type: text/plain; charset="UTF-8"
> Content-Transfer-Encoding: quoted-printable
> 
> I'm looking again at updating of_serial to work with ACPI properties.=20
> 
> Specifically, I want to support a serial port with a non-standard baud
> rate, something like this:
> 
>     Device(COM1) {
>         Name(_HID, EisaId("PNP0501"))
>         Name(_CID, EisaId("PRP0001"))
> 	Name(_DSD, Package() {
> 	    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>             Package () {
> 		Package () {"compatible", Package () {"ns16550a"}},
> 		Package () {"clock-frequency", 2457600},
> 	    }
> 	})
>         ...
>     }
> 
> Firstly, the of_serial driver doesn't even get *invoked* unless I do
> this:
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 0d08373..eb1201a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2083,6 +2086,8 @@ static int acpi_add_single_object(struct acpi_device =
> **child,
>  	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Added %s [%s] parent %s\n",
>  		dev_name(&device->dev), (char *) buffer.pointer,
>  		device->parent ? dev_name(&device->parent->dev) : "(null)"));
> +	if (device->data.of_compatible)
> +		acpi_create_platform_device(device);
>  	kfree(buffer.pointer);
>  	*child =3D device;
>  	return 0;
> 
> Now it doesn't work because it uses of_match_device() to look the device
> up and find the corresponding *data* for that entry in its match table.
> And without CONFIG_OF, I don't *have* of_match_device().
> 
> We've talked about the fact that the platform bus probe function doesn't
> pass you the match ID. Is that something we could potentially fix now
> that things are a little more unified?

We can do that in my view.

> Or do we expect drivers still to have to use something like
> of_match_id() to do the lookup again for themselves... and in that case
> should we make of_match_id() available or produce a new
> device_match_id() that they are expected to switch to?

So far we've been targeting drivers that already have of_match_id() rather
than ones that want to use the unified interface from the start and not
necessarily with CONFIG_OF set.  That looks like an overlooked use case to me
to be honest.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.