Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Frank Rowand" <frowand.list@gmail.com>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	"Richard Sproul" <sproul@cadence.com>,
	"Alan Douglas" <adouglas@cadence.com>,
	"Steve Creaney" <screaney@cadence.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Boris Brezillon" <boris.brezillon@bootlin.com>,
	"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
	"Hans Verkuil" <hans.verkuil@cisco.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Benoit Parrot" <bparrot@ti.com>,
	nm@ti.com, "Simon Hatliff" <hatliff@cadence.com>
Subject: Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver
Date: Tue, 17 Apr 2018 17:59:47 +0200	[thread overview]
Message-ID: <20180417155947.5hv74hbpmtfzv32l@flea> (raw)
In-Reply-To: <20180417132010.zhr3nrigeqzeorg3@valkosipuli.retiisi.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

On Tue, Apr 17, 2018 at 04:20:10PM +0300, Sakari Ailus wrote:
> On Tue, Apr 17, 2018 at 03:10:24PM +0200, Maxime Ripard wrote:
> > Hi Sakari,
> > 
> > On Fri, Apr 13, 2018 at 03:14:37PM +0300, Sakari Ailus wrote:
> > > > +static int csi2tx_set_pad_format(struct v4l2_subdev *subdev,
> > > > +				 struct v4l2_subdev_pad_config *cfg,
> > > > +				 struct v4l2_subdev_format *fmt)
> > > > +{
> > > > +	struct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);
> > > > +
> > > > +	if (fmt->pad >= CSI2TX_PAD_MAX)
> > > > +		return -EINVAL;
> > > > +
> > > > +	csi2tx->pad_fmts[fmt->pad] = fmt->format;
> > > 
> > > Have I asked previously if there are any limitations with this?
> > > 
> > > The CSI-2 TX link has multiple formats so I wouldn't support formats on
> > > that pad in order to be compatible with the planned VC/data type support
> > > patchset. Or do you see issues with that?
> > 
> > It's not just about the CSI-2 link, but more about the input pads as
> > well, that can be configured (and we need to know the format in order
> > to configure the IP properly).
> > 
> > Maybe we can simply prevent the format change on the CSI-2 pad, but
> > not the others?
> 
> Yes, that was what I wanted to suggest. It's in line with the intended way
> to support multiplexed pads.
> 
> The latest set is here:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=vc>

Thanks for the pointer.

I've looked at the smiapp set_format hook, and especially:
https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=vc&id=cb864a1d8e2d19b793d8f550b026dcf8d2f78f11#n1817

After reading this, I'm not quite sure to get what I should do for the
CSI-2 pad. Should I ignore all formats change (and thus return 0), or
should I return EINVAL (which would probably be a bit confusing to the
userspace)?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]