add mira220 image sensor

[fiepfiep]

add driver / device tree support for ams mira220

needed changes also done for libcamera.

[6by9]

I've started to review this, but I'm noticing a lot of my comments from #6552 haven't been addressed.

If embedded data isn't required, then do not include it.

[6by9]

Does this sensor actually have useful embedded metadata? It's disabled in libcamera.

I'd made the same comment in #6552 (comment) that this isn't how mainline are implementing metadata, so it's simpler to not implement it in the driver if not needed.

[fiepfiep]

I have removed the embedded data

[6by9]

You have about 18 registers that differ between these 3 huge tables. Break out all the common stuff into a common table, and only send the bits that change per mode

[fiepfiep]

thanks, will do.

[fiepfiep]

simplified, reduced driver to 1 mode only.

[pelwell]

I'll warn you now that, once the content of these commits has been approved, the end result should ideally be 3 commits - driver changes, config changes, and overlay/README changes.

[pelwell]

Did you delete this fragment by mistake? The media-controller parameter refers to it.

[fiepfiep]

Yes that is a mistake.

[fiepfiep]

I put it back. thanks for finding out.

[fiepfiep]

I'll warn you now that, once the content of these commits has been approved, the end result should ideally be 3 commits - driver changes, config changes, and overlay/README changes.

I will keep it in mind. Once all is approved by you I will refactor it to 3 commits.

[fiepfiep]

I cleaned up the driver a bit, implemented the new CCI_REG style writes, and took care of 6by9's comments.
Please let me know if you find other issues before we can do the merge.

[pelwell]

Apart from the extra blank lines in the README, the overlay looks good.

Checkpatch has lots of complaints about the driver which you should really fix. Nobody using printk directly anymore, for example.

The only additional change I would like (apart from refactoring down to 3 commits as discussed) is the addition of CONFIG_VIDEO_MIRA220 to the new PREEMPT_RT config file arm64/bcm2711_rt_defconfig.

[6by9]

Still a number of my previous review comments haven't been addressed.

• Please remove the references to the embedded data pad.
• Please remove all commented out code.
• Please remove all redundant comments.
• Please rationalise the logging messages. Very few should be needed at all.
• Regulator names should be consistently in lower case.

[pelwell]

FYI the build checks explicitly set CONFIG_WERROR=y, overriding the defconfigs:

scripts/config --file ${{github.workspace}}/build/.config --set-val CONFIG_WERROR y

[fiepfiep]

@pelwell could you run your checks again? I split my changes into 3 commits as requested.

[pelwell]

It looks like somebody has started them. Please mark the PR as "Ready for review" when you think it is. However, there is no excuse for some of these (https://github.com/raspberrypi/linux/actions/runs/14470568709/job/40583365956?pr=6717#step:6:80) checkpatch errors such as trailing whitespace.

[fiepfiep]

I still get this error in checkpatch, what does it mean?
@pelwell

 WARNING: From:/Signed-off-by: email name mismatch: 'From: philippe baetens <[email protected]>' != 'Signed-off-by: Philippe Baetens <[email protected]>'
Error: WARNING: From:/Signed-off-by: email name mismatch: 'From: philippe baetens <[email protected]>' != 'Signed-off-by: Philippe Baetens <[email protected]>'

[pelwell]

Is it possibly just that your name appears with and without uppercase letters? I'm not bothered about that, but it makes sense to fix it to avoid such errors in the future.

[fiepfiep]

Is it possibly just that your name appears with and without uppercase letters? I'm not bothered about that, but it makes sense to fix it to avoid such errors in the future.

that seems to be the case indeed. i modified the commit signoff message + some of the checkpatch remarks. Hope it is ok now.

[pelwell]

That's fixed it. I'll give these builds time to complete, in which time 6by9 may want to chip in, but I've been taking his silence for consent.

[6by9]

That's fixed it. I'll give these builds time to complete, in which time 6by9 may want to chip in, but I've been taking his silence for consent.

No, just hadn't had a chance to look through.
It looks like several of my previous comments haven't been addressed, eg the overlay override cam0 still setting VANA-supply and changing vana-supply.

[fiepfiep]

So - what are the next steps to get this driver into the raspberry linux kernel?
At the moment I'm only merging into the 6.12 kernel. What about older or newer kernel versions?
How does this driver propagate to official bookworm builds?

thanks.

[6by9]

In reading through to see if my previous comments had been addressed, add a number more.

There are a fair number of commented out blocks of code. They should be reviewed and removed where redundant.

[6by9]

So - what are the next steps to get this driver into the raspberry linux kernel? At the moment I'm only merging into the 6.12 kernel. What about older or newer kernel versions? How does this driver propagate to official bookworm builds?

Older kernels won't get updated.

Later kernels will get a forward port, as long as there aren't any significant build changes required by the every-changing kernel APIs. Simple changes are likely to get fixed up. Significant changes without obvious fixups will be reported back to you to fix - we don't undertake the full support burden of 3rd party drivers, and they may get dropped from the defconfigs if not fixed and/or get the BROKEN tag added to the Kconfig entry if the situation persists for a significant period.

rpi-update gets kernel updates normally several time a week.
Raspberry Pi OS gets updates every few months based on when significant things occur, providing no significant issues are known about at that point.

[pelwell]

Some of @6by9's comments have yet to be addressed, but the overlay and general structure look okay to me.

[fiepfiep]

Some of @6by9's comments have yet to be addressed, but the overlay and general structure look okay to me.

Thanks for reviewing. I believe I addressed all of @6by9's comments, but I might have missed something.. Can you be a bit more specific? thanks!

Also: each time I just 'force push' 3 new commits (overlay, conf, i2c driver) - not sure if this is your preferred way, or if you like to see my full commit history?

[pelwell]

On this page (https://github.com/raspberrypi/linux/pull/6717/files), scroll down and expand mira.c, then look at the comments that remain, e.g.: https://github.com/raspberrypi/linux/pull/6717/files#r2027459784 and https://github.com/raspberrypi/linux/pull/6717/files#r2027467385

[6by9]

There are still a number of remnants from the metadata pad kicking around. The things I've listed below would be nice to be cleaned up, but seeing as you'll be retaining responsibility for maintaining it, I'm not going to fuss. None of them should break the system.
You would be picked up on all these things if you try upstreaming the driver.

[fiepfiep]

i think the driver is now in good shape. requesting another review please..

[pelwell]

This still applies (both aspects):

The only additional change I would like (apart from refactoring down to 3 commits as discussed) is the addition of CONFIG_VIDEO_MIRA220 to the new PREEMPT_RT config file arm64/bcm2711_rt_defconfig.

[fiepfiep]

This still applies (both aspects):

The only additional change I would like (apart from refactoring down to 3 commits as discussed) is the addition of CONFIG_VIDEO_MIRA220 to the new PREEMPT_RT config file arm64/bcm2711_rt_defconfig.

okay, if all is good - I'll start refactoring down to 3 commits. I've added the rt_defconfig too.