Repository navigation
Conversation
The SIOCGIFCONF workaround is the same on the V100 and V200 generations, and the V100 branch carries a duplicate of it. Move the existing shim to fh_compat.c so both generations share one implementation before the V100 HAL is extracted.
Audio initialization failure stays fatal, as it is on every other platform. The watchdog has been disarmed on normal exits since 3f80a4a, so a bad microphone configuration no longer turns into a reboot loop and nothing here needs a platform-specific exception. A HAL for the Fullhan V100 generation (FH8852/FH8856, ARM1176/ARMv6), driving the vendor SDK's libdsp/libisp/libvmm and libacw_mpi through dlopen. Sensor drivers come from the userspace lib<name>.so modules that OpenIPC provides, the same mechanism as the V200 HAL. Identification follows ipctool: the PMU registers at 0xf0000000 report CHIP_ID 0x17092901 with IP_VER low nibble 0xc/0xd for FH8852V100 and FH8856V100, while 0x19112201 is the V200/V210 family. Keeping them apart matters because building a V100 board as V200 hangs the SoC in fh_pinctrl_init_devices() and corrupts the console baud. The platform lives in src/hal/fhan/ with an f1_ prefix so it mirrors the f2_ V200 HAL. The SDK has no public headers, so the interfaces were recovered from the vendor application and verified on hardware. Frames are copied once out of the uncached DMA buffer; the HEVC encoder bundles VPS, SPS and PPS into the IDR entry, so entries are split on start codes; H.264 is Main-only because the encoder rejects High and closes the channel; and there is no FH_VENC_DestroyChn, so channels are created once with both codecs reserved. The exported GPIO sysfs directory is GPIO<n> on these kernels, and the ISP reports an image gain that drives day/night switching without a light sensor. Extracted from #43, authored by Kasperi.
Move each platform's detection into its HAL (v4_system_identify, i6_system_readtemp, ...) and share the common probes from per-directory common.h headers. Move the register and chip-id constants out of hal_identify() and the HAL sources into the matching *_sys.h files, keep /sys/class/thermal as the generic temperature fallback, retry strict /dev/mem mappings a page at a time, and fall back to CLOCK_MONOTONIC when the toolchain has no CLOCK_MONOTONIC_COARSE.
PR Summary by QodoAdd Fullhan FH885xV100 support and modularize platform detection
AI Description
Diagram
High-Level Assessment
Files changed (68)
|
Code Review by Qodo
1. Night-mode changes can hang the service
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| else if (state != applied) { night_mode(state); applied = state; } | ||
| usleep(40000); | ||
| } | ||
| } else |
There was a problem hiding this comment.
1. Night-mode changes can hang the service 🐞 Bug ☼ Reliability
night_thread falls through to the no-sensor loop when F1 gain detection is unavailable, but that loop checks keepRunning rather than nightOn. With the documented default sensor pin of 999, an API reconfiguration calls night_disable(), sets nightOn to zero, and waits indefinitely for that loop to exit.
Agent Prompt
## Issue description
On F1 devices without gain detection, the newly selected fallback reaches the no-sensor loop, which cannot be stopped by `night_disable()`.
## Fix Focus Areas
- src/night.c[73-110]
## Recommended Fix
Make the no-sensor loop observe `nightOn` as well as `keepRunning`, so disabling or reconfiguring night mode releases the join.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| night_mode(night_mode_on()); | ||
|
|
||
| #if defined(__arm__) && !defined(__ARM_PCS_VFP) && __ARM_ARCH == 6 | ||
| if (plat == HAL_PLATFORM_F1 && f1_night_available()) { |
There was a problem hiding this comment.
4. Configured light sensors are ignored 🐞 Bug ≡ Correctness
night_thread selects F1 image-gain detection before checking app_config.adc_device. On a V100 device where gain detection is available, a configured ADC and its threshold never participate in day/night switching.
Agent Prompt
## Issue description
The F1 gain-detection branch overrides an explicitly configured ADC night sensor.
## Fix Focus Areas
- src/night.c[73-86]
## Recommended Fix
Use gain detection only when no explicit ADC or GPIO sensor is configured; preserve the existing configured-sensor paths.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+55
to
+57
| night_ircut(false); | ||
| night_irled((lamp & HAL_LIGHT_IR) != 0); | ||
| fh_lamp_enable(lamp); |
There was a problem hiding this comment.
5. Night lighting toggles an extra pin 🐞 Bug ≡ Correctness
night_mode calls night_irled() as well as fh_lamp_enable() on F1, so its IR setting drives both the generic configured GPIO and the Fullhan PWM lamp. When no generic LED pin is configured, the default value 999 is still passed to gpio_write(); when a pin is configured, it is toggled in addition to the selected Fullhan lamp.
Agent Prompt
## Issue description
F1 night-mode lamp control also writes the generic IR LED GPIO, including its unconfigured default.
## Fix Focus Areas
- src/night.c[40-59]
- src/hal/fhan/fh_lamp.c[15-21]
## Recommended Fix
Let the Fullhan lamp path control its hardware without calling `night_irled()` for an unconfigured or unrelated GPIO; retain coherent reported lamp state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| case HAL_PLATFORM_V3: ret = v3_pipeline_create(); break; | ||
| case HAL_PLATFORM_V4: ret = v4_pipeline_create(); break; | ||
| #if __ARM_ARCH == 6 | ||
| case HAL_PLATFORM_F1: ret = f1_pipeline_create(width, height, app_config.mirror, app_config.flip, framerate, app_config.antiflicker); break; |
There was a problem hiding this comment.
6. V100 ignores custom sensor tuning 🐞 Bug ≡ Correctness
sdk_start adds the F1 pipeline path but omits F1 from its later sensor_config loading dispatch. When an operator supplies a custom tuning file, f1_pipeline_create() can load its built-in search paths, but the configured file is never passed to the available f1_config_load() function.
Agent Prompt
## Issue description
The new F1 startup path never applies a configured `sensor_config` file despite providing an F1 tuning loader.
## Fix Focus Areas
- src/media.c[1029-1044]
- src/hal/fhan/f1_hal.c[291-324]
## Recommended Fix
Add F1 to the guarded post-start configuration dispatch and pass `app_config.sensor_config` to `f1_config_load()` when the configured file exists.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| case HAL_PLATFORM_V3: ret = v3_video_snapshot_grab(jpeg_index, jpeg); break; | ||
| case HAL_PLATFORM_V4: ret = v4_video_snapshot_grab(jpeg_index, jpeg); break; | ||
| #if __ARM_ARCH == 6 | ||
| case HAL_PLATFORM_F1: ret = f1_video_snapshot_grab(jpeg_index, jpeg); break; |
There was a problem hiding this comment.
7. V100 snapshots ignore request options 🐞 Bug ≡ Correctness
jpeg_get dispatches F1 snapshots using only the channel index and output buffer, leaving the request's dimensions and quality unused. Requests with different width, height, or qfactor values therefore capture from the same JPEG encoder configured once from application defaults.
Agent Prompt
## Issue description
F1 snapshot requests cannot affect the dimensions or quality supplied by the endpoint.
## Fix Focus Areas
- src/jpeg.c[117-141]
- src/hal/fhan/f1_hal.c[683-802]
- src/hal/fhan/f1_hal.c[834-947]
## Recommended Fix
Pass supported per-request settings into the F1 snapshot path and apply them before capture; explicitly reject settings the encoder cannot support rather than silently ignoring them.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+82
to
+86
| if (strstr(name, "V200")) | ||
| return HAL_PLATFORM_F2; | ||
| if (strstr(name, "V100") || !strncmp(name, "FH8852", 6) || | ||
| !strncmp(name, "FH8856", 6)) | ||
| return HAL_PLATFORM_F1; |
There was a problem hiding this comment.
2. Newer fullhan v210/v201 boards boot as v100 🐞 Bug ≡ Correctness
When the PMU register read fails or reports a package code missing from the table, the /proc/cpuinfo fallback in fh_identify returns F2 only if the name contains "V200". Otherwise it returns F1 for any name starting with FH8852/FH8856, so FH8852V210, FH8856V210 and FH8856V201 are treated as V100, and FH8858V210/FH8858V201 come back as an unknown platform. Before this PR, every "FH8" name mapped to F2, and the commit message says that running the wrong generation's HAL hangs the SoC.
Agent Prompt
## Issue description
The string fallback in fh_identify classifies FH8852V210/FH8856V210/FH8856V201 as V100 (F1) and FH8858V2x0 as unknown, because it only checks for the "V200" substring.
## Fix Focus Areas
- src/hal/fhan/common.h[80-87]
## Recommended Fix
Return HAL_PLATFORM_F1 only when the name contains "V100". Treat any other name containing V2 (V200/V201/V210), or any other FH8 name, as HAL_PLATFORM_F2, which matches the behaviour before this PR.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+303
to
+305
| offset = addr & ~((unsigned int)page - 1); | ||
| size = (unsigned int)page; | ||
| mapped_area = mmap64(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, mem_fd, offset); |
There was a problem hiding this comment.
3. A register read can crash after the page fallback 🐞 Bug ☼ Reliability
When the 64 KiB mmap fails with EPERM, hal_registry maps a single page and stores that page-aligned address in loaded_offset, but later calls still compare their 64 KiB-aligned offset against it. If the fallback page sits at a 64 KiB boundary, a later access elsewhere in the same 64 KiB window reuses the 4 KiB mapping and reads or writes past its end (SIGSEGV). If the page is not 64 KiB-aligned, every call instead unmaps and remaps.
Agent Prompt
## Issue description
After the single-page EPERM fallback, hal_registry can reuse a 4 KiB mapping for addresses outside it, because the reuse test compares only the 64 KiB-aligned offset.
## Fix Focus Areas
- src/hal/tools.c[276-323]
## Recommended Fix
Reuse the cached mapping only when `addr >= loaded_offset && addr + 4 <= loaded_offset + loaded_size`. Otherwise munmap and remap, trying the 64 KiB window first and the page fallback second. Use the same condition for the munmap check at the top of the function.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+77
| if (f1_aud_load(&f1_aud)) | ||
| HAL_WARNING("f1_hal", "Audio library unavailable, audio disabled\n"); |
There was a problem hiding this comment.
8. Missing audio symbol crashes audio startup 🐞 Bug ☼ Reliability
f1_aud_load returns failure after dlopen has set handle but before every function pointer is resolved, and f1_hal_init only logs a warning without unloading. f1_audio_init checks only f1_aud.handle, so a libacw_mpi missing, say, FH_AC_AI_MICIN_SetVol gets past that check and calls a NULL function pointer instead of returning an error.
Agent Prompt
## Issue description
A partial audio library load leaves f1_aud.handle set while some function pointers are NULL, and f1_audio_init then calls through them.
## Fix Focus Areas
- src/hal/fhan/f1_hal.c[76-77]
## Recommended Fix
When f1_aud_load fails, call f1_aud_unload(&f1_aud) before logging the warning, so that the handle check in f1_audio_init rejects audio cleanly.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+40
to
+42
| #if defined(__arm__) && !defined(__ARM_PCS_VFP) && __ARM_ARCH == 6 | ||
| if (plat == HAL_PLATFORM_F1) { | ||
| hal_light lamp = app_config.night_lamp; |
There was a problem hiding this comment.
9. Lamp setting is ignored on v200 fullhan 🐞 Bug ≡ Correctness
The new night_mode branch that honours app_config.night_lamp through fh_lamp_enable only runs when plat == HAL_PLATFORM_F1. FH885xV200 cameras still take the generic path, which never drives the PWM illuminators. The docs present lamp as a Fullhan setting, so V200 users who set white or ir,white get no change.
Agent Prompt
## Issue description
The `lamp` night-mode setting is only applied on HAL_PLATFORM_F1, even though it is documented as a Fullhan setting and fh_lamp_enable is shared with F2.
## Fix Focus Areas
- src/night.c[40-61]
- doc/config.md[31-31]
## Recommended Fix
Change the condition to `plat == HAL_PLATFORM_F1 || plat == HAL_PLATFORM_F2` so V200 also gets the lamp handling. If that is not intended, update the documentation to say the setting is V100-only.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+360
to
361
| &ini, "night_mode", "ir_sensor_pin", 0, 999, /* 999 = no sensor */ | ||
| &app_config.ir_sensor_pin); |
There was a problem hiding this comment.
10. Sensor pins above 127 read the wrong gpio 🐞 Bug ≡ Correctness
The ir_sensor_pin limit went from PIN_MAX (95) to 999 so that 999 can mean "no sensor", but every value from 96 to 998 is now accepted too. gpio_read takes a char pin, so a configured value above 127 is truncated and the night thread polls an unrelated or invalid line.
Agent Prompt
## Issue description
Widening the range to 999 makes pins 96-998 valid, and they get truncated to char later.
## Fix Focus Areas
- src/app_config.c[357-362]
## Recommended Fix
Parse into a temporary value. Accept it only if it is within 0..PIN_MAX or exactly 999, and otherwise keep the default of 999.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #43.
Integrated Fullhan FH885xV100 support extracted from #43, plus the shared Fullhan compatibility shim and the platform-identification refactor. The contributor's branch is left untouched.
All 14 canonical
build.shtargets pass, includingfhan-v1,fhan-v2,mips-musl, andak39xx.