xtask: refactor feature selection and package validation (#3358)

* apply_feature_rules applies more things

* apply features in one place only, fix missing features and clippy warnings

* move various logic to package enum, re-add the ability to test packages with custom feature sets

* small cleanup

* simplify msrv check, fix CI

* review feedback

* try and fix msrv check

* rebase fixups

* use msrv toolchain in check

.github/workflows/ci.yml

@@ -133,39 +133,18 @@ jobs:
           components: rust-src
       - uses: Swatinem/rust-cache@v2
 
-      # Verify the MSRV for all RISC-V chips.
+      - name: Stable toolchain checks
+        run: rustc +stable --version --verbose
+      - name: esp toolchain checks
+        run: rustc +esp --version --verbose
+
+      # Verify the MSRV for all chips by running a lint session
       - name: msrv RISCV (esp-hal)
         run: |
-          cargo xtask build-package --features=esp32c2,ci --target=riscv32imc-unknown-none-elf   esp-hal
+          cargo +stable xtask lint-packages --chips esp32c2,esp32c3,esp32c6,esp32h2
-          cargo xtask build-package --features=esp32c3,ci --target=riscv32imc-unknown-none-elf   esp-hal
-          cargo xtask build-package --features=esp32c6,ci --target=riscv32imac-unknown-none-elf  esp-hal
-          cargo xtask build-package --features=esp32h2,ci --target=riscv32imac-unknown-none-elf  esp-hal
-
-      - name: msrv RISCV (esp-wifi)
-        run: |
-          cargo xtask build-package --features=esp32c2,wifi,ble,esp-hal/unstable --target=riscv32imc-unknown-none-elf   esp-wifi
-          cargo xtask build-package --features=esp32c3,wifi,ble,esp-hal/unstable --target=riscv32imc-unknown-none-elf   esp-wifi
-          cargo xtask build-package --features=esp32c6,wifi,ble,esp-hal/unstable --target=riscv32imac-unknown-none-elf  esp-wifi
-          cargo xtask build-package --features=esp32h2,ble,esp-hal/unstable --target=riscv32imac-unknown-none-elf  esp-wifi
-
-        # Verify the MSRV for all Xtensa chips:
       - name: msrv Xtensa (esp-hal)
         run: |
-          cargo xtask build-package --toolchain=esp --features=esp32,ci   --target=xtensa-esp32-none-elf   esp-hal
+          cargo +esp xtask lint-packages --chips esp32,esp32s2,esp32s3
-          cargo xtask build-package --toolchain=esp --features=esp32s2,ci --target=xtensa-esp32s2-none-elf esp-hal
-          cargo xtask build-package --toolchain=esp --features=esp32s3,ci --target=xtensa-esp32s3-none-elf esp-hal
-
-      - name: msrv Xtensa (esp-wifi)
-        run: |
-          cargo xtask build-package --toolchain=esp --features=esp32,wifi,ble,esp-hal/unstable   --target=xtensa-esp32-none-elf   esp-wifi
-          cargo xtask build-package --toolchain=esp --features=esp32s2,wifi,esp-hal/unstable     --target=xtensa-esp32s2-none-elf esp-wifi
-          cargo xtask build-package --toolchain=esp --features=esp32s3,wifi,ble,esp-hal/unstable --target=xtensa-esp32s3-none-elf esp-wifi
-
-      - name: msrv (esp-lp-hal)
-        run: |
-          cargo xtask build-package --features=esp32c6 --target=riscv32imac-unknown-none-elf esp-lp-hal
-          cargo xtask build-package --features=esp32s2 --target=riscv32imc-unknown-none-elf  esp-lp-hal
-          cargo xtask build-package --features=esp32s3 --target=riscv32imc-unknown-none-elf  esp-lp-hal
 
   # --------------------------------------------------------------------------
   # Format

esp-config/src/generate/mod.rs

@@ -90,7 +90,7 @@ pub fn generate_config(
                 continue;
             }
             markdown::write_doc_table_line(&mut doc_table, name, option);
-            markdown::write_summary_table_line(&mut selected_config, &name, value);
+            markdown::write_summary_table_line(&mut selected_config, name, value);
         }
 
         write_out_file(format!("{file_name}_config_table.md"), doc_table);

