Re: [PATCH v3 10/39] media: platform: place Renesas drivers on a separate dir

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

Em Sun, 13 Mar 2022 14:34:39 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Sun, Mar 13, 2022 at 08:21:19AM +0100, Mauro Carvalho Chehab wrote:
> > In order to cleanup the main platform media directory, move Renesas
> > driver to its own directory.
> > 

...

> > +config VIDEO_RENESAS_FCP
> > +	tristate "Renesas Frame Compression Processor"
> > +	depends on V4L_MEM2MEM_DRIVERS
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	depends on OF
> > +	help
> > +	  This is a driver for the Renesas Frame Compression Processor (FCP).
> > +	  The FCP is a companion module of video processing modules in the
> > +	  Renesas R-Car Gen3 and RZ/G2 SoCs. It handles memory access for
> > +	  the codec, VSP and FDP modules.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called rcar-fcp.  
> 
> This should go before FDP1.

I'll do the reorder on the next version as proposed by you.

FYI, I opted to not sort the manufacturer's Kconfig files on this series
by purpose. After those patch series, the per-vendor Kconfig menus
have only a few drivers (up to ~10 config symbols). That makes it
a lot more manageable than what we had before. So, from my side, I don't
have any special requirements about Kconfig order.

On other words, the order inside each vendor-specific Kconfig
file should be a decision taken by the driver maintainers, which should
decide what fits better for them, considering that:

- the per-vendor driver order may not be in alphabetical order, as
  it may make sense to group drivers per type, platform and/or by
  its dependency chain;

- It is now easy to sort the per-vendor entries alphabetically,
  either per config title or per Kconfig symbol.

- Now that the main sort criteria is per-vendor, IMO it makes sense to
  have a per-vendor symbol space, e. g., instead of having:

	config VIDEO_RCAR_CSI2
	config VIDEO_RCAR_VIN
	config VIDEO_RCAR_ISP
	config VIDEO_RENESAS_CEU
	config VIDEO_SH_VOU
	config VIDEO_RENESAS_FCP
	config VIDEO_RENESAS_FDP1
	config VIDEO_RENESAS_JPU
	config VIDEO_RENESAS_VSP1
	config VIDEO_RCAR_DRIF

It could make sense to have all of them prepended with VIDEO_RENESAS:

	config VIDEO_RENESAS_CAR_CSI2
	config VIDEO_RENESAS_CAR_VIN
	config VIDEO_RENESAS_CAR_ISP
	config VIDEO_RENESAS_CEU
	config VIDEO_RENESAS_SH_VOU
	config VIDEO_RENESAS_FCP
	config VIDEO_RENESAS_FDP1
	config VIDEO_RENESAS_JPU
	config VIDEO_RENESAS_VSP1
	config VIDEO_RENESAS_CAR_DRIF

or some other variant of that, to produce smaller names, like using the
prefix adopted at the stock market (RNE, I guess): 

	config VIDEO_RNE_CAR_CSI2
	config VIDEO_RNE_CAR_VIN
	config VIDEO_RNE_CAR_ISP
	config VIDEO_RNE_CEU
	config VIDEO_RNE_SH_VOU
	config VIDEO_RNE_FCP
	config VIDEO_RNE_FDP1
	config VIDEO_RNE_JPU
	config VIDEO_RNE_VSP1
	config VIDEO_RNE_CAR_DRIF

With a change like that, a per Kconfig-symbol alphabetic order will change.

> 
> The FCP isn't a memory-to-memory device by the way, it's an ancillary
> device that handles compression of data transmitted on the bus between a
> DMA engine and memory. It's fine keeping it here, as it's related to the
> memory-to-memory devices.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks,
Mauro