| Message ID | 20191124190703.12138-7-slongerbeam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: Create media links in bound notifiers | expand |
Hi Steve, On Sun, Nov 24, 2019 at 11:06:46AM -0800, Steve Longerbeam wrote: > If the given endpoint fwnode passed to the .get_fwnode_pad() op is > the adv748x-csi2 TXA/TXB source endpoint, return the associated media > pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads > that are associated with fwnode endpoints. > > Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> > --- > drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c > index 2091cda50935..810085a1f2f0 100644 > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c > @@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = { > .pad = &adv748x_csi2_pad_ops, > }; > > +/* ----------------------------------------------------------------------------- > + * media_entity_operations > + */ > + > +static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity, > + struct fwnode_endpoint *endpoint) > +{ > + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); > + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); > + > + return endpoint->local_fwnode == tx->sd.fwnode ? > + ADV748X_CSI2_SOURCE : -ENXIO; Couldn't you check if the endpoint port is either 10 or 11, as those are the only port numbers that provide a CSI-2 source pad ? In that case you could drop extending get_fwnode_pad() with th entity argument, as it is only used here (this one is actually the first user in the whole code base of this operation) > +} > + > +static const struct media_entity_operations adv748x_csi2_entity_ops = { > + .get_fwnode_pad = adv748x_csi2_get_fwnode_pad, > +}; > + > /* ----------------------------------------------------------------------------- > * Subdev module and controls > */ > @@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx) > /* Register internal ops for incremental subdev registration */ > tx->sd.internal_ops = &adv748x_csi2_internal_ops; > > + /* Register media_entity ops */ > + tx->sd.entity.ops = &adv748x_csi2_entity_ops; > + > tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK; > tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE; > > -- > 2.17.1 >
Hi Jacopo, On 11/28/19 3:53 AM, Jacopo Mondi wrote: > Hi Steve, > > On Sun, Nov 24, 2019 at 11:06:46AM -0800, Steve Longerbeam wrote: >> If the given endpoint fwnode passed to the .get_fwnode_pad() op is >> the adv748x-csi2 TXA/TXB source endpoint, return the associated media >> pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads >> that are associated with fwnode endpoints. >> >> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> >> --- >> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c >> index 2091cda50935..810085a1f2f0 100644 >> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c >> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c >> @@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = { >> .pad = &adv748x_csi2_pad_ops, >> }; >> >> +/* ----------------------------------------------------------------------------- >> + * media_entity_operations >> + */ >> + >> +static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity, >> + struct fwnode_endpoint *endpoint) >> +{ >> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); >> + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); >> + >> + return endpoint->local_fwnode == tx->sd.fwnode ? >> + ADV748X_CSI2_SOURCE : -ENXIO; > Couldn't you check if the endpoint port is either 10 or 11, as those > are the only port numbers that provide a CSI-2 source pad ? > > In that case you could drop extending get_fwnode_pad() with th entity > argument, as it is only used here (this one is actually the first user > in the whole code base of this operation) I don't think that's a very good idea. Entities that implement .get_fwnode_pad() in the future will likely need access to themselves in order to do the work. This implementation is a good example of that and is a better approach to checking the port number, because it is actually verifying that the endpoint fwnode object is owned by this device. Steve > >> +} >> + >> +static const struct media_entity_operations adv748x_csi2_entity_ops = { >> + .get_fwnode_pad = adv748x_csi2_get_fwnode_pad, >> +}; >> + >> /* ----------------------------------------------------------------------------- >> * Subdev module and controls >> */ >> @@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx) >> /* Register internal ops for incremental subdev registration */ >> tx->sd.internal_ops = &adv748x_csi2_internal_ops; >> >> + /* Register media_entity ops */ >> + tx->sd.entity.ops = &adv748x_csi2_entity_ops; >> + >> tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK; >> tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE; >> >> -- >> 2.17.1 >>
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c index 2091cda50935..810085a1f2f0 100644 --- a/drivers/media/i2c/adv748x/adv748x-csi2.c +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c @@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = { .pad = &adv748x_csi2_pad_ops, }; +/* ----------------------------------------------------------------------------- + * media_entity_operations + */ + +static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity, + struct fwnode_endpoint *endpoint) +{ + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); + + return endpoint->local_fwnode == tx->sd.fwnode ? + ADV748X_CSI2_SOURCE : -ENXIO; +} + +static const struct media_entity_operations adv748x_csi2_entity_ops = { + .get_fwnode_pad = adv748x_csi2_get_fwnode_pad, +}; + /* ----------------------------------------------------------------------------- * Subdev module and controls */ @@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx) /* Register internal ops for incremental subdev registration */ tx->sd.internal_ops = &adv748x_csi2_internal_ops; + /* Register media_entity ops */ + tx->sd.entity.ops = &adv748x_csi2_entity_ops; + tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK; tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
If the given endpoint fwnode passed to the .get_fwnode_pad() op is the adv748x-csi2 TXA/TXB source endpoint, return the associated media pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads that are associated with fwnode endpoints. Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> --- drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)