| Message ID | 20230731-fpdlink-additions-v3-1-8acfc49c215a@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: ds90ub9xx: Misc improvements | expand |
Hi Tomi, Thank you for the patch. On Mon, Jul 31, 2023 at 04:24:35PM +0300, Tomi Valkeinen wrote: > 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode s/^/Commit / > matching") recently changed how async sub-device matching works. This > breaks the UB9x3 drivers, as they set the subdev.fwnode to an endpoint. > Afaiu, the fix is simply to not set subdev.fwnode at all. > > Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching") > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Sakari, was the v4l2-async series meant to break these drivers ? I understand the two series got merged for the same kernel version, is this a merge conflict, or is there an issue in the v4l2-async rework ? > --- > drivers/media/i2c/ds90ub913.c | 14 +------------- > drivers/media/i2c/ds90ub953.c | 13 +------------ > 2 files changed, 2 insertions(+), 25 deletions(-) > > diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c > index 80d9cf6dd945..5410ccb54057 100644 > --- a/drivers/media/i2c/ds90ub913.c > +++ b/drivers/media/i2c/ds90ub913.c > @@ -749,19 +749,9 @@ static int ub913_subdev_init(struct ub913_data *priv) > if (ret) > return dev_err_probe(dev, ret, "Failed to init pads\n"); > > - priv->sd.fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), > - UB913_PAD_SOURCE, 0, > - 0); > - > - if (!priv->sd.fwnode) { > - ret = -ENODEV; > - dev_err_probe(dev, ret, "Missing TX endpoint\n"); > - goto err_entity_cleanup; > - } > - > ret = v4l2_subdev_init_finalize(&priv->sd); > if (ret) > - goto err_fwnode_put; > + goto err_entity_cleanup; > > ret = ub913_v4l2_notifier_register(priv); > if (ret) { > @@ -782,8 +772,6 @@ static int ub913_subdev_init(struct ub913_data *priv) > ub913_v4l2_nf_unregister(priv); > err_subdev_cleanup: > v4l2_subdev_cleanup(&priv->sd); > -err_fwnode_put: > - fwnode_handle_put(priv->sd.fwnode); > err_entity_cleanup: > media_entity_cleanup(&priv->sd.entity); > > diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c > index cadf75eb0773..20b9cf893f74 100644 > --- a/drivers/media/i2c/ds90ub953.c > +++ b/drivers/media/i2c/ds90ub953.c > @@ -1221,18 +1221,9 @@ static int ub953_subdev_init(struct ub953_data *priv) > if (ret) > return dev_err_probe(dev, ret, "Failed to init pads\n"); > > - priv->sd.fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), > - UB953_PAD_SOURCE, 0, > - 0); > - if (!priv->sd.fwnode) { > - ret = -ENODEV; > - dev_err_probe(dev, ret, "Missing TX endpoint\n"); > - goto err_entity_cleanup; > - } > - > ret = v4l2_subdev_init_finalize(&priv->sd); > if (ret) > - goto err_fwnode_put; > + goto err_entity_cleanup; > > ret = ub953_v4l2_notifier_register(priv); > if (ret) { > @@ -1253,8 +1244,6 @@ static int ub953_subdev_init(struct ub953_data *priv) > ub953_v4l2_notifier_unregister(priv); > err_free_state: > v4l2_subdev_cleanup(&priv->sd); > -err_fwnode_put: > - fwnode_handle_put(priv->sd.fwnode); > err_entity_cleanup: > media_entity_cleanup(&priv->sd.entity); > >
Hi Laurent, On Mon, Jul 31, 2023 at 05:43:56PM +0300, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Mon, Jul 31, 2023 at 04:24:35PM +0300, Tomi Valkeinen wrote: > > 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode > > s/^/Commit / > > > matching") recently changed how async sub-device matching works. This > > breaks the UB9x3 drivers, as they set the subdev.fwnode to an endpoint. > > Afaiu, the fix is simply to not set subdev.fwnode at all. > > > > Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching") > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > > Sakari, was the v4l2-async series meant to break these drivers ? I > understand the two series got merged for the same kernel version, is > this a merge conflict, or is there an issue in the v4l2-async rework ? The ds90ub9xx drivers were merged after I had written the patch that converted all drivers and I didn't remember to revisit it. If you look at the patch, it's doing very similar things than the patch in the Fixes: tag. There's also a workaround for sub-device drivers (that register async sub-devices) but not for drivers that register a notifier. It probably doesn't make sense to add a workaround for those how, rather remove the one that exists (after some time).
diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c index 80d9cf6dd945..5410ccb54057 100644 --- a/drivers/media/i2c/ds90ub913.c +++ b/drivers/media/i2c/ds90ub913.c @@ -749,19 +749,9 @@ static int ub913_subdev_init(struct ub913_data *priv) if (ret) return dev_err_probe(dev, ret, "Failed to init pads\n"); - priv->sd.fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), - UB913_PAD_SOURCE, 0, - 0); - - if (!priv->sd.fwnode) { - ret = -ENODEV; - dev_err_probe(dev, ret, "Missing TX endpoint\n"); - goto err_entity_cleanup; - } - ret = v4l2_subdev_init_finalize(&priv->sd); if (ret) - goto err_fwnode_put; + goto err_entity_cleanup; ret = ub913_v4l2_notifier_register(priv); if (ret) { @@ -782,8 +772,6 @@ static int ub913_subdev_init(struct ub913_data *priv) ub913_v4l2_nf_unregister(priv); err_subdev_cleanup: v4l2_subdev_cleanup(&priv->sd); -err_fwnode_put: - fwnode_handle_put(priv->sd.fwnode); err_entity_cleanup: media_entity_cleanup(&priv->sd.entity); diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c index cadf75eb0773..20b9cf893f74 100644 --- a/drivers/media/i2c/ds90ub953.c +++ b/drivers/media/i2c/ds90ub953.c @@ -1221,18 +1221,9 @@ static int ub953_subdev_init(struct ub953_data *priv) if (ret) return dev_err_probe(dev, ret, "Failed to init pads\n"); - priv->sd.fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), - UB953_PAD_SOURCE, 0, - 0); - if (!priv->sd.fwnode) { - ret = -ENODEV; - dev_err_probe(dev, ret, "Missing TX endpoint\n"); - goto err_entity_cleanup; - } - ret = v4l2_subdev_init_finalize(&priv->sd); if (ret) - goto err_fwnode_put; + goto err_entity_cleanup; ret = ub953_v4l2_notifier_register(priv); if (ret) { @@ -1253,8 +1244,6 @@ static int ub953_subdev_init(struct ub953_data *priv) ub953_v4l2_notifier_unregister(priv); err_free_state: v4l2_subdev_cleanup(&priv->sd); -err_fwnode_put: - fwnode_handle_put(priv->sd.fwnode); err_entity_cleanup: media_entity_cleanup(&priv->sd.entity);
1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching") recently changed how async sub-device matching works. This breaks the UB9x3 drivers, as they set the subdev.fwnode to an endpoint. Afaiu, the fix is simply to not set subdev.fwnode at all. Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching") Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/i2c/ds90ub913.c | 14 +------------- drivers/media/i2c/ds90ub953.c | 13 +------------ 2 files changed, 2 insertions(+), 25 deletions(-)