Re: [PATCH] rust: ACPI: fix missing match data for PRP0001

[Markus Probst]

On Sat, 2026-04-04 at 22:23 +0100, Gary Guo wrote:
> On Wed Apr 1, 2026 at 11:15 PM BST, Danilo Krummrich wrote:
> > On Wed Apr 1, 2026 at 8:46 PM CEST, Markus Probst wrote:
> > > On Wed, 2026-04-01 at 20:32 +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Apr 01, 2026 at 02:06:25PM +0000, Markus Probst wrote:
> > > > > Export `acpi_of_match_device` function and use it to match the of device
> > > > > table against ACPI PRP0001 in Rust.
> > > > > 
> > > > > This fixes id_info being None on ACPI PRP0001 devices.
> > > > > 
> > > > > Using `device_get_match_data` is not possible, because Rust stores an
> > > > > index in the of device id instead of a data pointer.
> > > > 
> > > > I'm confused, why are we open-coding this in the rust layer?  What do we
> > > > need to change in the C side to make both layers be able to call the
> > > > same function instead?
> > > No commit message I have seen has explained why it was done this way. I
> > > don't think we would need to change anything on the C side.
> > 
> > The Rust code stores an index into the array the contains the actual device ID
> > info in the driver_data field of a device ID instead of a raw pointer to the
> > device ID info.
> > 
> > The reason for this is that it was the only way to build this in a way that
> > results in an API that is convinient and obvious to use for drivers when
> > declaring the device ID table, can be evaluated in const context (i.e. at
> > compile time), and does not rely on unstable language features. Fulfilling all
> > three of those requirements at the same was a rather tricky one.
> > 
> > The unfortunate consequence is that device_get_match_data() does not give us a
> > pointer to the actual device ID info, but it gives us the index of the device ID
> > info in the device ID table.
> > 
> > The problem is that this does not really help, because now we know the index,
> > but not which table it belongs to.
> > 
> > I.e. we wouldn't know whether to call
> > 
> > 	Self::acpi_id_table().info(index)
> > 
> > or
> > 
> > 	Self::of_id_table().info(index)
> > 
> > to obtain the actual device ID info.
> > 
> > So, unfortunately, I think we have to open code this for now.
> > 
> > But I think this is still a minor inconvinience for being able to fulfill the
> > requirements mentioned above.
> 
> I think there might be a chance that we can use const_refs_to_static to actually
> put pointer there. Of course, doing so is probably still quite tricky with
> feature support given all const generics hackery that we're doing :)
> 
> I might have a go when I have time.
What does this mean for this patch series?

Will it be merged in the meantime or should we wait for your rewrite?
> 
> BTW, if most drivers use driver_data of ID as pointers, why is it defined as
> kernel_ulong_t instead of just `void*`?
The return type of `device_get_match_data` is `const void *`.
`of_device_id->data` has type `const void *`.
`acpi_device_id->driver_data` and `pci_device_id->driver_data` has type
kernel_ulong_t.

Kernel doc of pci_device_id:
"
Data private to the driver.
Most drivers don't need to use driver_data field.
Best practice is to use driver_data as an index
into a static list of equivalent device types,
instead of using it as a pointer.
"
suggests it was intended for use as index in the first place.

Thanks
- Markus Probst

> 
> Best,
> Gary

Attachment: signature.asc
Description: This is a digitally signed message part