Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imx290 : implement HCG/LCG mode #4953

Closed
wants to merge 1 commit into from

Conversation

gechdcb
Copy link

@gechdcb gechdcb commented Mar 24, 2022

  1. Allow switch the HCG/LCG mode at runtime via module parameter.
  2. Add an optional dtoverlay parameter to set the max gain as the maximum analogue gain of the imx290 and imx327 is slightly different

1. Allow switch the HCG/LCG mode at runtime via module parameter.
2. Add a optional dtoverlay parameter to set the max gain as the maximum analogue gain of the imx290 and imx327 is slightly different
@gechdcb gechdcb changed the title * imx290 : * imx290 : implement HCG/LCG mode Mar 24, 2022
@gechdcb gechdcb changed the title * imx290 : implement HCG/LCG mode imx290 : implement HCG/LCG mode Mar 24, 2022
@6by9
Copy link
Contributor

6by9 commented Mar 24, 2022

Thanks for the PR.

The changes adding HCG mode are fine, however as this is a mainline Linux driver, we do ask that all commits follow the Linux coding standards. There's an automatic coding style checker in scripts/checkpatch.pl. For media subsystem patches you need to run it with --strict.

No codespell typos will be found - file '/usr/share/codespell/dictionary.txt': No such file or directory
WARNING: Missing commit description - Add an appropriate one

ERROR: do not initialise statics to false
#25: FILE: drivers/media/i2c/imx290.c:71:
+static bool hcgmode = false;

CHECK: Alignment should match open parenthesis
#76: FILE: drivers/media/i2c/imx290.c:1348:
+		ret = fwnode_property_read_u8(dev_fwnode(dev), "max-gain",
+									&max_gain);

ERROR: Missing Signed-off-by: line(s)

Commit message title should start with media: i2c: imx290: , and needs the signed off by (use git commit -s as the simple way to add it).
One change per commit, so adding LCG/HCG control is separate from maximum gain.

The change to maximum gain is moot - IMX327 supports 29.4dB in 0.3dB steps for max 98, vs IMX290 supporting 30dB in 0.3dB steps for a max of 100.
Digital gain is on the same register, so 100 on IMX327 you'll get 29.4dB of analogue gain and 0.6dB of digital gain, which is close enough in my book.

If you really want to distinguish between the two, then do so by adding an extra compatible string and adjusting the gain range based on that, not by creating a non-standard (and un-documented) device tree property.

@6by9
Copy link
Contributor

6by9 commented Aug 27, 2024

Closing as 5.15 is obsolete. Superceded by #5859 on 6.6 as well.

@6by9 6by9 closed this Aug 27, 2024
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.

2 participants