History

rust: driver: remove open-coded matching logic

With device ID info now including pointers instead of indices, the open-coded ACPI/OF matching is no longer needed and can be replaced with `device_get_match_data`. With the removal of open-coded matching, the exposed functions and helpers are also removed; this effectively reverts most of commit 2690d071584e ("rust: ACPI: fix missing match data for PRP0001").

Gary Guo committed 4 months ago

Commit 2c5d17c1 parent 0b763359 files · +22 −162Browse files

Unified · Split

9 files
  1. MAINTAINERS +0 −1
  2. drivers/acpi/bus.c +3 −3
  3. include/acpi/acpi_bus.h +0 −11
  4. rust/helpers/acpi.c +0 −16
  5. rust/helpers/helpers.c +0 −1
  6. rust/kernel/driver.rs +11 −126
  7. rust/kernel/i2c.rs +4 −2
  8. rust/kernel/platform.rs +2 −1
  9. rust/kernel/serdev.rs +2 −1
MAINTAINERS+0−1
@@ -297,7 +297,6 @@ F: include/linux/acpi.h · 296 unmodified lines
297F: include/linux/fwnode.h297F: include/linux/fwnode.h
298F: include/linux/fw_table.h298F: include/linux/fw_table.h
299F: lib/fw_table.c299F: lib/fw_table.c
300F: rust/helpers/acpi.c
301F: rust/kernel/acpi.rs300F: rust/kernel/acpi.rs
302F: tools/power/acpi/301F: tools/power/acpi/
303 302
drivers/acpi/bus.c+3−3
@@ -898,9 +898,9 @@ const struct acpi_device *acpi_companion_match(const struct device *dev) · 897 unmodified lines
898 * identifiers and a _DSD object with the "compatible" property, use that898 * identifiers and a _DSD object with the "compatible" property, use that
899 * property to match against the given list of identifiers.899 * property to match against the given list of identifiers.
900 */900 */
901bool acpi_of_match_device(const struct acpi_device *adev,901static bool acpi_of_match_device(const struct acpi_device *adev,
902 const struct of_device_id *of_match_table,902 const struct of_device_id *of_match_table,
903 const struct of_device_id **of_id)903 const struct of_device_id **of_id)
904{904{
905 const union acpi_object *of_compatible, *obj;905 const union acpi_object *of_compatible, *obj;
906 int i, nval;906 int i, nval;
include/acpi/acpi_bus.h+0−11
@@ -189,10 +189,6 @@ struct acpi_driver { · 188 unmodified lines
189 * -----------189 * -----------
190 */190 */
191 191
192bool acpi_of_match_device(const struct acpi_device *adev,
193 const struct of_device_id *of_match_table,
194 const struct of_device_id **of_id);
195
196/* Status (_STA) */192/* Status (_STA) */
197 193
198struct acpi_device_status {194struct acpi_device_status {
@@ -1000,13 +996,6 @@ int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices); · 801 unmodified lines
1000u32 arch_acpi_add_auto_dep(acpi_handle handle);996u32 arch_acpi_add_auto_dep(acpi_handle handle);
1001#else /* CONFIG_ACPI */997#else /* CONFIG_ACPI */
1002 998
1003static inline bool acpi_of_match_device(const struct acpi_device *adev,
1004 const struct of_device_id *of_match_table,
1005 const struct of_device_id **of_id)
1006{
1007 return false;
1008}
1009
1010static inline int register_acpi_bus_type(void *bus) { return 0; }999static inline int register_acpi_bus_type(void *bus) { return 0; }
1011static inline int unregister_acpi_bus_type(void *bus) { return 0; }1000static inline int unregister_acpi_bus_type(void *bus) { return 0; }
1012 1001
rust/helpers/acpi.c+0−16
@@ -1,16 +0,0 @@
1// SPDX-License-Identifier: GPL-2.0
2
3#include <linux/acpi.h>
4#include <acpi/acpi_bus.h>
5
6__rust_helper bool rust_helper_acpi_of_match_device(const struct acpi_device *adev,
7 const struct of_device_id *of_match_table,
8 const struct of_device_id **of_id)
9{
10 return acpi_of_match_device(adev, of_match_table, of_id);
11}
12
13__rust_helper struct acpi_device *rust_helper_to_acpi_device_node(struct fwnode_handle *fwnode)
14{
15 return to_acpi_device_node(fwnode);
16}
rust/helpers/helpers.c+0−1
@@ -38,7 +38,6 @@ · 37 unmodified lines
38#define __rust_helper __always_inline38#define __rust_helper __always_inline
39#endif39#endif
40 40
41#include "acpi.c"
42#include "atomic.c"41#include "atomic.c"
43#include "atomic_ext.c"42#include "atomic_ext.c"
44#include "auxiliary.c"43#include "auxiliary.c"
rust/kernel/driver.rs+11−126
@@ -107,7 +107,6 @@ · 106 unmodified lines
107use crate::{107use crate::{
108 acpi,108 acpi,
109 device,109 device,
110 device_id::RawDeviceIdIndex,
111 of,110 of,
112 prelude::*,111 prelude::*,
113 types::Opaque,112 types::Opaque,
@@ -292,26 +291,6 @@ macro_rules! module_driver { · 178 unmodified lines
292 }291 }
293}292}
294 293
295// Calling the FFI function directly from the `Adapter` impl may result in it being called
296// directly from driver modules. This happens since the Rust compiler will use monomorphisation, so
297// it might happen that functions are instantiated within the calling driver module. For now, work
298// around this with `#[inline(never)]` helpers.
299//
300// TODO: Remove once a more generic solution has been implemented. For instance, we may be able to
301// leverage `bindgen` to take care of this depending on whether a symbol is (already) exported.
302#[inline(never)]
303#[allow(clippy::missing_safety_doc)]
304#[allow(dead_code)]
305#[must_use]
306unsafe fn acpi_of_match_device(
307 adev: *const bindings::acpi_device,
308 of_match_table: *const bindings::of_device_id,
309 of_id: *mut *const bindings::of_device_id,
310) -> bool {
311 // SAFETY: Safety requirements are the same as `bindings::acpi_of_match_device`.
312 unsafe { bindings::acpi_of_match_device(adev, of_match_table, of_id) }
313}
314
315/// The bus independent adapter to match a drivers and a devices.294/// The bus independent adapter to match a drivers and a devices.
316///295///
317/// This trait should be implemented by the bus specific adapter, which represents the connection296/// This trait should be implemented by the bus specific adapter, which represents the connection
@@ -325,117 +304,23 @@ pub trait Adapter { · 7 unmodified lines
325 /// The [`acpi::IdTable`] of the corresponding driver304 /// The [`acpi::IdTable`] of the corresponding driver
326 fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>>;305 fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>>;
327 306
328 /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
329 ///
330 /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
331 fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
332 #[cfg(not(CONFIG_ACPI))]
333 {
334 let _ = dev;
335 None
336 }
337
338 #[cfg(CONFIG_ACPI)]
339 {
340 let table = Self::acpi_id_table()?;
341
342 // SAFETY:
343 // - `table` has static lifetime, hence it's valid for read,
344 // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
345 let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
346
347 if raw_id.is_null() {
348 None
349 } else {
350 // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct acpi_device_id`
351 // and does not add additional invariants, so it's safe to transmute.
352 let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
353
354 // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
355 Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
356 }
357 }
358 }
359
360 /// The [`of::IdTable`] of the corresponding driver.307 /// The [`of::IdTable`] of the corresponding driver.
361 fn of_id_table() -> Option<of::IdTable<Self::IdInfo>>;308 fn of_id_table() -> Option<of::IdTable<Self::IdInfo>>;
362 309
363 /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any.
364 ///
365 /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
366 fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
367 let table = Self::of_id_table()?;
368
369 #[cfg(not(any(CONFIG_OF, CONFIG_ACPI)))]
370 {
371 let _ = (dev, table);
372 }
373
374 #[cfg(CONFIG_OF)]
375 {
376 // SAFETY:
377 // - `table` has static lifetime, hence it's valid for read,
378 // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
379 let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
380
381 if !raw_id.is_null() {
382 // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
383 // and does not add additional invariants, so it's safe to transmute.
384 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
385
386 // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
387 return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
388 }
389 }
390
391 #[cfg(CONFIG_ACPI)]
392 {
393 use core::ptr;
394 use device::property::FwNode;
395
396 let mut raw_id = ptr::null();
397
398 let fwnode = dev.fwnode().map_or(ptr::null_mut(), FwNode::as_raw);
399
400 // SAFETY: `fwnode` is a pointer to a valid `fwnode_handle`. A null pointer will be
401 // passed through the function.
402 let adev = unsafe { bindings::to_acpi_device_node(fwnode) };
403
404 // SAFETY:
405 // - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
406 // valid as long as `dev` is alive.
407 // - `table` has static lifetime, hence it's valid for read.
408 if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {
409 // SAFETY:
410 // - the function returns true, therefore `raw_id` has been set to a pointer to a
411 // valid `of_device_id`.
412 // - `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
413 // and does not add additional invariants, so it's safe to transmute.
414 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
415
416 // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
417 return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
418 }
419 }
420
421 None
422 }
423
424 /// Returns the driver's private data from the matching entry of any of the ID tables, if any.310 /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
425 ///311 ///
426 /// If this returns `None`, it means that there is no match in any of the ID tables directly312 /// If this returns `None`, it means that there is no match in any of the ID tables directly
427 /// associated with a [`device::Device`].313 /// associated with a [`device::Device`].
428 fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {314 ///
429 let id = Self::acpi_id_info(dev);315 /// # Safety
430 if id.is_some() {316 ///
431 return id;317 /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
432 }318 #[inline]
433 319 unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
434 let id = Self::of_id_info(dev);320 // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
435 if id.is_some() {321 let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };
436 return id;322
437 }323 // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
438 324 unsafe { data.cast::<Self::IdInfo>().as_ref() }
439 None
440 }325 }
441}326}
rust/kernel/i2c.rs+4−2
@@ -149,8 +149,10 @@ impl<T: Driver> Adapter<T> { · 148 unmodified lines
149 // INVARIANT: `idev` is valid for the duration of `probe_callback()`.149 // INVARIANT: `idev` is valid for the duration of `probe_callback()`.
150 let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };150 let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
151 151
152 let info =152 let info = Self::i2c_id_info(idev).or_else(|| {
153 Self::i2c_id_info(idev).or_else(|| <Self as driver::Adapter>::id_info(idev.as_ref()));153 // SAFETY: `idev` matched data is of type `Self::IdInfo`.
154 unsafe { <Self as driver::Adapter>::id_info(idev.as_ref()) }
155 });
154 156
155 from_result(|| {157 from_result(|| {
156 let data = T::probe(idev, info);158 let data = T::probe(idev, info);
rust/kernel/platform.rs+2−1
@@ -100,7 +100,8 @@ impl<T: Driver> Adapter<T> { · 99 unmodified lines
100 //100 //
101 // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.101 // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
102 let pdev = unsafe { &*pdev.cast::<Device<device::CoreInternal<'_>>>() };102 let pdev = unsafe { &*pdev.cast::<Device<device::CoreInternal<'_>>>() };
103 let info = <Self as driver::Adapter>::id_info(pdev.as_ref());103 // SAFETY: `pdev` matched data is of type `Self::IdInfo`.
104 let info = unsafe { <Self as driver::Adapter>::id_info(pdev.as_ref()) };
104 105
105 from_result(|| {106 from_result(|| {
106 let data = T::probe(pdev, info);107 let data = T::probe(pdev, info);
rust/kernel/serdev.rs+2−1
@@ -162,7 +162,8 @@ impl<T: Driver> Adapter<T> { · 161 unmodified lines
162 //162 //
163 // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.163 // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
164 let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };164 let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
165 let info = <Self as driver::Adapter>::id_info(sdev.as_ref());165 // SAFETY: `sdev` matched data is of type `Self::IdInfo`.
166 let info = unsafe { <Self as driver::Adapter>::id_info(sdev.as_ref()) };
166 167
167 from_result(|| {168 from_result(|| {
168 sdev.as_ref().set_drvdata(try_pin_init!(PrivateData::<T> {169 sdev.as_ref().set_drvdata(try_pin_init!(PrivateData::<T> {