Skip to content

hi3516cv100: a driver for the SAR ADC, where boards wire the photocell - #265

Merged
openipc-ai merged 2 commits into
mainfrom
cv100-sar-adc
Oct 15, 2026
Merged

openipc-ai merged 2 commits into
mainfrom
cv100-sar-adc

Conversation

@openipc-ai

Copy link
Copy Markdown
Contributor

For OpenIPC/firmware#2595: a Hi3518EV100 dome whose photoresistor is on SAR ADC channel 0. HiSilicon's V1 SDK ships no driver for the block, so OpenIPC could not read it.

What

kernel/adc/hi3516cv100/adc.c builds open_adc.ko (GPL, plain kernel APIs, no OSAL), wired into kernel/hi3516cv100.kbuild.

  • Register map from the Hi3518 Data Sheet (Issue 01, 2013-02-05, §13.8): base 0x200B0000, 10-bit, two channels with dedicated pins (no pinmux). Clock and reset are in PERI_CRG32 (0x20030080: bit1 cken, bit0 srst).
  • At load it pulses reset, enables the clock, powers the block up and masks its interrupt. IRQ 19 is shared with the IR receiver, so completion is polled from ADC_INT_RAW (1 ms timeout, then -ETIMEDOUT).
  • /dev/ADC_CH0 and /dev/ADC_CH1 are misc devices; the names match what vendor firmware used. A read() of at least 4 bytes runs one conversion and returns an int, 0..1023. Shorter reads get -EINVAL.
  • The first conversion out of reset reads 0 whatever is on the pin, so it is spent at load time.
  • At unload the block is powered down and put back in reset.

Tested on a hi3516cv100 (kernel 3.0.8, lite)

  • First proved the sequence by hand with devmem. At boot PERI_CRG32 reads 0x1 (in reset, clock gated).
  • Module built through the firmware (hi3516cv100_lite with this tree as the opensdk source), flashed, and loaded at boot by load_hisilicon.
  • Both channels convert; the lab board's floating pins read 3..22. 20,000 back-to-back reads with no timeout. rmmod returns PERI_CRG32 to 0x1.
  • End to end: majestic's ADC day/night source reads /dev/ADC_CH0 and switches night and day.

Not yet built against the neo (7.0) kernel. That is #264, and it matters because the firmware install step for cv100 is not variant-gated.

The firmware change (install as hi_adc.ko, insmod in load_hisilicon) follows once this is merged and the opensdk pin is bumped.

The Hi3518 V100 family has a 10-bit, two-channel SAR ADC (Hi3518 Data
Sheet, 13.8) that day/night boards hang a photoresistor on. HiSilicon's
V1 SDK ships no driver for it, so OpenIPC could not read one
(OpenIPC/firmware#2595, a Hi3518EV100 dome with its photocell on
channel 0).

open_adc.ko brings the block out of reset, clocks it from PERI_CRG32 and
exposes each channel as a misc device, /dev/ADC_CH0 and /dev/ADC_CH1 (the
names the vendor firmware used). A read() of at least four bytes runs one
conversion and returns it as an int, 0..1023. The conversion interrupt is
shared with the IR receiver, so completion is polled from the raw status.
The first conversion out of reset reads 0 whatever is on the pin, and is
spent at load time.

Measured on a hi3516cv100 (kernel 3.0.8): both channels convert, with
floating pins reading 3..22; 20000 reads with no timeout; unloading puts
the block back in reset.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add SAR ADC driver for hi3516cv100-family photocell inputs

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add a two-channel SAR ADC driver so day/night software can read photocells on V100 boards.
• Expose vendor-compatible device names and poll conversion completion without claiming the shared
 interrupt.
• Build the module for hi3516cv100 and document its availability.
Diagram

graph TD
  U["Day night app"] --> M["ADC_CH devices"] --> D["Conversion driver"] --> H["SAR ADC block"] --> Q{"Done?"} -->|yes| R["10-bit result"]
  Q -->|timeout| T["Timeout error"]
  C["CRG clock reset"] --> H
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use the kernel IIO subsystem
  • ➕ Provides a standard ADC interface for future consumers.
  • ➖ Does not directly preserve the vendor-compatible /dev/ADC_CH0 and /dev/ADC_CH1 read interface.
  • ➖ Adds integration and compatibility work across the target kernels.

