Re: [PATCH v5 0/2] media: Add Himax HM1092 mono NIR sensor driver

[Ramshouriesh R]

Hi Jake,

Honestly, thank you for this. I'd much rather end up with one good driver than
have the two of us shipping HM1092 support in parallel. I went through all three
of your points against my own numbers and you're right on every one of them.

On the link frequency: yeah, it doesn't add up, and I don't even need your board
to see it. My own registers contradict it. 1616 x 750 x 10 / 2 is 181.8 MHz at
30 fps and 363.6 at 60, and 400 MHz is neither. The pixel_rate I derive from it
works out to about 66 fps, which isn't a mode this part even has. So it's just
wrong.

The one thing I don't want to do is blindly paste your number in, because my
EXTCLK is 24 MHz, not 19.2, so your 19.2 x 94 / 5 = 360.96 doesn't land on my
board the same way. What I'll do instead is actually pin down the link rate here
(measured fps against hts x vts, plus the PLL dividers for a 24 MHz input) and
put whatever falls out of that in the cover letter (or commit message), with the
working shown. My guess is it ends up a lot closer to your 180.48 than to 400,
but I'd rather measure it than assert it. You already ate the three months on
this one, so I can at least not guess.

And you nailed why it streams fine for me anyway. My CSIPHY gets its config from
the PHY driver and DT, not from this control, so a bad LINK_FREQ is completely
invisible on my setup and totally fatal on a receiver that actually reads it.
That's exactly the kind of thing that shouldn't reach the tree, so I'm glad you
flagged it.

On exposure and gain: fair hit, I'll fix the cover. Saying the map is "unknown"
is just wrong now that you've posted it, and it lines up with what I already
have anyway. I write 0x0202/0x0203 and 0x0104 in my init table (0x0203 is 0xbe
for me vs your 0xb4, same register, different default), which matches your
exposure and group-hold registers. I'd really like that patch on top of my
series. Like you said they need testing against my 560x360 mode first, so let's
just do it in the open: send them over, I'll run them here and tell you straight
whether they work. A driver that can't do AE/AGC can't do the actual job this
sensor exists for, so that gap matters a lot more to me than any of the cosmetic
stuff.

Geometry: you're right, and I can see where I went wrong. Those get_selectio
numbers came off the binned readout-window registers, so I was describing the
crop window, not the pixel array. Reporting a native size (1198x726) that's
narrower than both the 1280x720 active array and the 1296x736 mode in the
Windows tables makes no sense, exactly like you said. I'll redo them against
real array geometry so the selection targets actually describe the sensor
instead of my one crop.

On the bigger picture, your offer is clearly the better outcome, so I'll tak
it. Keep your ipu-bridge HIMX1092 patch, it's orthogonal and I've got nothing
like it, and I'll fold the three fixes in. Getting this onto two x86/IPU7 bo
with the sensor wired differently on each is worth a lot for a series
that's only
been proven on one ARM DT board so far, and I'd be glad to carry a Tested-by
each once it runs there. And if it doesn't run, tell me, that's just as useful.

On provenance, since you were upfront about yours: mine came out of
com.qti.sensormodule.hm1092.bin, the Qualcomm sensor-module blob in the ASUS
Windows camera stack (QualcommBSP/qccamauxsensor_extension8380),
regSetting block
at offset 0x9e8f. That's a different vendor artifact from your hm1092.sys, s
between the two of us the table's now been pulled from two independent Windows
sources and they agree, which is about as good as this kind of thing gets. A
maintainer taking either should know where it came from.

So from my end: I'll respin with the link-freq derivation, fixed geometry, and
the cover corrected on exposure/gain, then sort out the gain/exposure patch
getting it onto your boards with you. If you'd sooner send your findings as
standalone patches instead, that's fine too. I just think folding them in is
cleaner.

Anyway, thanks for doing this out in the open instead of just landing
a competing
driver. Better result for the part, and for whoever has to debug it next.

Cheers,
Ramshouriesh