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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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.

-- 
Regards,

Laurent Pinchart