Recommendation: Keep the small misc-device driver for the immediate vendor-compatible day/night use case. Consider IIO only if broader ADC consumers justify a standard interface; verify the module against the neo kernel before enabling that firmware variant.

Files changed (3) +196 / -1

Enhancement (1) +190 / -0
adc.cImplement the two-channel SAR ADC module +190/-0

Implement the two-channel SAR ADC module

• Adds vendor-named misc devices that return a 10-bit integer sample per read. The driver manages clock, reset, and power; serializes conversions; polls raw completion with a timeout; and discards the observed first zero reading after reset.

kernel/adc/hi3516cv100/adc.c

Documentation (1) +1 / -1
README.mdList ADC support for the V100 family +1/-1

List ADC support for the V100 family

• Marks open_adc as available in the first SoC-family column of the module support table.

README.md

Other (1) +5 / -0
hi3516cv100.kbuildBuild open_adc for hi3516cv100 +5/-0

Build open_adc for hi3516cv100

• Adds the SAR ADC source to the V100 module build and lists its device nodes in the source-module inventory.

kernel/hi3516cv100.kbuild

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Failed startup leaves ADC devices exposed ✓ Resolved
Description
adc_init() discards the return value from its initial adc_convert() call and registers both
devices even if that conversion times out. If the ADC does not complete a conversion at load time,
the module still reports successful initialization, leaving users to discover the failure on a later
read.
Code

kernel/adc/hi3516cv100/adc.c[155]

+	adc_convert(0, &discard);
Evidence
adc_convert() returns -ETIMEDOUT if completion never appears. Initialization ignores that
return, registers both devices, and returns success; subsequent reads call the same conversion
function and propagate its error.

kernel/adc/hi3516cv100/adc.c[59-76]
kernel/adc/hi3516cv100/adc.c[151-174]
kernel/adc/hi3516cv100/adc.c[94-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The startup conversion can return `-ETIMEDOUT`, but initialization ignores it and exposes both devices anyway.
## Fix Focus Areas
- kernel/adc/hi3516cv100/adc.c[151-159]
## Recommended Fix
Assign the initial conversion result to `ret`; on failure, branch to the existing `off` cleanup path rather than registering the misc devices.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread kernel/adc/hi3516cv100/adc.c Outdated
… (review)

The warm-up conversion at load time ignored its result, so a block that never completed a conversion still registered both nodes, and every read then timed out. Its failure now fails the load through the existing cleanup path.
@openipc-ai

Copy link
Copy Markdown
Contributor Author

Qodo finding addressed in 29b9190: the warm-up conversion's result is now checked, and a timeout fails the load (pr_err, then the existing off cleanup: power down, reset, iounmap) instead of registering nodes whose every read would time out. Rebuilt and re-tested on a hi3516cv100: it loads, both channels read (12..17 on floating pins), and rmmod returns PERI_CRG32 to 0x1.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Qodo finding addressed in 29b9190: the warm-up conversion's result is now checked, and a timeout fails the load (pr_err, then the existing off cleanup: power down, reset, iounmap) instead of registering nodes whose every read would time out. Rebuilt and re-tested on a hi3516cv100: it loads, both channels read (12..17 on floating pins), and rmmod returns PERI_CRG32 to 0x1.

Thanks for the update. finding 1 is addressed in commit 29b9190; the startup conversion timeout now aborts initialization and follows the existing cleanup path. The reported build, channel reads, and PERI_CRG32 reset behavior also cover the relevant validation.

@openipc-ai
openipc-ai merged commit 4cdb9ae into main Oct 15, 2026
35 checks passed
@openipc-ai
openipc-ai deleted the cv100-sar-adc branch October 15, 2026 11:41
openipc-ai added a commit to OpenIPC/firmware that referenced this pull request Oct 15, 2026
…pensdk 4cdb9ae (#2595)

Bumps hisilicon-opensdk to 4cdb9ae (OpenIPC/openhisilicon#265), which
builds open_adc.ko for the hi3516cv100 family: a driver for the SoC's
SAR ADC, where boards like the Hi3518EV100 GF_YTJ010E dome wire their
photoresistor. The bump carries nothing else.

Install it as hi_adc.ko and load it beside pwm.ko, so /dev/ADC_CH0 and
/dev/ADC_CH1 exist on every V1 camera. The module keeps its own name,
open_adc, which is what remove_ko has to give rmmod.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant