[v2,0/2] media: i2c: Add support for GC08A3 sensor

  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
  

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