Re: [GIT PULL FOR v6.3] NXP i.MX8 ISI driver (#89568)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux