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

[Mauro Carvalho Chehab <mchehab@kernel.org>]

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