linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: i2c: max9286: Pass default bus type when parsing fwnode endpoint
@ 2019-10-01 14:55 Manivannan Sadhasivam
  2019-10-02  7:54 ` Jacopo Mondi
  0 siblings, 1 reply; 3+ messages in thread
From: Manivannan Sadhasivam @ 2019-10-01 14:55 UTC (permalink / raw)
  To: jacopo+renesas, kieran.bingham+renesas, laurent.pinchart+renesas,
	niklas.soderlund+renesas, mchehab
  Cc: linux-media, linux-kernel, Manivannan Sadhasivam

The caller of v4l2_fwnode_endpoint_alloc_parse() is expected to pass a
valid bus_type parameter for proper working of this API. Hence, pass
V4L2_MBUS_CSI2_DPHY as the bus_type parameter as this driver only supports
MIPI CSI2 for now. Without this commit, the API fails on 96Boards
Dragonboard410c connected to MAX9286 deserializer.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

This patch depends on the latest "MAX9286 GMSL Support" series posted
by Kieran Bingham.

 drivers/media/i2c/max9286.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 9390edf5ad9c..6e1299f15493 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -976,7 +976,9 @@ static int max9286_parse_dt(struct max9286_device *max9286)
 
 		/* For the source endpoint just parse the bus configuration. */
 		if (ep.port == MAX9286_SRC_PAD) {
-			struct v4l2_fwnode_endpoint vep;
+			struct v4l2_fwnode_endpoint vep = {
+				.bus_type = V4L2_MBUS_CSI2_DPHY
+			};
 			int ret;
 
 			ret = v4l2_fwnode_endpoint_alloc_parse(
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: i2c: max9286: Pass default bus type when parsing fwnode endpoint
  2019-10-01 14:55 [PATCH] media: i2c: max9286: Pass default bus type when parsing fwnode endpoint Manivannan Sadhasivam
@ 2019-10-02  7:54 ` Jacopo Mondi
  2019-10-02  8:56   ` Sakari Ailus
  0 siblings, 1 reply; 3+ messages in thread
From: Jacopo Mondi @ 2019-10-02  7:54 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: jacopo+renesas, kieran.bingham+renesas, laurent.pinchart+renesas,
	niklas.soderlund+renesas, mchehab, linux-media, linux-kernel

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

Hello Manivannan,

On Tue, Oct 01, 2019 at 08:25:03PM +0530, Manivannan Sadhasivam wrote:
> The caller of v4l2_fwnode_endpoint_alloc_parse() is expected to pass a
> valid bus_type parameter for proper working of this API. Hence, pass
> V4L2_MBUS_CSI2_DPHY as the bus_type parameter as this driver only supports
> MIPI CSI2 for now. Without this commit, the API fails on 96Boards
> Dragonboard410c connected to MAX9286 deserializer.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>
> This patch depends on the latest "MAX9286 GMSL Support" series posted
> by Kieran Bingham.
>
>  drivers/media/i2c/max9286.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 9390edf5ad9c..6e1299f15493 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -976,7 +976,9 @@ static int max9286_parse_dt(struct max9286_device *max9286)
>
>  		/* For the source endpoint just parse the bus configuration. */
>  		if (ep.port == MAX9286_SRC_PAD) {
> -			struct v4l2_fwnode_endpoint vep;
> +			struct v4l2_fwnode_endpoint vep = {
> +				.bus_type = V4L2_MBUS_CSI2_DPHY
> +			};

Isn't the bus_type just an hint ? __v4l2_fwnode_endpoint_parse()
should try to parse CSI2 first, and should be able to to deduct the bus
type from the properties specified in DT...

Anyway, I'm not against this change, the contrary actually, I'm just
afraid it might hide some more subtile bug, as we don't need this on
our platforms...

Is the db410c mainline ? Do you have DT snippets for db410c+max9286 to
share?

Thanks
  j

>  			int ret;
>
>  			ret = v4l2_fwnode_endpoint_alloc_parse(
> --
> 2.17.1
>

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: i2c: max9286: Pass default bus type when parsing fwnode endpoint
  2019-10-02  7:54 ` Jacopo Mondi
@ 2019-10-02  8:56   ` Sakari Ailus
  0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2019-10-02  8:56 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Manivannan Sadhasivam, jacopo+renesas, kieran.bingham+renesas,
	laurent.pinchart+renesas, niklas.soderlund+renesas, mchehab,
	linux-media, linux-kernel

Hi Jacopo, Manivannan,

On Wed, Oct 02, 2019 at 09:54:32AM +0200, Jacopo Mondi wrote:
> Hello Manivannan,
> 
> On Tue, Oct 01, 2019 at 08:25:03PM +0530, Manivannan Sadhasivam wrote:
> > The caller of v4l2_fwnode_endpoint_alloc_parse() is expected to pass a
> > valid bus_type parameter for proper working of this API. Hence, pass
> > V4L2_MBUS_CSI2_DPHY as the bus_type parameter as this driver only supports
> > MIPI CSI2 for now. Without this commit, the API fails on 96Boards
> > Dragonboard410c connected to MAX9286 deserializer.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >
> > This patch depends on the latest "MAX9286 GMSL Support" series posted
> > by Kieran Bingham.
> >
> >  drivers/media/i2c/max9286.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > index 9390edf5ad9c..6e1299f15493 100644
> > --- a/drivers/media/i2c/max9286.c
> > +++ b/drivers/media/i2c/max9286.c
> > @@ -976,7 +976,9 @@ static int max9286_parse_dt(struct max9286_device *max9286)
> >
> >  		/* For the source endpoint just parse the bus configuration. */
> >  		if (ep.port == MAX9286_SRC_PAD) {
> > -			struct v4l2_fwnode_endpoint vep;
> > +			struct v4l2_fwnode_endpoint vep = {
> > +				.bus_type = V4L2_MBUS_CSI2_DPHY
> > +			};
> 
> Isn't the bus_type just an hint ? __v4l2_fwnode_endpoint_parse()
> should try to parse CSI2 first, and should be able to to deduct the bus
> type from the properties specified in DT...

It's basically telling the V4L2 fwnode parser to parse that bus only, and
not guess what the caller wanted.

This is what drivers should actually be doing. It also allows setting the
defaults that many drivers declare in their DT bindings but very few
actually implement.

> 
> Anyway, I'm not against this change, the contrary actually, I'm just
> afraid it might hide some more subtile bug, as we don't need this on
> our platforms...

I'd expect it to be safer this way, as there's no guessing involved.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-10-02  8:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-01 14:55 [PATCH] media: i2c: max9286: Pass default bus type when parsing fwnode endpoint Manivannan Sadhasivam
2019-10-02  7:54 ` Jacopo Mondi
2019-10-02  8:56   ` Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).