From f65eca99791d91b4bc24bf25f6a0948806b5381c Mon Sep 17 00:00:00 2001 From: AI Dev Date: Fri, 6 Nov 2026 04:15:04 +0000 Subject: [PATCH 1/2] ambarella: detect the SoC from the machine string An Ambarella camera got "SoC not recognised": these kernels list no UART in /proc/iomem, and manufacturers[] had no row for what /proc/cpuinfo names (#266, an S3L in an Amcrest IP4M-1026E). Every machine in the vendor kernel is called "Ambarella (Flattened Device Tree)", so the name comes from there. S3L and S3LM share the string. The exact part is a word the DSP firmware writes, reachable only through /dev/iav, so the report stops at S3L. Sensor addresses are the defaults of the SDK's VIN drivers. AR0521 is left out: it shares 0x6C with OV4689, and the onsemi probe runs first. Total RAM stays what the kernel sees. IAVMEM in the boot log ends at 512 MiB on a camera reported as 256 MiB, and Amboot computes it from a compile-time DRAM_SIZE, so it proves nothing. The DRAM config register is not in the programming manual. Only the parser is tested, in ambarella_test, with strings from the issue and the kernel source. It has not run on a camera. --- .github/workflows/pr-build-check.yml | 4 +- CLAUDE.md | 10 +++++ CMakeLists.txt | 9 +++- src/chipid.c | 3 ++ src/chipid.h | 1 + src/hal/ambarella.c | 47 +++++++++++++++++++++ src/hal/ambarella.h | 11 +++++ src/hal/ambarella_test.c | 63 ++++++++++++++++++++++++++++ src/hal/common.h | 1 + 9 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 src/hal/ambarella.c create mode 100644 src/hal/ambarella.h create mode 100644 src/hal/ambarella_test.c diff --git a/.github/workflows/pr-build-check.yml b/.github/workflows/pr-build-check.yml index 309cc516..a5e14a8f 100644 --- a/.github/workflows/pr-build-check.yml +++ b/.github/workflows/pr-build-check.yml @@ -147,7 +147,8 @@ jobs: run: | cmake -H. -Bbuild -DCMAKE_BUILD_TYPE=Release cmake --build build --target cYAML_test reginfo_test longse_test \ - sensors_test anyka_test hal_hisi_test \ + sensors_test anyka_test ambarella_test \ + hal_hisi_test \ report_test http_test mtd_alias_test \ nand_layout_test upgrade_env_test \ jffs2_test raw_image_test @@ -162,6 +163,7 @@ jobs: ./build/reginfo_test ./build/longse_test ./build/anyka_test + ./build/ambarella_test ./build/hal_hisi_test # Run from the tree root: the fingerprint it compares against is # src/sensors_golden.txt, and a sensor that stopped being identified diff --git a/CLAUDE.md b/CLAUDE.md index 227dbcf9..40087f9a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -126,6 +126,8 @@ CMake knobs worth knowing: purpose. Same situation as `longse_test` and the same `CHECK` macro: nobody on the project has an Anyka camera, so this is all the verification that code gets until a reporter runs it. +- `./build/ambarella_test`: the `/proc/cpuinfo` machine string the Ambarella + HAL parses, same `CHECK` macro and same reason. - `tools/test_pipeline.sh`: hardware-free end-to-end check of the sensor driver extraction pipeline (`trace_segment.py` -> `trace_to_driver.py` -> `gcc -fsyntax-only` -> `trace_diff.py`), plus the `--selftest` of every @@ -213,6 +215,14 @@ with. Reading the register is therefore a refinement and never the gate, which is also what lets detection survive a kernel that will not hand over `/dev/mem`. +Ambarella (`src/hal/ambarella.c`) stops at the machine string on purpose. The +exact part (S3L33M, S3L55M, S3L66, ...) is a word the DSP firmware writes, +reachable only through `/dev/iav`, so the report says `S3L`. Do not derive +total RAM from the boot log's `IAVMEM` line: Amboot computes it from a +compile-time `DRAM_SIZE`, and #266 has it ending at 512 MiB on a camera +reported as 256 MiB. Dahua kernels also refuse `execve()` from writable +filesystems, so `/tmp/ipctool` fails there with "Success" and exit code 2. + Sensor I2C addresses in tables are the 8-bit (write) form; the default `i2c_change_addr` shifts right by one for the kernel, and some HALs install `i2c_change_plain_addr` instead. Ingenic gates the sensor clock, which is why diff --git a/CMakeLists.txt b/CMakeLists.txt index ac3ff5ce..5f591321 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,7 +51,7 @@ include_directories(./src/) # directly rather than through the table. Affects libipchw only; the ipctool # executable is a diagnostic tool and always carries every vendor. set(IPCHW_OPTIONAL_VENDORS - allwinner anyka bcm fh gm ingenic novatek rockchip sstar tegra xilinx xm) + allwinner ambarella anyka bcm fh gm ingenic novatek rockchip sstar tegra xilinx xm) set(IPCHW_VENDORS "all" CACHE STRING "Vendor HALs to compile into libipchw: 'all', 'none', or a ;-separated \ subset of: ${IPCHW_OPTIONAL_VENDORS}") @@ -525,6 +525,13 @@ if(NOT ONLY_LIBRARY) target_include_directories(anyka_test PRIVATE include src) target_link_libraries(anyka_test m Threads::Threads) + # Same reason as anyka_test: the parser is all that can run off a camera. + add_executable(ambarella_test src/hal/ambarella_test.c src/hal/ambarella.c + src/hal/common.c src/tools.c src/cjson/cJSON.c + ${VERSION_OBJ}) + target_include_directories(ambarella_test PRIVATE include src) + target_link_libraries(ambarella_test m Threads::Threads) + # The body `ipctool upload` sends: part framing, the backup's layout and # the length that must match it, and openipc.org's answer. Off a camera, # like the others. diff --git a/src/chipid.c b/src/chipid.c index f973d94b..e9ce82b9 100644 --- a/src/chipid.c +++ b/src/chipid.c @@ -74,6 +74,9 @@ static const manufacturers_t manufacturers[] = { #ifdef IPCHW_VENDOR_FH {"FH", fh_detect_cpu, VENDOR_FH, fh_setup_hal}, #endif +#ifdef IPCHW_VENDOR_AMBARELLA + {"Ambarella", ambarella_detect_cpu, VENDOR_AMBARELLA, ambarella_setup_hal}, +#endif #ifdef IPCHW_VENDOR_ANYKA {NULL /* Generic */, anyka_detect_cpu, VENDOR_ANYKA, anyka_setup_hal}, #endif diff --git a/src/chipid.h b/src/chipid.h index 8b008526..e1007e44 100644 --- a/src/chipid.h +++ b/src/chipid.h @@ -4,6 +4,7 @@ #include #define VENDOR_ALLWINNER "Allwinner" +#define VENDOR_AMBARELLA "Ambarella" #define VENDOR_ANYKA "Anyka" #define VENDOR_BCM "Broadcom" #define VENDOR_FH "Fullhan" diff --git a/src/hal/ambarella.c b/src/hal/ambarella.c new file mode 100644 index 00000000..1c715cbb --- /dev/null +++ b/src/hal/ambarella.c @@ -0,0 +1,47 @@ +#include "hal/ambarella.h" + +#include + +#include "hal/common.h" +#include "tools.h" + +/* Defaults of the `addr` module parameter across the S3L SDK's VIN drivers, + * the 8-bit form ipctool's tables use. AR0521 is the one part left out: it + * answers on 0x6C, and the onsemi probe runs before the OmniVision one, so + * listing it would have that probe reading an OV4689. */ +static unsigned char sony_addrs[] = {0x34, 0}; +static unsigned char onsemi_addrs[] = {0x20, 0}; +static unsigned char omni_addrs[] = {0x6c, 0}; + +static sensor_addr_t ambarella_possible_i2c_addrs[] = { + {SENSOR_SONY, sony_addrs}, + {SENSOR_ONSEMI, onsemi_addrs}, + {SENSOR_OMNIVISION, omni_addrs}, + {0, NULL}}; + +/* Every DT_MACHINE_START in the vendor tree reads "Ambarella (Flattened + * Device Tree)". These kernels register no UART with /proc/iomem, and S3L + * and S3LM share one machine string: the exact part (33M, 55M, 66, ...) is + * known only to the DSP firmware once it runs, so "S3L" is as far as this + * goes. */ +bool ambarella_machine_name(const char *cpuinfo, char *out, size_t outlen) { + if (!line_from_file(cpuinfo, "Hardware.+: Ambarella ([A-Za-z0-9]+)", out, + outlen)) + return false; + + out[outlen - 1] = '\0'; + return true; +} + +bool ambarella_detect_cpu(char *chip_name) { + char machine[128]; + + if (!ambarella_machine_name("/proc/cpuinfo", machine, sizeof(machine))) + return false; + strcpy(chip_name, machine); + return true; +} + +void ambarella_setup_hal() { + possible_i2c_addrs = ambarella_possible_i2c_addrs; +} diff --git a/src/hal/ambarella.h b/src/hal/ambarella.h new file mode 100644 index 00000000..963c5b8c --- /dev/null +++ b/src/hal/ambarella.h @@ -0,0 +1,11 @@ +#ifndef HAL_AMBARELLA_H +#define HAL_AMBARELLA_H + +#include +#include + +bool ambarella_machine_name(const char *cpuinfo, char *out, size_t outlen); +bool ambarella_detect_cpu(char *chip_name); +void ambarella_setup_hal(); + +#endif /* HAL_AMBARELLA_H */ diff --git a/src/hal/ambarella_test.c b/src/hal/ambarella_test.c new file mode 100644 index 00000000..f39bd205 --- /dev/null +++ b/src/hal/ambarella_test.c @@ -0,0 +1,63 @@ +/* Hardware-free cover for the Ambarella machine-string parser. The strings + * are the DT_MACHINE_START names in the vendor kernel, and the S3L cpuinfo is + * the one from issue #266. */ +#include +#include +#include +#include + +#include "hal/ambarella.h" + +int chip_generation; +char chip_name[128]; +const char *getchipfamily() { return chip_name; } + +static int failures; + +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, " FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ + } while (0) + +static bool parse(const char *cpuinfo, char *out, size_t outlen) { + char path[] = "/tmp/ambarella_test.XXXXXX"; + int fd = mkstemp(path); + + CHECK(fd >= 0); + CHECK(write(fd, cpuinfo, strlen(cpuinfo)) == (ssize_t)strlen(cpuinfo)); + close(fd); + bool res = ambarella_machine_name(path, out, outlen); + unlink(path); + return res; +} + +int main() { + char buf[128]; + + CHECK(parse("processor\t: 0\n" + "Hardware\t: Ambarella S3L (Flattened Device Tree)\n" + "Revision\t: 0000\n", + buf, sizeof(buf))); + CHECK(!strcmp(buf, "S3L")); + + CHECK(parse("Hardware\t: Ambarella S2L (Flattened Device Tree)\n", buf, + sizeof(buf))); + CHECK(!strcmp(buf, "S2L")); + + CHECK(parse("Hardware\t: Ambarella A5S (Flattened Device Tree)\n", buf, + sizeof(buf))); + CHECK(!strcmp(buf, "A5S")); + + CHECK(!parse("Hardware\t: SStar Soc (Flattened Device Tree)\n", buf, + sizeof(buf))); + CHECK(!parse("model name\t: ARMv7 Processor rev 1 (v7l)\n", buf, + sizeof(buf))); + + if (failures) + return EXIT_FAILURE; + puts("ambarella_test: all cases passed"); + return EXIT_SUCCESS; +} diff --git a/src/hal/common.h b/src/hal/common.h index 9dcb2e67..94c74a3c 100644 --- a/src/hal/common.h +++ b/src/hal/common.h @@ -8,6 +8,7 @@ #include "cjson/cJSON.h" #include "hal/allwinner.h" +#include "hal/ambarella.h" #include "hal/anyka.h" #include "hal/bcm.h" #include "hal/fh.h" From 22db123f1405310c52d1b8b80248e56117db4fa6 Mon Sep 17 00:00:00 2001 From: AI Dev Date: Fri, 6 Nov 2026 04:20:55 +0000 Subject: [PATCH 2/2] ambarella: say why chip_generation stays 0, refuse an empty buffer Review of #267. No caller passes a zero outlen, but the header exports the parser and the guard is one line. The comment records that leaving chip_generation at 0 is deliberate, since ipctool has no Ambarella tables. CLAUDE.md now pins the execve() note to the one kernel it was seen on rather than to Dahua in general. --- CLAUDE.md | 2 +- src/hal/ambarella.c | 7 +++++++ src/hal/ambarella_test.c | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 40087f9a..850db735 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -220,7 +220,7 @@ exact part (S3L33M, S3L55M, S3L66, ...) is a word the DSP firmware writes, reachable only through `/dev/iav`, so the report says `S3L`. Do not derive total RAM from the boot log's `IAVMEM` line: Amboot computes it from a compile-time `DRAM_SIZE`, and #266 has it ending at 512 MiB on a camera -reported as 256 MiB. Dahua kernels also refuse `execve()` from writable +reported as 256 MiB. That Dahua kernel also refuses `execve()` from writable filesystems, so `/tmp/ipctool` fails there with "Success" and exit code 2. Sensor I2C addresses in tables are the 8-bit (write) form; the default diff --git a/src/hal/ambarella.c b/src/hal/ambarella.c index 1c715cbb..c73113dc 100644 --- a/src/hal/ambarella.c +++ b/src/hal/ambarella.c @@ -25,6 +25,9 @@ static sensor_addr_t ambarella_possible_i2c_addrs[] = { * known only to the DSP firmware once it runs, so "S3L" is as far as this * goes. */ bool ambarella_machine_name(const char *cpuinfo, char *out, size_t outlen) { + if (!outlen) + return false; + if (!line_from_file(cpuinfo, "Hardware.+: Ambarella ([A-Za-z0-9]+)", out, outlen)) return false; @@ -39,6 +42,10 @@ bool ambarella_detect_cpu(char *chip_name) { if (!ambarella_machine_name("/proc/cpuinfo", machine, sizeof(machine))) return false; strcpy(chip_name, machine); + + /* chip_generation stays 0 on purpose. It keys the pad-mux, clock and boot + * ROM tables, and ipctool has none for Ambarella, so 0 sends those paths to + * their no-table answer and getchipfamily() back to the name. */ return true; } diff --git a/src/hal/ambarella_test.c b/src/hal/ambarella_test.c index f39bd205..46ca45e7 100644 --- a/src/hal/ambarella_test.c +++ b/src/hal/ambarella_test.c @@ -56,6 +56,12 @@ int main() { CHECK(!parse("model name\t: ARMv7 Processor rev 1 (v7l)\n", buf, sizeof(buf))); + /* Nothing may land in the buffer, or just before it. */ + char guarded[3] = {'x', 'y', 'z'}; + CHECK(!parse("Hardware\t: Ambarella S3L (Flattened Device Tree)\n", + guarded + 1, 0)); + CHECK(guarded[0] == 'x' && guarded[1] == 'y' && guarded[2] == 'z'); + if (failures) return EXIT_FAILURE; puts("ambarella_test: all cases passed");