esp-config/src/generate/validator.rs

@@ -4,6 +4,8 @@ use serde::Serialize;
 
 use super::{snake_case, value::Value, Error};
 
+type CustomValidatorFn = Box<dyn Fn(&Value) -> Result<(), Error>>;
+
 /// Configuration value validation functions.
 #[derive(Serialize)]
 pub enum Validator {
@@ -22,13 +24,10 @@ pub enum Validator {
     /// type.
     #[serde(serialize_with = "serialize_custom")]
     #[serde(untagged)]
-    Custom(Box<dyn Fn(&Value) -> Result<(), Error>>),
+    Custom(CustomValidatorFn),
 }
 
-pub(crate) fn serialize_custom<S>(
+pub(crate) fn serialize_custom<S>(_: &CustomValidatorFn, serializer: S) -> Result<S::Ok, S::Error>
-    _: &Box<dyn Fn(&Value) -> Result<(), Error>>,
-    serializer: S,
-) -> Result<S::Ok, S::Error>
 where
     S: serde::Serializer,
 {
@@ -75,25 +74,22 @@ impl Validator {
         config_key: &str,
         actual_value: &Value,
     ) {
-        match self {
+        if let Validator::Enumeration(values) = self {
-            Validator::Enumeration(values) => {
+            for possible_value in values {
-                for possible_value in values {
-                    writeln!(
-                        stdout,
-                        "cargo:rustc-check-cfg=cfg({config_key}_{})",
-                        snake_case(possible_value)
-                    )
-                    .ok();
-                }
-
                 writeln!(
                     stdout,
-                    "cargo:rustc-cfg={config_key}_{}",
+                    "cargo:rustc-check-cfg=cfg({config_key}_{})",
-                    snake_case(&actual_value.to_string())
+                    snake_case(possible_value)
                 )
                 .ok();
             }
-            _ => (),
+
+            writeln!(
+                stdout,
+                "cargo:rustc-cfg={config_key}_{}",
+                snake_case(&actual_value.to_string())
+            )
+            .ok();
         }
     }
 }
@@ -109,9 +105,9 @@ pub(crate) fn enumeration(values: &Vec<String>, value: &Value) -> Result<(), Err
 
         Ok(())
     } else {
-        return Err(Error::parse(
+        Err(Error::parse(
             "Validator::Enumeration can only be used with string values",
-        ));
+        ))
     }
 }
 

esp-config/src/generate/value.rs

@@ -33,7 +33,7 @@ impl Value {
                     [b'0', b'x', ..] => i128::from_str_radix(&s[2..], 16),
                     [b'0', b'o', ..] => i128::from_str_radix(&s[2..], 8),
                     [b'0', b'b', ..] => i128::from_str_radix(&s[2..], 2),
-                    _ => i128::from_str_radix(&s, 10),
+                    _ => s.parse(),
                 }
                 .map_err(|_| Error::parse(format!("Expected valid intger value, found: '{s}'")))?;
 

esp-hal-procmacros/src/lib.rs

@@ -8,27 +8,24 @@
 //!
 //! - Placing statics and functions into RAM
 //! - Marking interrupt handlers
-//! - Automatically creating an `embassy` executor instance and spawning the
+//! - Blocking and Async `#[main]` macros
-//!   defined entry point
 //!
 //! These macros offer developers a convenient way to control memory placement
 //! and define interrupt handlers in their embedded applications, allowing for
 //! optimized memory usage and precise handling of hardware interrupts.
 //!
 //! Key Components:
-//!  - [`interrupt`](attr.interrupt.html) - Attribute macro for marking
+//!  - [`handler`](macro@handler) - Attribute macro for marking interrupt
-//!    interrupt handlers. Interrupt handlers are used to handle specific
+//!    handlers. Interrupt handlers are used to handle specific hardware
-//!    hardware interrupts generated by peripherals.
+//!    interrupts generated by peripherals.
 //!
-//!    The macro allows users to specify the interrupt name explicitly or use
+//!  - [`ram`](macro@ram) - Attribute macro for placing statics and functions
-//!    the function name to match the interrupt.
+//!    into specific memory sections, such as SRAM or RTC RAM (slow or fast)
-//!  - [`main`](attr.main.html) - Creates a new `executor` instance and declares
+//!    with different initialization options. See its documentation for details.
-//!    an application entry point spawning the corresponding function body as an
+//!
-//!    async task.
+//!  - [`embassy::main`](macro@embassy_main) - Creates a new `executor` instance
-//!  - [`ram`](attr.ram.html) - Attribute macro for placing statics and
+//!    and declares an application entry point spawning the corresponding
-//!    functions into specific memory sections, such as SRAM or RTC RAM (slow or
+//!    function body as an async task.
-//!    fast) with different initialization options. See its documentation for
-//!    details.
 //!
 //! ## Examples
 //!
@@ -49,7 +46,7 @@
 
 use proc_macro::TokenStream;
 
-mod blocking_main;
+mod blocking;
 mod builder;
 #[cfg(feature = "embassy")]
 mod embassy;
@@ -209,7 +206,7 @@ pub fn embassy_main(args: TokenStream, item: TokenStream) -> TokenStream {
 /// ```
 #[proc_macro_attribute]
 pub fn blocking_main(args: TokenStream, input: TokenStream) -> TokenStream {
-    blocking_main::main(args, input)
+    blocking::main(args, input)
 }
 
 /// Automatically implement the [Builder Lite] pattern for a struct.

esp-hal/Cargo.toml

@@ -146,9 +146,6 @@ defmt = [
 ## Use externally connected PSRAM (`quad` by default, can be configured to `octal` via ESP_HAL_CONFIG_PSRAM_MODE)
 psram = []
 
-# This feature is intended for testing; you probably don't want to enable it:
-ci = ["defmt", "bluetooth"]
-
 #! ### Unstable APIs
 #! Unstable APIs are drivers and features that are not yet ready for general use.
 #! They may be incomplete, have bugs, or be subject to change without notice.

esp-hal/src/soc/esp32/psram.rs

@@ -889,7 +889,7 @@ pub(crate) mod utils {
                 // Enable MOSI
                 spi.user().modify(|_, w| w.usr_mosi().clear_bit());
                 // Load send buffer
-                let len = (p_in_data.tx_data_bit_len + 31) / 32;
+                let len = p_in_data.tx_data_bit_len.div_ceil(32);
                 if !p_tx_val.is_null() {
                     for i in 0..len {
                         spi.w(0)

esp-metadata/src/generate_cfg.rs

@@ -100,7 +100,7 @@ pub enum Chip {
 }
 
 impl Chip {
-    pub fn target(&self) -> &str {
+    pub fn target(&self) -> &'static str {
         use Chip::*;
 
         match self {
@@ -118,7 +118,7 @@ impl Chip {
         matches!(self, Esp32c6 | Esp32s2 | Esp32s3)
     }
 
-    pub fn lp_target(&self) -> Result<&str> {
+    pub fn lp_target(&self) -> Result<&'static str> {
         use Chip::*;
 
         match self {
@@ -186,6 +186,20 @@ impl Config {
         }
     }
 
+    /// Create an empty configuration
+    pub fn empty() -> Self {
+        Self {
+            device: Device {
+                name: "".to_owned(),
+                arch: Arch::RiscV,
+                cores: Cores::Single,
+                peripherals: Vec::new(),
+                symbols: Vec::new(),
+                memory: Vec::new(),
+            },
+        }
+    }
+
     /// The name of the device.
     pub fn name(&self) -> String {
         self.device.name.clone()

xtask/src/documentation.rs

@@ -58,7 +58,7 @@ pub fn build_documentation(
                 _ => vec![Chip::Esp32c6],
             }
         } else {
-            log::warn!("Package '{package}' does not have chip features, ignoring argument");
+            log::debug!("Package '{package}' does not have chip features, ignoring argument");
             vec![]
         };
 
@@ -97,7 +97,7 @@ fn build_documentation_for_package(
 
     // Ensure that the package/chip combination provided are valid:
     if let Some(chip) = chip {
-        if let Err(err) = crate::validate_package_chip(package, &chip) {
+        if let Err(err) = package.validate_package_chip(&chip) {
             log::warn!("{err}");
             return Ok(());
         }
@@ -192,15 +192,17 @@ fn cargo_doc(workspace: &Path, package: Package, chip: Option<Chip>) -> Result<P
     // Determine the appropriate build target for the given package and chip,
     // if we're able to:
     let target = if let Some(ref chip) = chip {
-        Some(crate::target_triple(package, chip)?)
+        Some(package.target_triple(chip)?)
     } else {
         None
     };
 
     let mut features = vec![];
-    if let Some(chip) = chip {
+    if let Some(chip) = &chip {
         features.push(chip.to_string());
-        features.extend(apply_feature_rules(&package, Config::for_chip(&chip)));
+        features.extend(package.feature_rules(Config::for_chip(&chip)));
+    } else {
+        features.extend(package.feature_rules(&Config::empty()));
     }
 
     // Build up an array of command-line arguments to pass to `cargo`:
@@ -249,48 +251,6 @@ fn cargo_doc(workspace: &Path, package: Package, chip: Option<Chip>) -> Result<P
     Ok(crate::windows_safe_path(&docs_path))
 }
 
-fn apply_feature_rules(package: &Package, config: &Config) -> Vec<String> {
-    let chip_name = &config.name();
-
-    let mut features = vec![];
-    match package {
-        Package::EspBacktrace => features.push("defmt".to_owned()),
-        Package::EspConfig => features.push("build".to_owned()),
-        Package::EspHal => {
-            features.push("unstable".to_owned());
-            features.push("ci".to_owned());
-            match chip_name.as_str() {
-                "esp32" => features.push("psram".to_owned()),
-                "esp32s2" => features.push("psram".to_owned()),
-                "esp32s3" => features.push("psram".to_owned()),
-                _ => {}
-            };
-        }
-        Package::EspWifi => {
-            features.push("esp-hal/unstable".to_owned());
-            if config.contains("wifi") {
-                features.push("wifi".to_owned());
-                features.push("esp-now".to_owned());
-                features.push("sniffer".to_owned());
-                features.push("smoltcp/proto-ipv4".to_owned());
-                features.push("smoltcp/proto-ipv6".to_owned());
-            }
-            if config.contains("ble") {
-                features.push("ble".to_owned());
-            }
-            if config.contains("wifi") && config.contains("ble") {
-                features.push("coex".to_owned());
-            }
-        }
-        Package::EspHalEmbassy | Package::EspIeee802154 => {
-            features.push("esp-hal/unstable".to_owned());
-        },
-        _ => {}
-    }
-
-    features
-}
-
 fn patch_documentation_index_for_package(
     workspace: &Path,
     package: &Package,
@@ -454,7 +414,7 @@ fn generate_documentation_meta_for_package(
 
     for chip in chips {
         // Ensure that the package/chip combination provided are valid:
-        crate::validate_package_chip(&package, chip)?;
+        package.validate_package_chip(chip)?;
 
         // Build the context object required for rendering this particular build's
         // information on the documentation index:

xtask/src/lib.rs

@@ -6,10 +6,10 @@ use std::{
     process::Command,
 };
 
-use anyhow::{ensure, Context, Result};
+use anyhow::{anyhow, Context, Result};
 use cargo::CargoAction;
 use clap::ValueEnum;
-use esp_metadata::Chip;
+use esp_metadata::{Chip, Config};
 use strum::{Display, EnumIter, IntoEnumIterator as _};
 
 use crate::{cargo::CargoArgsBuilder, firmware::Metadata};
@@ -96,6 +96,135 @@ impl Package {
 
         matches!(self, EspBuild | EspConfig | EspMetadata)
     }
+
+    /// Given a device config, return the features which should be enabled for
+    /// this package.
+    pub fn feature_rules(&self, config: &Config) -> Vec<String> {
+        let mut features = vec![];
+        match self {
+            Package::EspBacktrace => features.push("defmt".to_owned()),
+            Package::EspConfig => features.push("build".to_owned()),
+            Package::EspHal => {
+                features.push("unstable".to_owned());
+                if config.contains("psram") {
+                    // TODO this doesn't test octal psram (since `ESP_HAL_CONFIG_PSRAM_MODE`
+                    // defaults to `quad`) as it would require a separate build
+                    features.push("psram".to_owned())
+                }
+                if config.contains("usb0") {
+                    features.push("usb-otg".to_owned());
+                }
+                if config.contains("bt") {
+                    features.push("bluetooth".to_owned());
+                }
+            }
+            Package::EspWifi => {
+                features.push("esp-hal/unstable".to_owned());
+                features.push("defmt".to_owned());
+                if config.contains("wifi") {
+                    features.push("wifi".to_owned());
+                    features.push("esp-now".to_owned());
+                    features.push("sniffer".to_owned());
+                    features.push("smoltcp/proto-ipv4".to_owned());
+                    features.push("smoltcp/proto-ipv6".to_owned());
+                }
+                if config.contains("ble") {
+                    features.push("ble".to_owned());
+                }
+                if config.contains("coex") {
+                    features.push("coex".to_owned());
+                }
+            }
+            Package::EspHalProcmacros => {
+                features.push("embassy".to_owned());
+            }
+            Package::EspHalEmbassy => {
+                features.push("esp-hal/unstable".to_owned());
+                features.push("defmt".to_owned());
+                features.push("executors".to_owned());
+            }
+            Package::EspIeee802154 => {
+                features.push("defmt".to_owned());
+                features.push("esp-hal/unstable".to_owned());
+            }
+            Package::EspLpHal => {
+                if config.contains("lp_core") {
+                    features.push("embedded-io".to_owned());
+                }
+            }
+            Package::EspPrintln => {
+                features.push("auto".to_owned());
+                features.push("defmt-espflash".to_owned());
+            }
+            Package::EspStorage => {
+                features.push("storage".to_owned());
+                features.push("nor-flash".to_owned());
+                features.push("low-level".to_owned());
+            }
+            Package::EspBootloaderEspIdf => {
+                features.push("defmt".to_owned());
+                features.push("validation".to_owned());
+            }
+            Package::EspAlloc => {
+                features.push("defmt".to_owned());
+            }
+            _ => {}
+        }
+
+        features
+    }
+
+    /// Additional feature rules to test subsets of features for a package.
+    pub fn lint_feature_rules(&self, _config: &Config) -> Vec<Vec<String>> {
+        let mut cases = Vec::new();
+
+        match self {
+            Package::EspWifi => {
+                // minimal set of features that when enabled _should_ still compile
+                cases.push(vec![
+                    "esp-hal/unstable".to_owned(),
+                    "builtin-scheduler".to_owned(),
+                ]);
+            }
+            _ => {}
+        }
+
+        cases
+    }
+
+    /// Return the target triple for a given package/chip pair.
+    pub fn target_triple(&self, chip: &Chip) -> Result<&'static str> {
+        if *self == Package::EspLpHal {
+            chip.lp_target()
+        } else {
+            Ok(chip.target())
+        }
+    }
+
+    /// Validate that the specified chip is valid for the specified package.
+    pub fn validate_package_chip(&self, chip: &Chip) -> Result<()> {
+        let device = Config::for_chip(chip);
+
+        let check = match self {
+            Package::EspIeee802154 => device.contains("ieee802154"),
+            Package::EspLpHal => chip.has_lp_core(),
+            Package::XtensaLx | Package::XtensaLxRt | Package::XtensaLxRtProcMacros => {
+                chip.is_xtensa()
+            }
+            Package::EspRiscvRt => chip.is_riscv(),
+            _ => true,
+        };
+
+        if check {
+            Ok(())
+        } else {
+            Err(anyhow!(
+                "Invalid chip provided for package '{}': '{}'",
+                self,
+                chip
+            ))
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, Display, ValueEnum)]
@@ -536,24 +665,3 @@ pub fn package_version(workspace: &Path, package: Package) -> Result<semver::Ver
 pub fn windows_safe_path(path: &Path) -> PathBuf {
     PathBuf::from(path.to_str().unwrap().to_string().replace("\\\\?\\", ""))
 }
-
-/// Return the target triple for a given package/chip pair.
-pub fn target_triple(package: Package, chip: &Chip) -> Result<&str> {
-    if package == Package::EspLpHal {
-        chip.lp_target()
-    } else {
-        Ok(chip.target())
-    }
-}
-
-/// Validate that the specified chip is valid for the specified package.
-pub fn validate_package_chip(package: &Package, chip: &Chip) -> Result<()> {
-    ensure!(
-        *package != Package::EspLpHal || chip.has_lp_core(),
-        "Invalid chip provided for package '{}': '{}'",
-        package,
-        chip
-    );
-
-    Ok(())
-}

xtask/src/main.rs

@@ -7,12 +7,11 @@ use std::{
 
 use anyhow::{bail, ensure, Context as _, Result};
 use clap::{Args, Parser};
-use esp_metadata::{Arch, Chip, Config};
+use esp_metadata::{Chip, Config};
 use strum::IntoEnumIterator;
 use xtask::{
     cargo::{CargoAction, CargoArgsBuilder},
     firmware::Metadata,
-    target_triple,
     Package,
     Version,
 };
@@ -165,7 +164,7 @@ struct LintPackagesArgs {
     packages: Vec<Package>,
 
     /// Lint for a specific chip
-    #[arg(long, value_enum, default_values_t = Chip::iter())]
+    #[arg(long, value_enum, value_delimiter = ',', default_values_t = Chip::iter())]
     chips: Vec<Chip>,
 
     /// Automatically apply fixes
@@ -253,7 +252,7 @@ fn main() -> Result<()> {
 
 fn examples(workspace: &Path, mut args: ExampleArgs, action: CargoAction) -> Result<()> {
     // Ensure that the package/chip combination provided are valid:
-    xtask::validate_package_chip(&args.package, &args.chip)?;
+    args.package.validate_package_chip(&args.chip)?;
 
     // If the 'esp-hal' package is specified, what we *really* want is the
     // 'examples' package instead:
@@ -305,7 +304,7 @@ fn build_examples(
     out_path: PathBuf,
 ) -> Result<()> {
     // Determine the appropriate build target for the given package and chip:
-    let target = target_triple(args.package, &args.chip)?;
+    let target = args.package.target_triple(&args.chip)?;
 
     if examples
         .iter()
@@ -346,7 +345,7 @@ fn build_examples(
 
 fn run_example(args: ExampleArgs, examples: Vec<Metadata>, package_path: &Path) -> Result<()> {
     // Determine the appropriate build target for the given package and chip:
-    let target = target_triple(args.package, &args.chip)?;
+    let target = args.package.target_triple(&args.chip)?;
 
     // Filter the examples down to only the binary we're interested in, assuming it
     // actually supports the specified chip:
@@ -375,7 +374,7 @@ fn run_example(args: ExampleArgs, examples: Vec<Metadata>, package_path: &Path)
 
 fn run_examples(args: ExampleArgs, examples: Vec<Metadata>, package_path: &Path) -> Result<()> {
     // Determine the appropriate build target for the given package and chip:
-    let target = target_triple(args.package, &args.chip)?;
+    let target = args.package.target_triple(&args.chip)?;
 
     // Filter the examples down to only the binaries we're interested in
     let mut examples: Vec<Metadata> = examples
@@ -451,7 +450,7 @@ fn tests(workspace: &Path, args: TestArgs, action: CargoAction) -> Result<()> {
     let package_path = xtask::windows_safe_path(&workspace.join("hil-test"));
 
     // Determine the appropriate build target for the given package and chip:
-    let target = target_triple(Package::HilTest, &args.chip)?;
+    let target = Package::HilTest.target_triple(&args.chip)?;
 
     // Load all tests which support the specified chip and parse their metadata:
     let mut tests = xtask::firmware::load(&package_path.join("tests"))?
@@ -636,188 +635,36 @@ fn lint_packages(workspace: &Path, args: LintPackagesArgs) -> Result<()> {
     let mut packages = args.packages;
     packages.sort();
 
-    for package in packages {
+    for package in packages.iter().filter(|p| p.is_published()) {
-        let path = workspace.join(package.to_string());
-
         // Unfortunately each package has its own unique requirements for
         // building, so we need to handle each individually (though there
         // is *some* overlap)
-
         for chip in &args.chips {
             let device = Config::for_chip(chip);
 
-            match package {
+            if let Err(_) = package.validate_package_chip(chip) {
-                Package::EspBacktrace => {
+                continue;
-                    lint_package(
+            }
-                        chip,
+
-                        &path,
+            let feature_sets = [
-                        &[
+                vec![package.feature_rules(device)], // initially test all features
-                            "--no-default-features",
+                package.lint_feature_rules(device), // add separate test cases
-                            &format!("--target={}", chip.target()),
+            ]
-                        ],
+            .concat();
-                        &[&format!("{chip},defmt")],
+
-                        args.fix,
+            for mut features in feature_sets {
-                        package.build_on_host(),
+                if package.has_chip_features() {
-                    )?;
+                    features.push(device.name())
                 }
 
-                Package::EspHal => {
+                lint_package(
-                    let mut features = format!("{chip},ci,unstable");
+                    workspace,
-
+                    *package,
-                    // Cover all esp-hal features where a device is supported
+                    chip,
-                    if device.contains("usb0") {
+                    &["--no-default-features"],
-                        features.push_str(",usb-otg")
+                    &features,
-                    }
+                    args.fix,
-                    if device.contains("bt") {
+                )?;
-                        features.push_str(",bluetooth")
-                    }
-                    if device.contains("psram") {
-                        // TODO this doesn't test octal psram (since `ESP_HAL_CONFIG_PSRAM_MODE`
-                        // defaults to `quad`) as it would require a separate build
-                        features.push_str(",psram")
-                    }
-
-                    lint_package(
-                        chip,
-                        &path,
-                        &[&format!("--target={}", chip.target())],
-                        &[&features],
-                        args.fix,
-                        package.build_on_host(),
-                    )?;
-                }
-
-                Package::EspHalEmbassy => {
-                    lint_package(
-                        chip,
-                        &path,
-                        &[&format!("--target={}", chip.target())],
-                        &[&format!("{chip},executors,defmt,esp-hal/unstable")],
-                        args.fix,
-                        package.build_on_host(),
-                    )?;
-                }
-
-                Package::EspIeee802154 => {
-                    if device.contains("ieee802154") {
-                        lint_package(
-                            chip,
-                            &path,
-                            &[&format!("--target={}", chip.target())],
-                            &[&format!("{chip},defmt,esp-hal/unstable")],
-                            args.fix,
-                            package.build_on_host(),
-                        )?;
-                    }
-                }
-                Package::EspLpHal => {
-                    if device.contains("lp_core") {
-                        lint_package(
-                            chip,
-                            &path,
-                            &[&format!("--target={}", chip.lp_target().unwrap())],
-                            &[&format!("{chip},embedded-io")],
-                            args.fix,
-                            package.build_on_host(),
-                        )?;
-                    }
-                }
-
-                Package::EspPrintln => {
-                    lint_package(
-                        chip,
-                        &path,
-                        &[&format!("--target={}", chip.target())],
-                        &[&format!("{chip},defmt-espflash")],
-                        args.fix,
-                        package.build_on_host(),
-                    )?;
-                }
-
-                Package::EspRiscvRt => {
-                    if matches!(device.arch(), Arch::RiscV) {
-                        lint_package(
-                            chip,
-                            &path,
-                            &[&format!("--target={}", chip.target())],
-                            &[""],
-                            args.fix,
-                            package.build_on_host(),
-                        )?;
-                    }
-                }
-
-                Package::EspStorage => {
-                    lint_package(
-                        chip,
-                        &path,
-                        &[&format!("--target={}", chip.target())],
-                        &[&format!("{chip},storage,nor-flash,low-level")],
-                        args.fix,
-                        package.build_on_host(),
-                    )?;
-                }
-
-                Package::EspWifi => {
-                    let minimal_features = format!("{chip},esp-hal/unstable,builtin-scheduler");
-                    let mut all_features = minimal_features.clone();
-
-                    all_features.push_str(",defmt");
-
-                    if device.contains("wifi") {
-                        all_features.push_str(",esp-now,sniffer")
-                    }
-                    if device.contains("bt") {
-                        all_features.push_str(",ble")
-                    }
-                    if device.contains("coex") {
-                        all_features.push_str(",coex")
-                    }
-                    lint_package(
-                        chip,
-                        &path,
-                        &[
-                            &format!("--target={}", chip.target()),
-                            "--no-default-features",
-                        ],
-                        &[&minimal_features, &all_features],
-                        args.fix,
-                        package.build_on_host(),
-                    )?;
-                }
-
-                Package::XtensaLx => {
-                    if matches!(device.arch(), Arch::Xtensa) {
-                        lint_package(
-                            chip,
-                            &path,
-                            &[&format!("--target={}", chip.target())],
-                            &[""],
-                            args.fix,
-                            package.build_on_host(),
-                        )?
-                    }
-                }
-
-                Package::XtensaLxRt => {
-                    if matches!(device.arch(), Arch::Xtensa) {
-                        lint_package(
-                            chip,
-                            &path,
-                            &[&format!("--target={}", chip.target())],
-                            &[&format!("{chip}")],
-                            args.fix,
-                            package.build_on_host(),
-                        )?
-                    }
-                }
-
-                // We will *not* check the following packages with `clippy`; this
-                // may or may not change in the future:
-                Package::Examples | Package::HilTest | Package::QaTest => {}
-
-                // By default, no `clippy` arguments are required:
-                _ => lint_package(chip, &path, &[], &[], args.fix, package.build_on_host())?,
             }
         }
     }
@@ -826,54 +673,53 @@ fn lint_packages(workspace: &Path, args: LintPackagesArgs) -> Result<()> {
 }
 
 fn lint_package(
+    workspace: &Path,
+    package: Package,
     chip: &Chip,
-    path: &Path,
     args: &[&str],
-    feature_sets: &[&str],
+    features: &[String],
     fix: bool,
-    build_on_host: bool,
 ) -> Result<()> {
-    for features in feature_sets {
+    log::info!(
-        log::info!(
+        "Linting package: {} ({}, features: {:?})",
-            "Linting package: {} ({}, features: {})",
+        package,
-            path.display(),
+        chip,
-            chip,
+        features
-            features
+    );
-        );
 
-        let builder = CargoArgsBuilder::default().subcommand("clippy");
+    let path = workspace.join(package.to_string());
 
-        let mut builder = if chip.is_xtensa() {
+    let mut builder = CargoArgsBuilder::default().subcommand("clippy");
-            let builder = if build_on_host {
-                builder
-            } else {
-                builder.arg("-Zbuild-std=core,alloc")
-            };
 
+    let mut builder = if !package.build_on_host() {
+        if chip.is_xtensa() {
             // We only overwrite Xtensas so that externally set nightly/stable toolchains
             // are not overwritten.
-            builder.toolchain("esp")
+            builder = builder.arg("-Zbuild-std=core,alloc");
-        } else {
+            builder = builder.toolchain("esp");
-            builder
-        };
-
-        for arg in args {
-            builder = builder.arg(arg.to_string());
         }
 
-        builder = builder.arg(format!("--features={features}"));
+        builder.target(package.target_triple(chip)?)
+    } else {
+        builder
+    };
 
-        let builder = if fix {
+    for arg in args {
-            builder.arg("--fix").arg("--lib").arg("--allow-dirty")
+        builder = builder.arg(arg.to_string());
-        } else {
-            builder.arg("--").arg("-D").arg("warnings").arg("--no-deps")
-        };
-
-        let cargo_args = builder.build();
-
-        xtask::cargo::run_with_env(&cargo_args, path, [("CI", "1")], false)?;
     }
 
+    builder = builder.arg(format!("--features={}", features.join(",")));
+
+    let builder = if fix {
+        builder.arg("--fix").arg("--lib").arg("--allow-dirty")
+    } else {
+        builder.arg("--").arg("-D").arg("warnings").arg("--no-deps")
+    };
+
+    let cargo_args = builder.build();
+
+    xtask::cargo::run_with_env(&cargo_args, &path, [("CI", "1")], false)?;
+
     Ok(())
 }
 
@@ -964,7 +810,7 @@ fn run_doc_tests(workspace: &Path, args: ExampleArgs) -> Result<()> {
 
     // Determine the appropriate build target, and cargo features for the given
     // package and chip:
-    let target = target_triple(args.package, &chip)?;
+    let target = args.package.target_triple(&chip)?;
     let features = vec![chip.to_string(), "unstable".to_string()];
 
     // We need `nightly` for building the doc tests, unfortunately: