| Message ID | 20231207052016.25954-1-zhi.mao@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: i2c: Add support for GC08A3 sensor | expand |
On 07/12/2023 06:20, Zhi Mao wrote: > This series adds YAML DT binding and V4L2 sub-device driver for Galaxycore's > GC08A3 8-megapixel 10-bit RAW CMOS 1/4" sensor, with an MIPI CSI-2 image data > interface and the I2C control bus. > > The driver is implemented with V4L2 framework. > - Async registered as a V4L2 sub-device. > - As the first component of camera system including Seninf, ISP pipeline. > - A media entity that provides one source pad in common. > - Used in camera features on ChromeOS application. > > Also this driver supports following features: > - manual exposure and analog gain control support > - vertical blanking control support > - test pattern support > - media controller support > - runtime PM support > - support resolution: 3264x2448@30fps, 1920x1080@60fps > > Previous versions of this patch-set can be found here: > v1: https://lore.kernel.org/linux-media/20231123115104.32094-1-zhi.mao@mediatek.com/ > > Changes of v2 mainly address comments from Krzysztof/Rob Herring&Conor Dooley. > Compared to v1: > - Fix some review comments What exactly fixed? This cannot be that vague! Best regards, Krzysztof
On Thu, Dec 07, 2023 at 09:19:01AM +0100, Krzysztof Kozlowski wrote: > On 07/12/2023 06:20, Zhi Mao wrote: > > This series adds YAML DT binding and V4L2 sub-device driver for Galaxycore's > > GC08A3 8-megapixel 10-bit RAW CMOS 1/4" sensor, with an MIPI CSI-2 image data > > interface and the I2C control bus. > > > > The driver is implemented with V4L2 framework. > > - Async registered as a V4L2 sub-device. > > - As the first component of camera system including Seninf, ISP pipeline. > > - A media entity that provides one source pad in common. > > - Used in camera features on ChromeOS application. > > > > Also this driver supports following features: > > - manual exposure and analog gain control support > > - vertical blanking control support > > - test pattern support > > - media controller support > > - runtime PM support > > - support resolution: 3264x2448@30fps, 1920x1080@60fps > > > > Previous versions of this patch-set can be found here: > > v1: https://lore.kernel.org/linux-media/20231123115104.32094-1-zhi.mao@mediatek.com/ > > > > Changes of v2 mainly address comments from Krzysztof/Rob Herring&Conor Dooley. > > Compared to v1: > > - Fix some review comments > > What exactly fixed? This cannot be that vague! Detailed changelogs are very useful for reviewers, and they should ideally be recorded for each patch, not just in the cover letter. It's not as difficult and time consuming as it sounds, here's how I usually handle it when working on a patch series (the explanation is meant more for Zhi Mao than Krzysztof :-)). When taking review comments into account, I will take the comments one by one, and update the code accordingly. I then compile-test the change, and apply it as a new 'fixup' commit: $ git commit -a --edit --fixup <id of the commit being fixed> In the editor, I type a single line to describe the change. The procedure is repeated for all review comments on all patches. When I'm done, I test the final result, and then rebase the branch to *squash* all the fixups with the original patch. git opens a text editor with all the commit messages of the fixups being concatenated after the commit message of the original patch. I edit that manually to format it as a changelog, but adding --- Changes since vX: manually, and follow with the one-line descriptions of all the changes. This is a fast process, it's much easier and faster to record a one-line description of each change as I go along than trying to write a changelog manually at the end, remembering all the changes I've made. Krzysztof, if you plan to make a talk about tooling for Linux kernel contributors, similar to your excellent talk at LPC about tooling for maintainers, this is something you could include :-)
Hi Zhi Mao, I have reviewed your v1 before noticing the v2, sorry about that. Most of the comments I made on v1 are still applicable, so I'll skip v2 and review v3. On Thu, Dec 07, 2023 at 01:20:14PM +0800, Zhi Mao wrote: > This series adds YAML DT binding and V4L2 sub-device driver for Galaxycore's > GC08A3 8-megapixel 10-bit RAW CMOS 1/4" sensor, with an MIPI CSI-2 image data > interface and the I2C control bus. > > The driver is implemented with V4L2 framework. > - Async registered as a V4L2 sub-device. > - As the first component of camera system including Seninf, ISP pipeline. > - A media entity that provides one source pad in common. > - Used in camera features on ChromeOS application. > > Also this driver supports following features: > - manual exposure and analog gain control support > - vertical blanking control support > - test pattern support > - media controller support > - runtime PM support > - support resolution: 3264x2448@30fps, 1920x1080@60fps > > Previous versions of this patch-set can be found here: > v1: https://lore.kernel.org/linux-media/20231123115104.32094-1-zhi.mao@mediatek.com/ > > Changes of v2 mainly address comments from Krzysztof/Rob Herring&Conor Dooley. > Compared to v1: > - Fix some review comments > - Add reviewed-by for sensor driver > - Fix some build-error and warning message > > Thanks > > > Zhi Mao (2): > media: i2c: Add GC08A3 image sensor driver > media: dt-bindings: media: i2c: Document GC08A3 bindings > > .../bindings/media/i2c/galaxycore,gc08a3.yaml | 127 ++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > drivers/media/i2c/Kconfig | 14 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/gc08a3.c | 1888 +++++++++++++++++ > 5 files changed, 2032 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml > create mode 100644 drivers/media/i2c/gc08a3.c