| Message ID | Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL,FOR,v6.3] NXP i.MX8 ISI driver | expand |
On Fri, Feb 3, 2023 at 4:03 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Mauro, > > The following changes since commit 7120d6bfd6d0b26b49958f429701996f2d3e2c2a: > > media: tm6000: remove deprecated driver (2023-01-22 09:57:19 +0100) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git tags/media-imx-isi-next-20230203 > > for you to fetch changes up to e8126b9f0ee306e784dfa20f5390b97d573986ae: > > media: nxp: Add i.MX8 ISI driver (2023-02-03 11:15:18 +0200) > Please forgive my ignorance. I've been trying to follow this, but I am not sure where this goes after the merge request is complete. Can someone point me to the right repo? thank you, adam > This is a new driver for the NXP i.MX8 ISI, found in multiple i.MX8 SoCs > including the i.MX8MP (which I have used for testing) and i.MX8MN. The > driver uses the V4L2 streams API that you have merged in the media > staging tree, so I've based the pull request on the latest master branch > of that tree. > > As the streams API is going to land in v6.3, I think it would be nice to > also have one user of the API in the same kernel version. Note that the > API isn't exposed to userspace by default, doing so requires flipping a > variable in v4l2-subdev.c, so we'll have a few kernel releases to test > and stabilize everything with multiple drivers (not that I expect > issues, as we've extensively tested that API over the course of multiple > years in at least 6 different drivers - which we'll work on upstreaming > of course, some of them have already been posted for review). > > ---------------------------------------------------------------- > NXP i.MX8 ISI driver > > ---------------------------------------------------------------- > Laurent Pinchart (2): > dt-bindings: media: Add i.MX8 ISI DT bindings > media: nxp: Add i.MX8 ISI driver > > .../devicetree/bindings/media/nxp,imx8-isi.yaml | 173 +++ > MAINTAINERS | 7 + > drivers/media/platform/nxp/Kconfig | 2 + > drivers/media/platform/nxp/Makefile | 1 + > drivers/media/platform/nxp/imx8-isi/Kconfig | 22 + > drivers/media/platform/nxp/imx8-isi/Makefile | 8 + > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 645 +++++++++ > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 395 +++++ > .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 529 +++++++ > .../media/platform/nxp/imx8-isi/imx8-isi-debug.c | 109 ++ > drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c | 651 +++++++++ > drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c | 858 +++++++++++ > .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 867 +++++++++++ > .../media/platform/nxp/imx8-isi/imx8-isi-regs.h | 418 ++++++ > .../media/platform/nxp/imx8-isi/imx8-isi-video.c | 1512 ++++++++++++++++++++ > 15 files changed, 6197 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > create mode 100644 drivers/media/platform/nxp/imx8-isi/Kconfig > create mode 100644 drivers/media/platform/nxp/imx8-isi/Makefile > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c > > -- > Regards, > > Laurent Pinchart
Hi Adam, On Mon, Feb 27, 2023 at 07:44:24AM -0600, Adam Ford wrote: > On Fri, Feb 3, 2023 at 4:03 AM Laurent Pinchart wrote: > > > > Hi Mauro, > > > > The following changes since commit 7120d6bfd6d0b26b49958f429701996f2d3e2c2a: > > > > media: tm6000: remove deprecated driver (2023-01-22 09:57:19 +0100) > > > > are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git tags/media-imx-isi-next-20230203 > > > > for you to fetch changes up to e8126b9f0ee306e784dfa20f5390b97d573986ae: > > > > media: nxp: Add i.MX8 ISI driver (2023-02-03 11:15:18 +0200) > > Please forgive my ignorance. I've been trying to follow this, but I > am not sure where this goes after the merge request is complete. > Can someone point me to the right repo? Patches first get merged to the master branch of [1], then of [2]. [1] git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git [2] git://linuxtv.org/media_tree.git > > This is a new driver for the NXP i.MX8 ISI, found in multiple i.MX8 SoCs > > including the i.MX8MP (which I have used for testing) and i.MX8MN. The > > driver uses the V4L2 streams API that you have merged in the media > > staging tree, so I've based the pull request on the latest master branch > > of that tree. > > > > As the streams API is going to land in v6.3, I think it would be nice to > > also have one user of the API in the same kernel version. Note that the > > API isn't exposed to userspace by default, doing so requires flipping a > > variable in v4l2-subdev.c, so we'll have a few kernel releases to test > > and stabilize everything with multiple drivers (not that I expect > > issues, as we've extensively tested that API over the course of multiple > > years in at least 6 different drivers - which we'll work on upstreaming > > of course, some of them have already been posted for review). > > > > ---------------------------------------------------------------- > > NXP i.MX8 ISI driver > > > > ---------------------------------------------------------------- > > Laurent Pinchart (2): > > dt-bindings: media: Add i.MX8 ISI DT bindings > > media: nxp: Add i.MX8 ISI driver > > > > .../devicetree/bindings/media/nxp,imx8-isi.yaml | 173 +++ > > MAINTAINERS | 7 + > > drivers/media/platform/nxp/Kconfig | 2 + > > drivers/media/platform/nxp/Makefile | 1 + > > drivers/media/platform/nxp/imx8-isi/Kconfig | 22 + > > drivers/media/platform/nxp/imx8-isi/Makefile | 8 + > > .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 645 +++++++++ > > .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 395 +++++ > > .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 529 +++++++ > > .../media/platform/nxp/imx8-isi/imx8-isi-debug.c | 109 ++ > > drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c | 651 +++++++++ > > drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c | 858 +++++++++++ > > .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 867 +++++++++++ > > .../media/platform/nxp/imx8-isi/imx8-isi-regs.h | 418 ++++++ > > .../media/platform/nxp/imx8-isi/imx8-isi-video.c | 1512 ++++++++++++++++++++ > > 15 files changed, 6197 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml > > create mode 100644 drivers/media/platform/nxp/imx8-isi/Kconfig > > create mode 100644 drivers/media/platform/nxp/imx8-isi/Makefile > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h > > create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
From: builder@linuxtv.org
Pull request: https://patchwork.linuxtv.org/project/linux-media/patch/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com/
Build log: https://builder.linuxtv.org/job/patchwork/290881/
Build time: 00:18:56
Link: https://lore.kernel.org/linux-media/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com
gpg: Signature made Fri 03 Feb 2023 09:46:05 AM UTC
gpg: using EDDSA key C09EF871B3827B413F971CA9CC3F2D800327DE64
gpg: issuer "laurent.pinchart@ideasonboard.com"
gpg: Can't check signature: No public key
Summary: got 2/2 patches with issues, being 1 at build time, plus one error when buinding PDF document
Error/warnings:
patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch:
allyesconfig: return code #0:
../scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
../scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
../scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
../drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:415 ov2680_s_stream() warn: missing error code 'ret'
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3357 atomisp_cp_dvs_6axis_config() warn: missing unwind goto?
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3456 atomisp_cp_morph_table() warn: missing unwind goto?
allyesconfig: return code #0:
../drivers/media/i2c/adp1653.c: ../drivers/media/i2c/adp1653.c:444 adp1653_of_init() warn: missing unwind goto?
../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: ../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:2485 mxc_jpeg_probe() warn: missing unwind goto?
SMATCH:../drivers/media/usb/siano/smsusb.c ../drivers/media/usb/siano/smsusb.c:53:38: :warning: array of flexible structures
../drivers/media/i2c/ov5645.c: ../drivers/media/i2c/ov5645.c:687 ov5645_set_power_on() warn: 'ov5645->xclk' from clk_prepare_enable() not released on lines: 687.
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: OOM: 3000032Kb sm_state_count = 1939314
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() warn: Function too hairy. No more merges.
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: __split_smt: function too hairy. Giving up after 52 seconds
../drivers/media/usb/pvrusb2/pvrusb2-hdw.c: ../drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3293 pvr2_hdw_get_tuner_status() warn: inconsistent indenting
../drivers/media/usb/em28xx/em28xx-video.c: ../drivers/media/usb/em28xx/em28xx-video.c:2878 em28xx_v4l2_init() parse error: turning off implications after 60 seconds
checkpatch.pl:
$ cat patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:20: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch:
checkpatch.pl:
$ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:98: WARNING: please write a help paragraph that fully describes the config symbol
-:112: WARNING: please write a help paragraph that fully describes the config symbol
-:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/
-:1128: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1178: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
Error #512 when building PDF docs
From: builder@linuxtv.org
Pull request: https://patchwork.linuxtv.org/project/linux-media/patch/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com/
Build log: https://builder.linuxtv.org/job/patchwork/290886/
Build time: 00:18:12
Link: https://lore.kernel.org/linux-media/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com
gpg: Signature made Fri 03 Feb 2023 09:46:05 AM UTC
gpg: using EDDSA key C09EF871B3827B413F971CA9CC3F2D800327DE64
gpg: issuer "laurent.pinchart@ideasonboard.com"
gpg: Can't check signature: No public key
Summary: got 2/2 patches with issues, being 1 at build time
Error/warnings:
patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch:
allyesconfig: return code #0:
../scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
../scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
../scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
../drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:415 ov2680_s_stream() warn: missing error code 'ret'
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3357 atomisp_cp_dvs_6axis_config() warn: missing unwind goto?
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3456 atomisp_cp_morph_table() warn: missing unwind goto?
allyesconfig: return code #0:
../drivers/media/i2c/adp1653.c: ../drivers/media/i2c/adp1653.c:444 adp1653_of_init() warn: missing unwind goto?
../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: ../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:2485 mxc_jpeg_probe() warn: missing unwind goto?
SMATCH:../drivers/media/usb/siano/smsusb.c ../drivers/media/usb/siano/smsusb.c:53:38: :warning: array of flexible structures
../drivers/media/i2c/ov5645.c: ../drivers/media/i2c/ov5645.c:687 ov5645_set_power_on() warn: 'ov5645->xclk' from clk_prepare_enable() not released on lines: 687.
../drivers/media/i2c/ov5670.c: ../drivers/media/i2c/ov5670.c:2670 ov5670_probe() warn: passing zero to 'PTR_ERR'
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: OOM: 3000024Kb sm_state_count = 1939314
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() warn: Function too hairy. No more merges.
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: __split_smt: function too hairy. Giving up after 53 seconds
../drivers/media/usb/pvrusb2/pvrusb2-hdw.c: ../drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3293 pvr2_hdw_get_tuner_status() warn: inconsistent indenting
../drivers/media/usb/em28xx/em28xx-video.c: ../drivers/media/usb/em28xx/em28xx-video.c:2878 em28xx_v4l2_init() parse error: turning off implications after 60 seconds
checkpatch.pl:
$ cat patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:20: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch:
checkpatch.pl:
$ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:98: WARNING: please write a help paragraph that fully describes the config symbol
-:112: WARNING: please write a help paragraph that fully describes the config symbol
-:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/
-:1128: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1178: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
Hi Laurent, Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins <jenkins@linuxtv.org> escreveu: > checkpatch.pl: > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > -:98: WARNING: please write a help paragraph that fully describes the config symbol > -:112: WARNING: please write a help paragraph that fully describes the config symbol > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst There are still a couple of checkpatch issues there. Please check. In special: - either DT file is wrong or patch 2. Patch 1 declares: properties: compatible: enum: - fsl,imx8mn-isi - fsl,imx8mp-isi But patch 2 has an additional one: static const struct of_device_id mxc_isi_of_match[] = { { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, { /* sentinel */ }, - Instead of udelay(300, could it use, instead usleep_range()? - There are two nitpick checkpatch warnings related to blank lines. Thanks, Mauro
Hi Mauro, On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > checkpatch.pl: > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > There are still a couple of checkpatch issues there. Please check. > In special: > > - either DT file is wrong or patch 2. Patch 1 declares: > > properties: > compatible: > enum: > - fsl,imx8mn-isi > - fsl,imx8mp-isi > > But patch 2 has an additional one: > > static const struct of_device_id mxc_isi_of_match[] = { > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > { /* sentinel */ }, Indeed, that should be fixed. Would you mind if I addressed this on top ? I will be travelling next week, which would delay a new pull request to v6.3-rc5. That's still within the allowed time frame, but if I get delayed a bit more, I'm worried about missing the v6.4 merge window. I will actively maintain the ISI driver and will fix this for v6.5 at the latest if I can't make it for v6.4. It shouldn't cause any regression for supported DT bindings, and the "fsl,imx8-isi" compatible string isn't listed in the bindings and won't be used in device tree sources. > - Instead of udelay(300, could it use, instead usleep_range()? This could be fixed on top too. > - There are two nitpick checkpatch warnings related to blank lines. I've seen those two and ignored them as the current code matches the coding style used in many places for such inline function stubs.
Em Thu, 23 Mar 2023 01:33:52 +0200 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > Hi Mauro, > > On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > > > checkpatch.pl: > > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > > > There are still a couple of checkpatch issues there. Please check. > > In special: > > > > - either DT file is wrong or patch 2. Patch 1 declares: > > > > properties: > > compatible: > > enum: > > - fsl,imx8mn-isi > > - fsl,imx8mp-isi > > > > But patch 2 has an additional one: > > > > static const struct of_device_id mxc_isi_of_match[] = { > > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > > { /* sentinel */ }, > > Indeed, that should be fixed. Would you mind if I addressed this on top > ? I will be travelling next week, which would delay a new pull request > to v6.3-rc5. That's still within the allowed time frame, but if I get > delayed a bit more, I'm worried about missing the v6.4 merge window. > > I will actively maintain the ISI driver and will fix this for v6.5 at > the latest if I can't make it for v6.4. It shouldn't cause any > regression for supported DT bindings, and the "fsl,imx8-isi" compatible > string isn't listed in the bindings and won't be used in device tree > sources. > > > - Instead of udelay(300, could it use, instead usleep_range()? > > This could be fixed on top too. Sure. Feel free to add patches on the top. Regards, Mauro > > > - There are two nitpick checkpatch warnings related to blank lines. > > I've seen those two and ignored them as the current code matches the > coding style used in many places for such inline function stubs. > Thanks, Mauro
Hi Mauro, On Thu, Mar 23, 2023 at 05:24:30PM +0100, Mauro Carvalho Chehab wrote: > Em Thu, 23 Mar 2023 01:33:52 +0200 Laurent Pinchart escreveu: > > On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > > > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > > > > > checkpatch.pl: > > > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > > > > > There are still a couple of checkpatch issues there. Please check. > > > In special: > > > > > > - either DT file is wrong or patch 2. Patch 1 declares: > > > > > > properties: > > > compatible: > > > enum: > > > - fsl,imx8mn-isi > > > - fsl,imx8mp-isi > > > > > > But patch 2 has an additional one: > > > > > > static const struct of_device_id mxc_isi_of_match[] = { > > > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > > > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > > > { /* sentinel */ }, > > > > Indeed, that should be fixed. Would you mind if I addressed this on top > > ? I will be travelling next week, which would delay a new pull request > > to v6.3-rc5. That's still within the allowed time frame, but if I get > > delayed a bit more, I'm worried about missing the v6.4 merge window. > > > > I will actively maintain the ISI driver and will fix this for v6.5 at > > the latest if I can't make it for v6.4. It shouldn't cause any > > regression for supported DT bindings, and the "fsl,imx8-isi" compatible > > string isn't listed in the bindings and won't be used in device tree > > sources. > > > > > - Instead of udelay(300, could it use, instead usleep_range()? > > > > This could be fixed on top too. > > Sure. Feel free to add patches on the top. Thanks, I will do so. Just to make sure, does that mean you will merge this pull request in the stage tree in the near future ? Or do you want to wait for the additional patches ? > > > - There are two nitpick checkpatch warnings related to blank lines. > > > > I've seen those two and ignored them as the current code matches the > > coding style used in many places for such inline function stubs.
Em Thu, 23 Mar 2023 17:24:30 +0100 Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > Em Thu, 23 Mar 2023 01:33:52 +0200 > Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > > > Hi Mauro, > > > > On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > > > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > > > > > checkpatch.pl: > > > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > > > > > There are still a couple of checkpatch issues there. Please check. > > > In special: > > > > > > - either DT file is wrong or patch 2. Patch 1 declares: > > > > > > properties: > > > compatible: > > > enum: > > > - fsl,imx8mn-isi > > > - fsl,imx8mp-isi > > > > > > But patch 2 has an additional one: > > > > > > static const struct of_device_id mxc_isi_of_match[] = { > > > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > > > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > > > { /* sentinel */ }, > > > > Indeed, that should be fixed. Would you mind if I addressed this on top > > ? I will be travelling next week, which would delay a new pull request > > to v6.3-rc5. That's still within the allowed time frame, but if I get > > delayed a bit more, I'm worried about missing the v6.4 merge window. > > > > I will actively maintain the ISI driver and will fix this for v6.5 at > > the latest if I can't make it for v6.4. It shouldn't cause any > > regression for supported DT bindings, and the "fsl,imx8-isi" compatible > > string isn't listed in the bindings and won't be used in device tree > > sources. > > > > > - Instead of udelay(300, could it use, instead usleep_range()? > > > > This could be fixed on top too. > > Sure. Feel free to add patches on the top. Jus re-checked the PR. DT is still broken there: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ #894: FILE: drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c:624: + { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, total: 0 errors, 5 warnings, 6215 lines checked Please fix. Regards, Mauro
Hi Mauro, On Sat, Apr 15, 2023 at 11:05:56AM +0100, Mauro Carvalho Chehab wrote: > Em Thu, 23 Mar 2023 17:24:30 +0100 Mauro Carvalho Chehab escreveu: > > Em Thu, 23 Mar 2023 01:33:52 +0200 Laurent Pinchart escreveu: > > > On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > > > > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > > > > > > > checkpatch.pl: > > > > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > > > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > > > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > > > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > > > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > > > > > > > There are still a couple of checkpatch issues there. Please check. > > > > In special: > > > > > > > > - either DT file is wrong or patch 2. Patch 1 declares: > > > > > > > > properties: > > > > compatible: > > > > enum: > > > > - fsl,imx8mn-isi > > > > - fsl,imx8mp-isi > > > > > > > > But patch 2 has an additional one: > > > > > > > > static const struct of_device_id mxc_isi_of_match[] = { > > > > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > > > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > > > > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > > > > { /* sentinel */ }, > > > > > > Indeed, that should be fixed. Would you mind if I addressed this on top > > > ? I will be travelling next week, which would delay a new pull request > > > to v6.3-rc5. That's still within the allowed time frame, but if I get > > > delayed a bit more, I'm worried about missing the v6.4 merge window. > > > > > > I will actively maintain the ISI driver and will fix this for v6.5 at > > > the latest if I can't make it for v6.4. It shouldn't cause any > > > regression for supported DT bindings, and the "fsl,imx8-isi" compatible > > > string isn't listed in the bindings and won't be used in device tree > > > sources. > > > > > > > - Instead of udelay(300, could it use, instead usleep_range()? > > > > > > This could be fixed on top too. > > > > Sure. Feel free to add patches on the top. > > Jus re-checked the PR. DT is still broken there: > > WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > #894: FILE: drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c:624: > + { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > total: 0 errors, 5 warnings, 6215 lines checked > > Please fix. You told me it was fine to fix this on top. I have patches to do so.
From: builder@linuxtv.org
Pull request: https://patchwork.linuxtv.org/project/linux-media/patch/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com/
Build log: https://builder.linuxtv.org/job/patchwork/297889/
Build time: 00:26:01
Link: https://lore.kernel.org/linux-media/Y9zZLJobSYuMwP9o@pendragon.ideasonboard.com
gpg: Signature made Fri 03 Feb 2023 09:46:05 AM UTC
gpg: using EDDSA key C09EF871B3827B413F971CA9CC3F2D800327DE64
gpg: issuer "laurent.pinchart@ideasonboard.com"
gpg: Can't check signature: No public key
Summary: got 2/2 patches with issues, being 1 at build time, plus one error when buinding PDF document
Error/warnings:
patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch:
allyesconfig: return code #0:
../scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
../scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
../scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
../drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:416 ov2680_s_stream() warn: missing error code 'ret'
../drivers/staging/media/atomisp/i2c/atomisp-gc0310.c:212 gc0310_s_stream() warn: missing error code 'ret'
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3013 atomisp_cp_dvs_6axis_config() warn: missing unwind goto?
../drivers/staging/media/atomisp/pci/atomisp_cmd.c: ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:3112 atomisp_cp_morph_table() warn: missing unwind goto?
allyesconfig: return code #0:
../drivers/media/i2c/adp1653.c: ../drivers/media/i2c/adp1653.c:444 adp1653_of_init() warn: missing unwind goto?
../drivers/media/i2c/ov5645.c: ../drivers/media/i2c/ov5645.c:687 ov5645_set_power_on() warn: 'ov5645->xclk' from clk_prepare_enable() not released on lines: 687.
../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: ../drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:2775 mxc_jpeg_probe() warn: missing unwind goto?
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: OOM: 3000020Kb sm_state_count = 1974725
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() warn: Function too hairy. No more merges.
../drivers/media/pci/cx23885/cx23885-dvb.c: ../drivers/media/pci/cx23885/cx23885-dvb.c:2570 dvb_register() parse error: __split_smt: function too hairy. Giving up after 53 seconds
SMATCH:../drivers/media/usb/siano/smsusb.c ../drivers/media/usb/siano/smsusb.c:53:38: :warning: array of flexible structures
../drivers/media/usb/em28xx/em28xx-video.c: ../drivers/media/usb/em28xx/em28xx-video.c:2864 em28xx_v4l2_init() parse error: turning off implications after 60 seconds
../drivers/media/usb/pvrusb2/pvrusb2-hdw.c: ../drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3293 pvr2_hdw_get_tuner_status() warn: inconsistent indenting
checkpatch.pl:
$ cat patches/0001-dt-bindings-media-Add-i.MX8-ISI-DT-bindings.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:20: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch:
checkpatch.pl:
$ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict
-:98: WARNING: please write a help paragraph that fully describes the config symbol
-:112: WARNING: please write a help paragraph that fully describes the config symbol
-:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/
-:1128: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1178: CHECK: Please use a blank line after function/struct/union/enum declarations
-:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
Error #512 when building PDF docs
Em Sat, 15 Apr 2023 13:20:17 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > Hi Mauro, > > On Sat, Apr 15, 2023 at 11:05:56AM +0100, Mauro Carvalho Chehab wrote: > > Em Thu, 23 Mar 2023 17:24:30 +0100 Mauro Carvalho Chehab escreveu: > > > Em Thu, 23 Mar 2023 01:33:52 +0200 Laurent Pinchart escreveu: > > > > On Sun, Mar 19, 2023 at 10:08:21PM +0100, Mauro Carvalho Chehab wrote: > > > > > Em Sun, 19 Mar 2023 02:34:23 +0000 Jenkins escreveu: > > > > > > > > > > > checkpatch.pl: > > > > > > $ cat patches/0002-media-nxp-Add-i.MX8-ISI-driver.patch | formail -c | ./scripts/checkpatch.pl --terse --mailback --no-summary --strict > > > > > > -:98: WARNING: please write a help paragraph that fully describes the config symbol > > > > > > -:112: WARNING: please write a help paragraph that fully describes the config symbol > > > > > > -:761: WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > > > > > -:1128: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > > > -:1178: CHECK: Please use a blank line after function/struct/union/enum declarations > > > > > > -:1928: CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst > > > > > > > > > > There are still a couple of checkpatch issues there. Please check. > > > > > In special: > > > > > > > > > > - either DT file is wrong or patch 2. Patch 1 declares: > > > > > > > > > > properties: > > > > > compatible: > > > > > enum: > > > > > - fsl,imx8mn-isi > > > > > - fsl,imx8mp-isi > > > > > > > > > > But patch 2 has an additional one: > > > > > > > > > > static const struct of_device_id mxc_isi_of_match[] = { > > > > > { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > > > > { .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data }, > > > > > { .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data }, > > > > > { /* sentinel */ }, > > > > > > > > Indeed, that should be fixed. Would you mind if I addressed this on top > > > > ? I will be travelling next week, which would delay a new pull request > > > > to v6.3-rc5. That's still within the allowed time frame, but if I get > > > > delayed a bit more, I'm worried about missing the v6.4 merge window. > > > > > > > > I will actively maintain the ISI driver and will fix this for v6.5 at > > > > the latest if I can't make it for v6.4. It shouldn't cause any > > > > regression for supported DT bindings, and the "fsl,imx8-isi" compatible > > > > string isn't listed in the bindings and won't be used in device tree > > > > sources. > > > > > > > > > - Instead of udelay(300, could it use, instead usleep_range()? > > > > > > > > This could be fixed on top too. > > > > > > Sure. Feel free to add patches on the top. > > > > Jus re-checked the PR. DT is still broken there: > > > > WARNING: DT compatible string "fsl,imx8-isi" appears un-documented -- check ./Documentation/devicetree/bindings/ > > #894: FILE: drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c:624: > > + { .compatible = "fsl,imx8-isi", .data = &mxc_imx8_data_v1 }, > > > > total: 0 errors, 5 warnings, 6215 lines checked > > > > Please fix. > > You told me it was fine to fix this on top. I have patches to do so. Those patches aren't at the branch. Please add them to the branch you want me to pull and send another PR to trigger Jenkins to test it. Regards, Mauro
Hi Mauro, The following changes since commit 7120d6bfd6d0b26b49958f429701996f2d3e2c2a: media: tm6000: remove deprecated driver (2023-01-22 09:57:19 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git tags/media-imx-isi-next-20230203 for you to fetch changes up to e8126b9f0ee306e784dfa20f5390b97d573986ae: media: nxp: Add i.MX8 ISI driver (2023-02-03 11:15:18 +0200) This is a new driver for the NXP i.MX8 ISI, found in multiple i.MX8 SoCs including the i.MX8MP (which I have used for testing) and i.MX8MN. The driver uses the V4L2 streams API that you have merged in the media staging tree, so I've based the pull request on the latest master branch of that tree. As the streams API is going to land in v6.3, I think it would be nice to also have one user of the API in the same kernel version. Note that the API isn't exposed to userspace by default, doing so requires flipping a variable in v4l2-subdev.c, so we'll have a few kernel releases to test and stabilize everything with multiple drivers (not that I expect issues, as we've extensively tested that API over the course of multiple years in at least 6 different drivers - which we'll work on upstreaming of course, some of them have already been posted for review). ---------------------------------------------------------------- NXP i.MX8 ISI driver ---------------------------------------------------------------- Laurent Pinchart (2): dt-bindings: media: Add i.MX8 ISI DT bindings media: nxp: Add i.MX8 ISI driver .../devicetree/bindings/media/nxp,imx8-isi.yaml | 173 +++ MAINTAINERS | 7 + drivers/media/platform/nxp/Kconfig | 2 + drivers/media/platform/nxp/Makefile | 1 + drivers/media/platform/nxp/imx8-isi/Kconfig | 22 + drivers/media/platform/nxp/imx8-isi/Makefile | 8 + .../media/platform/nxp/imx8-isi/imx8-isi-core.c | 645 +++++++++ .../media/platform/nxp/imx8-isi/imx8-isi-core.h | 395 +++++ .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 529 +++++++ .../media/platform/nxp/imx8-isi/imx8-isi-debug.c | 109 ++ drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c | 651 +++++++++ drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c | 858 +++++++++++ .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 867 +++++++++++ .../media/platform/nxp/imx8-isi/imx8-isi-regs.h | 418 ++++++ .../media/platform/nxp/imx8-isi/imx8-isi-video.c | 1512 ++++++++++++++++++++ 15 files changed, 6197 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml create mode 100644 drivers/media/platform/nxp/imx8-isi/Kconfig create mode 100644 drivers/media/platform/nxp/imx8-isi/Makefile create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c