| Message ID | 20200914215011.339387-3-niklas.soderlund+renesas@ragnatech.se |
|---|---|
| State | Accepted |
| Commit | 055e124eec851996567910def94ead0aaac8fa36 |
| Headers | show |
| Series | rcar-csi2: Update how DT is traversed and parsed | expand |
Hi Niklas, On Mon, Sep 14, 2020 at 11:50:11PM +0200, Niklas Söderlund wrote: > The only supported bus for the R-Car CSI-2 driver is CSI-2 DPHY, specify > this before parsing the fwnode. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > index 23e89ef2429d310a..b2e58f51b94fccd7 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -874,7 +874,9 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) > struct v4l2_async_subdev *asd; > struct fwnode_handle *fwnode; > struct fwnode_handle *ep; > - struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; > + struct v4l2_fwnode_endpoint v4l2_ep = { > + .bus_type = V4L2_MBUS_CSI2_DPHY > + }; I would also take the occasion to make bus-type mandatory in bindings as v4l2_fwnode_endpoint_parse() will fail only if it detect a mismatch between bus_type and "bus-type". For older DTS we won't detect mismatches, but that's not worse than what we have today. In case you update bindings I would update the error message in the v4l2_fwnode_endpoint_parse() failure path to report the mismatch. The patch itself is good Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Thanks j > int ret; > > ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(priv->dev), 0, 0, 0); > -- > 2.28.0 >
Hi Jacopo, On Tue, Sep 15, 2020 at 01:27:52PM +0200, Jacopo Mondi wrote: > Hi Niklas, > > On Mon, Sep 14, 2020 at 11:50:11PM +0200, Niklas Söderlund wrote: > > The only supported bus for the R-Car CSI-2 driver is CSI-2 DPHY, specify > > this before parsing the fwnode. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > drivers/media/platform/rcar-vin/rcar-csi2.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > > index 23e89ef2429d310a..b2e58f51b94fccd7 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > > @@ -874,7 +874,9 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) > > struct v4l2_async_subdev *asd; > > struct fwnode_handle *fwnode; > > struct fwnode_handle *ep; > > - struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; > > + struct v4l2_fwnode_endpoint v4l2_ep = { > > + .bus_type = V4L2_MBUS_CSI2_DPHY > > + }; > > I would also take the occasion to make bus-type mandatory in > bindings as v4l2_fwnode_endpoint_parse() will fail only if it detect a > mismatch between bus_type and "bus-type". You don't really need bus-type property if the hardware supports a single type. Then you can, as above, parse the endpoint with that type set by the caller.
Hi Sakari, On Sat, Oct 03, 2020 at 12:21:03AM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Tue, Sep 15, 2020 at 01:27:52PM +0200, Jacopo Mondi wrote: > > Hi Niklas, > > > > On Mon, Sep 14, 2020 at 11:50:11PM +0200, Niklas Söderlund wrote: > > > The only supported bus for the R-Car CSI-2 driver is CSI-2 DPHY, specify > > > this before parsing the fwnode. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > --- > > > drivers/media/platform/rcar-vin/rcar-csi2.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > > > index 23e89ef2429d310a..b2e58f51b94fccd7 100644 > > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > > > @@ -874,7 +874,9 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) > > > struct v4l2_async_subdev *asd; > > > struct fwnode_handle *fwnode; > > > struct fwnode_handle *ep; > > > - struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; > > > + struct v4l2_fwnode_endpoint v4l2_ep = { > > > + .bus_type = V4L2_MBUS_CSI2_DPHY > > > + }; > > > > I would also take the occasion to make bus-type mandatory in > > bindings as v4l2_fwnode_endpoint_parse() will fail only if it detect a > > mismatch between bus_type and "bus-type". > > You don't really need bus-type property if the hardware supports a single > type. Then you can, as above, parse the endpoint with that type set by the > caller. Ok, that's a bit confusing as if there's no bus-type property no bus mismatch could ever be detected, not at run-time by the v4l2-fwnode framework, nor by DTS validation. Of course, the chances that a DTS for a device that only supports CSI-2 specifies (in example) parallel bus properties are quite low, so I'm fine with the way things are. > > -- > Sakari Ailus
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c index 23e89ef2429d310a..b2e58f51b94fccd7 100644 --- a/drivers/media/platform/rcar-vin/rcar-csi2.c +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c @@ -874,7 +874,9 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv) struct v4l2_async_subdev *asd; struct fwnode_handle *fwnode; struct fwnode_handle *ep; - struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 }; + struct v4l2_fwnode_endpoint v4l2_ep = { + .bus_type = V4L2_MBUS_CSI2_DPHY + }; int ret; ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(priv->dev), 0, 0, 0);
The only supported bus for the R-Car CSI-2 driver is CSI-2 DPHY, specify this before parsing the fwnode. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-csi2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)