Re: [PATCH v9 00/13] media: staging/imx7: add i.MX7 media driver
Hi Hans and Dan, On Fri 07 Dec 2018 at 13:38, Dan Carpenter wrote: On Fri, Dec 07, 2018 at 01:44:00PM +0100, Hans Verkuil wrote: CHECK: Alignment should match open parenthesis #936: FILE: drivers/staging/media/imx/imx7-mipi-csis.c:921: + ret = v4l2_async_register_fwnode_subdev(mipi_sd, + sizeof(struct v4l2_async_subdev), _port, 1, Apparently the latest coding style is that alignment is more important than line length, although I personally do not agree. But since you need to respin in any case due to the wrong SPDX identifier you used you might as well take this into account. I added this in the cover letter: - some alignment parenthesis that were left as they are, to be more readable this ones were left as they are as they seem impossible to fit all rules. I hope you really do not mind about this checks. if you have a strong opinion about this let me know, otherwise I will leave this as it is. I'm pretty sure it complains about both equally. If you make fix one warning it will complain about the other. So you just have to pick which warning to not care about. Yeah, I agree with you, I normally like to avoid at all cost the line length issue. --- Cheers, Rui regards, dan carpenter
Re: [PATCH v9 00/13] media: staging/imx7: add i.MX7 media driver
On Fri, Dec 07, 2018 at 01:44:00PM +0100, Hans Verkuil wrote: > CHECK: Alignment should match open parenthesis > #936: FILE: drivers/staging/media/imx/imx7-mipi-csis.c:921: > + ret = v4l2_async_register_fwnode_subdev(mipi_sd, > + sizeof(struct v4l2_async_subdev), _port, > 1, > > Apparently the latest coding style is that alignment is more important than > line length, although I personally do not agree. But since you need to > respin in any case due to the wrong SPDX identifier you used you might as > well take this into account. I'm pretty sure it complains about both equally. If you make fix one warning it will complain about the other. So you just have to pick which warning to not care about. regards, dan carpenter
Re: [PATCH v9 00/13] media: staging/imx7: add i.MX7 media driver
On 11/22/2018 04:18 PM, Rui Miguel Silva wrote:
> Hi,
> This series introduces the Media driver to work with the i.MX7 SoC. it uses
> the
> already existing imx media core drivers but since the i.MX7, contrary to
> i.MX5/6, do not have an IPU and because of that some changes in the imx media
> core are made along this series to make it support that case.
>
> This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along with several
> configurations changes for this to work as a capture subsystem. Some bugs are
> also fixed along the line. And necessary documentation.
>
> For a more detailed view of the capture paths, pads links in the i.MX7 please
> take a look at the documentation in PATCH 10.
>
> The system used to test and develop this was the Warp7 board with an OV2680
> sensor, which output format is 10-bit bayer. So, only MIPI interface was
> tested, a scenario with an parallel input would nice to have.
I got a few checkpatch warnings about coding style:
CHECK: Alignment should match open parenthesis
#953: FILE: drivers/staging/media/imx/imx7-media-csi.c:911:
+static struct v4l2_mbus_framefmt *imx7_csi_get_format(struct imx7_csi *csi,
+ struct v4l2_subdev_pad_config *cfg,
CHECK: Alignment should match open parenthesis
#1341: FILE: drivers/staging/media/imx/imx7-media-csi.c:1299:
+ ret = v4l2_async_register_fwnode_subdev(>sd,
+ sizeof(struct v4l2_async_subdev),
CHECK: Lines should not end with a '('
#684: FILE: drivers/staging/media/imx/imx7-mipi-csis.c:669:
+static struct csis_pix_format const *mipi_csis_try_format(
CHECK: Alignment should match open parenthesis
#708: FILE: drivers/staging/media/imx/imx7-mipi-csis.c:693:
+static struct v4l2_mbus_framefmt *mipi_csis_get_format(struct csi_state *state,
+ struct v4l2_subdev_pad_config *cfg,
CHECK: Alignment should match open parenthesis
#936: FILE: drivers/staging/media/imx/imx7-mipi-csis.c:921:
+ ret = v4l2_async_register_fwnode_subdev(mipi_sd,
+ sizeof(struct v4l2_async_subdev), _port, 1,
Apparently the latest coding style is that alignment is more important than
line length, although I personally do not agree. But since you need to
respin in any case due to the wrong SPDX identifier you used you might as
well take this into account.
I was really hoping I could merge this, but the SPDX license issue killed it.
Regards,
Hans
>
>
> Bellow goes an example of the output of the pads and links and the output of
> v4l2-compliance testing.
>
> The v4l-utils version used is:
> v4l2-compliance SHA : 044d5ab7b0d02683070d01a369c73d462d7a0cee from Nov 19th
>
> The Media Driver fail some tests but this failures are coming from code out of
> scope of this series (imx-capture), and some from the sensor OV2680
> but that I think not related with the sensor driver but with the testing and
> core.
>
> The csi and mipi-csi entities pass all compliance tests.
>
> Cheers,
> Rui
>
> v8->v9:
> Hans Verkuil:
> - Fix issues detected by checkpatch strict, still some left:
> - bigger kconfig option description
> - some alignement parenthesis that were left as they are, to be more
> readable
> - added new patch (PATCH13) for Maintainers update
> - SPDX in documentation rst file
> Sakari Ailus:
> - remove pad check in csi, this is done by core already
> - destroy mutex in probe error path (add label)
> - swap order in driver release
> - initialize endpoint in stack
> - use clk_bulk
> kbuild test robot:
> - add the missing imx-media-dev-common.c in patch 1/13
> - remove OWNER of module csis
> Myself:
> - add MAINTAINERS entries - new patch
>
> v7->v8:
> Myself:
> - rebase to latest linux-next (s/V4L2_MBUS_CSI2/V4L2_MBUS_CSI2_DPHY/)
> - Rebuild and test with latest v4l2-compliance
> - add Sakari reviewed-by tag to dt-bindings
>
> v6->v7:
> Myself:
> - Clock patches removed from this version since they were already merged
> - Rebuild and test with the latest v4l2-compliance
> - Add patch to video-mux regarding bayer formats
> - remove reference to dependent patch serie (was already merged)
>
> Sakari Ailus:
> - add port and endpoint explanantions
> - fix some wording should -> shall
>
> v5->v6:
> Rob Herring:
> - rename power-domain node name from: pgc-power-domain to power-domain
> - change mux-control-cells to 0
> - remove bus-width from mipi bindings and dts
> - remove err... regarding clock names line
> - remove clk-settle from example
> - split mipi-csi2 and csi bindings per file
> - add OF graph description to CSI
>
> Philipp Zabel:
> - rework group IDs and rename them with an _IPU_ prefix, this allowed to
> remove
>the ipu_present flag need.
>
> v4->v5:
> Sakari Ailus:
> - fix remove of the capture entries in dts bindings in the right patch
>
> Stephen Boyd:
> - Send all series to clk list
>
> v3->v4:
> Philipp Zabel:
> -
[PATCH v9 00/13] media: staging/imx7: add i.MX7 media driver
Hi, This series introduces the Media driver to work with the i.MX7 SoC. it uses the already existing imx media core drivers but since the i.MX7, contrary to i.MX5/6, do not have an IPU and because of that some changes in the imx media core are made along this series to make it support that case. This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along with several configurations changes for this to work as a capture subsystem. Some bugs are also fixed along the line. And necessary documentation. For a more detailed view of the capture paths, pads links in the i.MX7 please take a look at the documentation in PATCH 10. The system used to test and develop this was the Warp7 board with an OV2680 sensor, which output format is 10-bit bayer. So, only MIPI interface was tested, a scenario with an parallel input would nice to have. Bellow goes an example of the output of the pads and links and the output of v4l2-compliance testing. The v4l-utils version used is: v4l2-compliance SHA : 044d5ab7b0d02683070d01a369c73d462d7a0cee from Nov 19th The Media Driver fail some tests but this failures are coming from code out of scope of this series (imx-capture), and some from the sensor OV2680 but that I think not related with the sensor driver but with the testing and core. The csi and mipi-csi entities pass all compliance tests. Cheers, Rui v8->v9: Hans Verkuil: - Fix issues detected by checkpatch strict, still some left: - bigger kconfig option description - some alignement parenthesis that were left as they are, to be more readable - added new patch (PATCH13) for Maintainers update - SPDX in documentation rst file Sakari Ailus: - remove pad check in csi, this is done by core already - destroy mutex in probe error path (add label) - swap order in driver release - initialize endpoint in stack - use clk_bulk kbuild test robot: - add the missing imx-media-dev-common.c in patch 1/13 - remove OWNER of module csis Myself: - add MAINTAINERS entries - new patch v7->v8: Myself: - rebase to latest linux-next (s/V4L2_MBUS_CSI2/V4L2_MBUS_CSI2_DPHY/) - Rebuild and test with latest v4l2-compliance - add Sakari reviewed-by tag to dt-bindings v6->v7: Myself: - Clock patches removed from this version since they were already merged - Rebuild and test with the latest v4l2-compliance - Add patch to video-mux regarding bayer formats - remove reference to dependent patch serie (was already merged) Sakari Ailus: - add port and endpoint explanantions - fix some wording should -> shall v5->v6: Rob Herring: - rename power-domain node name from: pgc-power-domain to power-domain - change mux-control-cells to 0 - remove bus-width from mipi bindings and dts - remove err... regarding clock names line - remove clk-settle from example - split mipi-csi2 and csi bindings per file - add OF graph description to CSI Philipp Zabel: - rework group IDs and rename them with an _IPU_ prefix, this allowed to remove the ipu_present flag need. v4->v5: Sakari Ailus: - fix remove of the capture entries in dts bindings in the right patch Stephen Boyd: - Send all series to clk list v3->v4: Philipp Zabel: - refactor initialization code from media device probe to be possible to used from other modules - Remove index of csi from all accurrencs (dts, code, documentation) - Remove need for capture node for imx7 - fix pinctrl for ov2680 - add reviewed tag to add multiplexer controls patch Fabio Estevam: - remove always on from new regulator Randy Dunlap: - several text editing fixes in documentation Myself: - rebase on top of v4 of Steve series - change CSI probe to initialize imx media device - remove csi mux parallel endpoint from mux to avoid warning message v2->v3: Philipp Zabel: - use of_match_device in imx-media-dev instead of of_device_match - fix number of data lanes from 4 to 2 - change the clock definitions and use of mipi - move hs-settle from endpoint Rob Herring: - fix phy-supply description - add vendor properties - fix examples indentations Stephen Boyd: patch 3/14 - fix double sign-off - add fixes tag Dong Aisheng: patch 3/14 - fix double sign-off - add Acked-by tag Shawn Guo: patch 4/14 - remove line breakage in parent redifiniton - added Acked-by tag - dropped CMA area increase and add more verbose information in case of dma allocation failure patch 9/14 - remove extra line between cells and reg masks Myself: - rework on frame end in csi - add rxcount in csi driver - add power supplies to ov2680 node and fix gpio polarity v1->v2: Dan Carpenter: - fix return paths and codes; - fix clk_frequency validation and return code; - handle the csi remove (release resources that was missing) - revert the logic arround the ipu_present flag Philipp Zabel: - drop patch that changed the rgb formats and address the pixel/bus format in mipi_csis code. MySelf: - add patch that add ov2680 node to the warp7 dts, so the all data path is
