From 52012f5ea7a0c015b13b500616b406f4d118b94b Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:07:03 +1100 Subject: [PATCH] iio: aop: Decode the full lid angle report and expose its scale The LAS service already uses the same EPIC fake-HID receive path as ALS, but the decoder treats byte 1 of every report as an angle. Report 1 has a 9-bit little-endian angle; for example, 256 degrees becomes zero with the old decoder. Other report IDs can also overwrite the cached angle. Accept only complete report-1 samples in the 0..360 range and ignore the padding bits. Keep the raw count in degrees and expose pi/180 through in_angl_scale so IIO consumers can convert it to radians. A fixed scale does not need to wait for the first reading. Processed ALS channels keep their existing interface through the default no-scale implementation. Add host tests of the production decoder and IIO callback, document the existing J616s DT/config wiring and a hardware test plan, and register the new documentation and test under the existing AOP maintainer entry. Assisted-by: LLM Signed-off-by: Ryan Murray --- Documentation/iio/aop-sensors.rst | 65 ++++++++ Documentation/iio/index.rst | 1 + MAINTAINERS | 2 + drivers/iio/common/aop_sensors/aop_las.rs | 28 +++- rust/bindings/bindings_helper.h | 1 + rust/kernel/iio/common/aop_sensors.rs | 18 +++ .../testing/selftests/drivers/apple/Makefile | 3 +- .../selftests/drivers/apple/aop-las.py | 143 ++++++++++++++++++ 8 files changed, 256 insertions(+), 5 deletions(-) create mode 100644 Documentation/iio/aop-sensors.rst create mode 100755 tools/testing/selftests/drivers/apple/aop-las.py diff --git a/Documentation/iio/aop-sensors.rst b/Documentation/iio/aop-sensors.rst new file mode 100644 index 0000000000000..2292ad208350a --- /dev/null +++ b/Documentation/iio/aop-sensors.rst @@ -0,0 +1,65 @@ +.. SPDX-License-Identifier: GPL-2.0-only OR MIT + +================= +Apple AOP sensors +================= + +The AOP firmware announces sensor services over EPIC. The AOP core matches +each service to a child node in the device tree and forwards its fake-HID +reports to the corresponding IIO driver. Ambient light (``als``) and lid +angle (``las``) use this same transport; no HID bus device is created. + +Lid angle +========= + +Enable ``CONFIG_APPLE_AOP`` and ``CONFIG_IIO_AOP_SENSOR_LAS``. The AOP must +have a ``las`` child with a SoC-specific compatible followed by +``apple,aop-las``. The T6040 device tree already supplies this child for +J616s, and ``asahi.config`` enables the driver. The bootloader must hand +over the AOP firmware memory regions before the AOP can start. + +The device is named ``aop-sensors-las``. Its ``in_angl_raw`` attribute is +the latest valid report-1 angle, in whole degree counts from 0 to 360. +Multiply it by ``in_angl_scale`` (0.017453293) to obtain radians as +specified by the IIO angle ABI. The scale is fixed and can be read before +the first sample; a raw read before the first valid sample returns +``ENODATA``. Other report IDs, truncated reports and out-of-range angles +leave the last valid reading unchanged. + +Report 1 carries a 9-bit little-endian value following the report ID. +The remaining bits in the last byte are padding. Reads return the cached +unsolicited report, rather than issuing a synchronous HID feature request. + +Verification +============ + +Host regression tests require Python 3 and a Rust compiler:: + + tools/testing/selftests/drivers/apple/aop-las.py + +The tests compile the production decoder and shared IIO read callback +against host mocks. They cover all 16-bit payloads (including padding), +report IDs, short reports, invalid readings, the scale before a sample, +last-valid-sample behaviour and processed channels without a scale. +They do not test EPIC transport or firmware behaviour. + +On J616s, or another supported laptop: + +1. Build with ``asahi.config`` and the board's DTB. Check that the AOP, + LAS and ALS options survived configuration, and that the bootloader + supplies the AOP memory regions. +2. Boot and locate the IIO device whose ``name`` is ``aop-sensors-las``. + Check ``in_angl_scale`` is 0.017453293. Read ``in_angl_raw`` repeatedly + while moving the lid through several safe positions. Expect whole + degree counts that follow the hinge position. Do not force the hinge + beyond its mechanical limit to test values above 255; the host tests + cover that encoding. +3. Confirm the value updates after boot and after resume, and that ALS + still reports illuminance. Check the kernel log for AOP timeouts, + DART faults or warnings. Repeat on an older supported AOP platform + to check the shared receive path. +4. If raw reads keep returning ``ENODATA``, check that firmware announces + ``las`` and sends EPIC report subtype 0xc4 on that service's channel. + A readable feature report alone does not prove unsolicited reports + arrive. Establish that transport behaviour before adding any polling + or firmware setup request. diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst index b02b879b053a1..f7ccdaac25d7a 100644 --- a/Documentation/iio/index.rst +++ b/Documentation/iio/index.rst @@ -38,6 +38,7 @@ Industrial I/O Kernel Drivers adxl313 adxl380 adxl345 + aop-sensors bno055 ep93xx_adc opt4060 diff --git a/MAINTAINERS b/MAINTAINERS index a903e83bd0ecb..2250beb8efb91 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2111,6 +2111,7 @@ F: Documentation/devicetree/bindings/sound/apple,t8140-aop-audio.yaml F: Documentation/devicetree/bindings/usb/vialab,vl122.yaml F: Documentation/gpu/apple-dcp-backlight.rst F: Documentation/hid/apple-j700-multitouch.rst +F: Documentation/iio/aop-sensors.rst F: Documentation/networking/device_drivers/wifi/mt7932-j700-antenna.rst F: Documentation/networking/device_drivers/wifi/mt7932-neo.rst F: arch/arm64/boot/dts/apple/t8140* @@ -2136,6 +2137,7 @@ F: tools/testing/apple/ F: tools/testing/kunit/configs/apple_firmware.config F: tools/testing/kunit/configs/apple_t8140_bootstrap.config F: tools/testing/kunit/configs/mt7932.config +F: tools/testing/selftests/drivers/apple/aop-las.py F: tools/testing/selftests/drivers/gpu/apple-dcp/ APPLETALK NETWORK LAYER diff --git a/drivers/iio/common/aop_sensors/aop_las.rs b/drivers/iio/common/aop_sensors/aop_las.rs index 7e0f5ccc01b19..598c1b607a5d7 100644 --- a/drivers/iio/common/aop_sensors/aop_las.rs +++ b/drivers/iio/common/aop_sensors/aop_las.rs @@ -17,7 +17,21 @@ struct MsgProc; impl MessageProcessor for MsgProc { fn process(&self, message: &[u8]) -> Option { - message.get(1).map(|angle| u64::from(*angle) * MICRO) + // Report 1 contains a little-endian, 9-bit angle in degrees. + // Other report IDs carry sensor properties, not angle samples. + if *message.first()? != 1 { + return None; + } + let angle = u16::from_le_bytes([*message.get(1)?, *message.get(2)?]) & 0x1ff; + if angle > 360 { + return None; + } + Some(u64::from(angle) * MICRO) + } + + fn scale(&self) -> Option<(i32, i32)> { + // IIO_ANGL uses radians: one raw count is pi / 180 radians. + Some((0, 17_453_293)) } } @@ -59,9 +73,15 @@ impl platform::Driver for IIOAopLasDriver { let ty = bindings::BINDINGS_IIO_ANGL; let data = AopSensorData::new(dev.into(), ty, MsgProc)?; let listener = FakehidRegistration::new(adata, service, data.clone())?; - let info_mask = 1 << bindings::BINDINGS_IIO_CHAN_INFO_RAW; - let iio = - IIORegistration::::new(data, c"aop-sensors-las", ty, info_mask, kernel::module::this_module::())?; + let info_mask = (1 << bindings::BINDINGS_IIO_CHAN_INFO_RAW) + | (1 << bindings::BINDINGS_IIO_CHAN_INFO_SCALE); + let iio = IIORegistration::::new( + data, + c"aop-sensors-las", + ty, + info_mask, + kernel::module::this_module::(), + )?; Ok(IIOAopLasDriver { listener, _iio: iio, diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index ff4f810297ba4..9e384a40cd494 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -163,6 +163,7 @@ const snd_ctl_elem_iface_t BINDINGS_SNDRV_CTL_ELEM_IFACE_MIXER = SNDRV_CTL_ELEM_ const u32 BINDINGS_IIO_CHAN_INFO_RAW = IIO_CHAN_INFO_RAW; const u32 BINDINGS_IIO_CHAN_INFO_PROCESSED = IIO_CHAN_INFO_PROCESSED; +const u32 BINDINGS_IIO_CHAN_INFO_SCALE = IIO_CHAN_INFO_SCALE; const u32 BINDINGS_IIO_ANGL = IIO_ANGL; const u32 BINDINGS_IIO_LIGHT = IIO_LIGHT; diff --git a/rust/kernel/iio/common/aop_sensors.rs b/rust/kernel/iio/common/aop_sensors.rs index 7c80cef56b92f..94da1c11b1fa6 100644 --- a/rust/kernel/iio/common/aop_sensors.rs +++ b/rust/kernel/iio/common/aop_sensors.rs @@ -41,6 +41,12 @@ pub trait MessageProcessor: Send + Sync { /// Returns the reading carried by `message` in micro-units of the channel (millionths of a /// lux, of a degree, ...), or `None` when the message carries no reading. fn process(&self, message: &[u8]) -> Option; + + /// Returns the fixed scale of a raw channel as an integer and a fractional part in + /// billionths, or `None` for a channel without a scale attribute. + fn scale(&self) -> Option<(i32, i32)> { + None + } } /// The state shared between a sensor's report listener and its IIO device: the latest reading. @@ -99,6 +105,18 @@ unsafe extern "C" fn aop_read_raw( if data.ty != ty { return EINVAL.to_errno(); } + // A fixed scale is available even before the first sensor report. + if mask == bindings::BINDINGS_IIO_CHAN_INFO_SCALE as isize { + let Some((int, frac)) = data.msg_proc.scale() else { + return EINVAL.to_errno(); + }; + // SAFETY: `val` and `val2` point to the caller's result variables. + unsafe { + *val = int; + *val2 = frac; + } + return bindings::IIO_VAL_INT_PLUS_NANO as i32; + } // A sensor that has not reported yet, for instance because it lacks its // calibration, has no reading rather than a reading of zero. if !data.valid.load(Acquire) { diff --git a/tools/testing/selftests/drivers/apple/Makefile b/tools/testing/selftests/drivers/apple/Makefile index 932df4d386328..0408fc701dfa0 100644 --- a/tools/testing/selftests/drivers/apple/Makefile +++ b/tools/testing/selftests/drivers/apple/Makefile @@ -5,9 +5,10 @@ # dwc3-initial-role.py checks initial host-role admission before core reset. # sn201202x-transport.py checks the production SPMI selection and payload path. # bcm4377 tests exercise DMA ownership, context addresses and firmware handshakes. +# aop-las.py checks the production angle decoder and IIO callback on the host. TEST_PROGS := macsmc-j713.py atc-t8132-usb3.py pmgr-inherited.py dwc3-initial-role.py \ sn201202x-transport.py bcm4377-dma-window.py bcm4377-radio-sids.py \ - bcm4377-state-wait.py bcm4377-rti-address.py + bcm4377-state-wait.py bcm4377-rti-address.py aop-las.py TEST_FILES := j813-peripherals.sh include ../../lib.mk diff --git a/tools/testing/selftests/drivers/apple/aop-las.py b/tools/testing/selftests/drivers/apple/aop-las.py new file mode 100755 index 0000000000000..ce4da0dcec63d --- /dev/null +++ b/tools/testing/selftests/drivers/apple/aop-las.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only OR MIT +"""Run the production LAS decoder and IIO read callback with host Rust mocks.""" +import os +from pathlib import Path +import shlex +import subprocess +import tempfile + + +def block(source, marker): + start = source.index(marker) + brace = source.index("{", start) + depth = 1 + end = brace + 1 + while depth: + depth += (source[end] == "{") - (source[end] == "}") + end += 1 + return source[start:end] + + +def main(): + root = Path(__file__).resolve().parents[5] + common = (root / "rust/kernel/iio/common/aop_sensors.rs").read_text() + las = (root / "drivers/iio/common/aop_sensors/aop_las.rs").read_text() + parts = [r''' +#![allow(non_camel_case_types)] +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering::{Acquire, Relaxed, Release}}; +const MICRO: u64 = 1_000_000; +const MAX_MICRO: u64 = i32::MAX as u64 * MICRO + MICRO - 1; +#[derive(Debug)] +struct Error(i32); +impl Error { fn to_errno(&self) -> i32 { self.0 } } +const EINVAL: Error = Error(-22); +const ENODATA: Error = Error(-61); +type Result = std::result::Result; +mod bindings { + pub const BINDINGS_IIO_CHAN_INFO_RAW: u32 = 0; + pub const BINDINGS_IIO_CHAN_INFO_PROCESSED: u32 = 1; + pub const BINDINGS_IIO_CHAN_INFO_SCALE: u32 = 2; + pub const IIO_VAL_INT: u32 = 1; + pub const IIO_VAL_INT_PLUS_MICRO: u32 = 2; + pub const IIO_VAL_INT_PLUS_NANO: u32 = 3; + pub struct iio_dev { pub priv_: *const std::ffi::c_void } + pub struct iio_chan_spec { pub type_: u32 } +} +struct Arc(std::marker::PhantomData); +impl Arc { + unsafe fn borrow<'a>(p: *const std::ffi::c_void) -> &'a T { + unsafe { &*p.cast::() } + } +} +trait FakehidListener { + fn process_fakehid_report(&self, data: &[u8]) -> Result<()>; +} +struct AopSensorData { + ty: u32, value: AtomicU64, valid: AtomicBool, msg_proc: T, +} +struct MsgProc; +''', block(common, "pub trait MessageProcessor"), + block(las, "impl MessageProcessor for MsgProc"), + block(common, "impl FakehidListener"), + block(common, 'unsafe extern "C" fn aop_read_raw'), r''' +fn sensor(msg_proc: T) -> AopSensorData { + AopSensorData { + ty: 17, value: AtomicU64::new(0), valid: AtomicBool::new(false), msg_proc, + } +} +fn read(data: &AopSensorData, mask: u32) -> (i32, i32, i32) { + let mut dev = bindings::iio_dev { priv_: (data as *const AopSensorData).cast() }; + let chan = bindings::iio_chan_spec { type_: data.ty }; + let (mut val, mut val2) = (-1, -1); + let ret = unsafe { aop_read_raw::(&mut dev, &chan, &mut val, &mut val2, mask as isize) }; + (ret, val, val2) +} +#[test] +fn full_angle_and_padding() { + // Synthetic reports: cover bit 8, both limits, invalid values and every padding pattern. + for wire in 0..=u16::MAX { + let bytes = wire.to_le_bytes(); + let angle = wire & 0x1ff; + let expected = if angle <= 360 { Some(u64::from(angle) * MICRO) } else { None }; + assert_eq!(MsgProc.process(&[1, bytes[0], bytes[1]]), expected); + } + assert_eq!(MsgProc.process(&[1, 99, 0]), Some(99 * MICRO)); + assert_eq!(MsgProc.process(&[1, 0, 1]), Some(256 * MICRO)); + assert_eq!(MsgProc.process(&[1, 104, 1]), Some(360 * MICRO)); +} +#[test] +fn report_identity_and_truncation() { + for id in 0..=u8::MAX { + if id != 1 { assert_eq!(MsgProc.process(&[id, 99, 0]), None); } + } + for truncated in [&[][..], &[1][..], &[1, 99][..]] { + assert_eq!(MsgProc.process(truncated), None); + } +} +#[test] +fn raw_scale_and_last_valid_sample() { + use bindings::*; + let data = sensor(MsgProc); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_RAW).0, -61); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_SCALE), + (IIO_VAL_INT_PLUS_NANO as i32, 0, 17_453_293)); + data.process_fakehid_report(&[2, 39]).unwrap(); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_RAW).0, -61); + data.process_fakehid_report(&[1, 99, 0]).unwrap(); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_RAW), (1, 99, 0)); + for report in [&[3, 0, 0, 0, 0, 1][..], &[1, 105, 1][..], &[1, 42][..]] { + data.process_fakehid_report(report).unwrap(); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_RAW), (1, 99, 0)); + } + let (_, int, nano) = read(&data, BINDINGS_IIO_CHAN_INFO_SCALE); + let radians = 99.0 * (int as f64 + nano as f64 / 1e9); + assert!((radians - 99.0_f64.to_radians()).abs() < 1e-7); +} +struct Processed; +impl MessageProcessor for Processed { + fn process(&self, _: &[u8]) -> Option { Some(12_345_678) } +} +#[test] +fn processed_channel_unchanged() { + use bindings::*; + let data = sensor(Processed); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_PROCESSED).0, -61); + data.process_fakehid_report(&[]).unwrap(); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_PROCESSED), (2, 12, 345_678)); + assert_eq!(read(&data, BINDINGS_IIO_CHAN_INFO_SCALE).0, -22); + assert_eq!(read(&data, 999).0, -22); +} +'''] + with tempfile.TemporaryDirectory(prefix="aop-las-") as tmp: + source = Path(tmp) / "test.rs" + binary = Path(tmp) / "test" + source.write_text("\n".join(parts)) + subprocess.run(shlex.split(os.environ.get("RUSTC", "rustc")) + [ + "--edition=2021", "--test", "-Dwarnings", str(source), "-o", str(binary), + ], check=True) + subprocess.run([str(binary)], check=True) + + +if __name__ == "__main__": + main()