* [PATCH v10 04/24] v4l: async: Add V4L2 async documentation to the documentation build
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
The V4L2 async wasn't part of the documentation build. Fix this.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
Documentation/media/kapi/v4l2-async.rst | 3 +++
Documentation/media/kapi/v4l2-core.rst | 1 +
2 files changed, 4 insertions(+)
create mode 100644 Documentation/media/kapi/v4l2-async.rst
diff --git a/Documentation/media/kapi/v4l2-async.rst b/Documentation/media/kapi/v4l2-async.rst
new file mode 100644
index 000000000000..523ff9eb09a0
--- /dev/null
+++ b/Documentation/media/kapi/v4l2-async.rst
@@ -0,0 +1,3 @@
+V4L2 async kAPI
+^^^^^^^^^^^^^^^
+.. kernel-doc:: include/media/v4l2-async.h
diff --git a/Documentation/media/kapi/v4l2-core.rst b/Documentation/media/kapi/v4l2-core.rst
index c7434f38fd9c..5cf292037a48 100644
--- a/Documentation/media/kapi/v4l2-core.rst
+++ b/Documentation/media/kapi/v4l2-core.rst
@@ -19,6 +19,7 @@ Video4Linux devices
v4l2-mc
v4l2-mediabus
v4l2-mem2mem
+ v4l2-async
v4l2-fwnode
v4l2-rect
v4l2-tuner
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 04/24] v4l: async: Add V4L2 async documentation to the documentation build
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
The V4L2 async wasn't part of the documentation build. Fix this.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Documentation/media/kapi/v4l2-async.rst | 3 +++
Documentation/media/kapi/v4l2-core.rst | 1 +
2 files changed, 4 insertions(+)
create mode 100644 Documentation/media/kapi/v4l2-async.rst
diff --git a/Documentation/media/kapi/v4l2-async.rst b/Documentation/media/kapi/v4l2-async.rst
new file mode 100644
index 000000000000..523ff9eb09a0
--- /dev/null
+++ b/Documentation/media/kapi/v4l2-async.rst
@@ -0,0 +1,3 @@
+V4L2 async kAPI
+^^^^^^^^^^^^^^^
+.. kernel-doc:: include/media/v4l2-async.h
diff --git a/Documentation/media/kapi/v4l2-core.rst b/Documentation/media/kapi/v4l2-core.rst
index c7434f38fd9c..5cf292037a48 100644
--- a/Documentation/media/kapi/v4l2-core.rst
+++ b/Documentation/media/kapi/v4l2-core.rst
@@ -19,6 +19,7 @@ Video4Linux devices
v4l2-mc
v4l2-mediabus
v4l2-mem2mem
+ v4l2-async
v4l2-fwnode
v4l2-rect
v4l2-tuner
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 06/24] omap3isp: Use generic parser for parsing fwnode endpoints
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
Instead of using driver implementation, use
v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
of the device.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
drivers/media/platform/omap3isp/isp.c | 115 +++++++++++-----------------------
drivers/media/platform/omap3isp/isp.h | 5 +-
2 files changed, 37 insertions(+), 83 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 1a428fe9f070..a546cf774d40 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
__omap3isp_put(isp, false);
media_entity_enum_cleanup(&isp->crashed);
+ v4l2_async_notifier_release(&isp->notifier);
return 0;
}
@@ -2011,44 +2012,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
};
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
- struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
+ struct isp_async_subdev *isd =
+ container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = &isd->bus;
- struct v4l2_fwnode_endpoint vep;
- unsigned int i;
- int ret;
bool csi1 = false;
-
- ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
- if (ret)
- return ret;
+ unsigned int i;
dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
- to_of_node(fwnode), vep.base.port);
+ to_of_node(vep->base.local_fwnode), vep->base.port);
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
- vep.bus.parallel.data_shift;
+ vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
- !!(vep.bus.parallel.flags
+ !!(vep->bus.parallel.flags
& V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
- buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+ !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+ buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- switch (vep.bus_type) {
+ switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
- vep.bus_type);
+ vep->bus_type);
return -EINVAL;
}
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
- vep.bus.mipi_csi1.clock_lane;
+ vep->bus.mipi_csi1.clock_lane;
buscfg->bus.ccp2.lanecfg.clk.pol =
- vep.bus.mipi_csi1.lane_polarity[0];
+ vep->bus.mipi_csi1.lane_polarity[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.ccp2.lanecfg.clk.pol,
buscfg->bus.ccp2.lanecfg.clk.pos);
buscfg->bus.ccp2.lanecfg.data[0].pos =
- vep.bus.mipi_csi1.data_lane;
+ vep->bus.mipi_csi1.data_lane;
buscfg->bus.ccp2.lanecfg.data[0].pol =
- vep.bus.mipi_csi1.lane_polarity[1];
+ vep->bus.mipi_csi1.lane_polarity[1];
dev_dbg(dev, "data lane polarity %u, pos %u\n",
buscfg->bus.ccp2.lanecfg.data[0].pol,
buscfg->bus.ccp2.lanecfg.data[0].pos);
buscfg->bus.ccp2.strobe_clk_pol =
- vep.bus.mipi_csi1.clock_inv;
- buscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;
+ vep->bus.mipi_csi1.clock_inv;
+ buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
buscfg->bus.ccp2.ccp2_mode =
- vep.bus_type == V4L2_MBUS_CCP2;
+ vep->bus_type == V4L2_MBUS_CCP2;
buscfg->bus.ccp2.vp_clk_pol = 1;
buscfg->bus.ccp2.crc = 1;
} else {
buscfg->bus.csi2.lanecfg.clk.pos =
- vep.bus.mipi_csi2.clock_lane;
+ vep->bus.mipi_csi2.clock_lane;
buscfg->bus.csi2.lanecfg.clk.pol =
- vep.bus.mipi_csi2.lane_polarities[0];
+ vep->bus.mipi_csi2.lane_polarities[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.csi2.lanecfg.clk.pol,
buscfg->bus.csi2.lanecfg.clk.pos);
buscfg->bus.csi2.num_data_lanes =
- vep.bus.mipi_csi2.num_data_lanes;
+ vep->bus.mipi_csi2.num_data_lanes;
for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
buscfg->bus.csi2.lanecfg.data[i].pos =
- vep.bus.mipi_csi2.data_lanes[i];
+ vep->bus.mipi_csi2.data_lanes[i];
buscfg->bus.csi2.lanecfg.data[i].pol =
- vep.bus.mipi_csi2.lane_polarities[i + 1];
+ vep->bus.mipi_csi2.lane_polarities[i + 1];
dev_dbg(dev,
"data lane %u polarity %u, pos %u\n", i,
buscfg->bus.csi2.lanecfg.data[i].pol,
@@ -2137,57 +2135,13 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
default:
dev_warn(dev, "%pOF: invalid interface %u\n",
- to_of_node(fwnode), vep.base.port);
+ to_of_node(vep->base.local_fwnode), vep->base.port);
return -EINVAL;
}
return 0;
}
-static int isp_fwnodes_parse(struct device *dev,
- struct v4l2_async_notifier *notifier)
-{
- struct fwnode_handle *fwnode = NULL;
-
- notifier->subdevs = devm_kcalloc(
- dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
- if (!notifier->subdevs)
- return -ENOMEM;
-
- while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
- (fwnode = fwnode_graph_get_next_endpoint(
- of_fwnode_handle(dev->of_node), fwnode))) {
- struct isp_async_subdev *isd;
-
- isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
- if (!isd)
- goto error;
-
- if (isp_fwnode_parse(dev, fwnode, isd)) {
- devm_kfree(dev, isd);
- continue;
- }
-
- notifier->subdevs[notifier->num_subdevs] = &isd->asd;
-
- isd->asd.match.fwnode.fwnode =
- fwnode_graph_get_remote_port_parent(fwnode);
- if (!isd->asd.match.fwnode.fwnode) {
- dev_warn(dev, "bad remote port parent\n");
- goto error;
- }
-
- isd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
- notifier->num_subdevs++;
- }
-
- return notifier->num_subdevs;
-
-error:
- fwnode_handle_put(fwnode);
- return -EINVAL;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
@@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);
+ ret = v4l2_async_notifier_parse_fwnode_endpoints(
+ &pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
+ isp_fwnode_parse);
if (ret < 0)
return ret;
@@ -2407,6 +2363,7 @@ static int isp_probe(struct platform_device *pdev)
__omap3isp_put(isp, false);
error:
mutex_destroy(&isp->isp_mutex);
+ v4l2_async_notifier_release(&isp->notifier);
return ret;
}
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index e528df6efc09..8b9043db94b3 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -220,14 +220,11 @@ struct isp_device {
unsigned int sbl_resources;
unsigned int subclk_resources;
-
-#define ISP_MAX_SUBDEVS 8
- struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
};
struct isp_async_subdev {
- struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
+ struct isp_bus_cfg bus;
};
#define v4l2_subdev_to_bus_cfg(sd) \
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 06/24] omap3isp: Use generic parser for parsing fwnode endpoints
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Instead of using driver implementation, use
v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
of the device.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/omap3isp/isp.c | 115 +++++++++++-----------------------
drivers/media/platform/omap3isp/isp.h | 5 +-
2 files changed, 37 insertions(+), 83 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 1a428fe9f070..a546cf774d40 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
__omap3isp_put(isp, false);
media_entity_enum_cleanup(&isp->crashed);
+ v4l2_async_notifier_release(&isp->notifier);
return 0;
}
@@ -2011,44 +2012,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
};
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
- struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
+ struct isp_async_subdev *isd =
+ container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = &isd->bus;
- struct v4l2_fwnode_endpoint vep;
- unsigned int i;
- int ret;
bool csi1 = false;
-
- ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
- if (ret)
- return ret;
+ unsigned int i;
dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
- to_of_node(fwnode), vep.base.port);
+ to_of_node(vep->base.local_fwnode), vep->base.port);
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
- vep.bus.parallel.data_shift;
+ vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
- !!(vep.bus.parallel.flags
+ !!(vep->bus.parallel.flags
& V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
- buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+ !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+ buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- switch (vep.bus_type) {
+ switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
- vep.bus_type);
+ vep->bus_type);
return -EINVAL;
}
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
- vep.bus.mipi_csi1.clock_lane;
+ vep->bus.mipi_csi1.clock_lane;
buscfg->bus.ccp2.lanecfg.clk.pol =
- vep.bus.mipi_csi1.lane_polarity[0];
+ vep->bus.mipi_csi1.lane_polarity[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.ccp2.lanecfg.clk.pol,
buscfg->bus.ccp2.lanecfg.clk.pos);
buscfg->bus.ccp2.lanecfg.data[0].pos =
- vep.bus.mipi_csi1.data_lane;
+ vep->bus.mipi_csi1.data_lane;
buscfg->bus.ccp2.lanecfg.data[0].pol =
- vep.bus.mipi_csi1.lane_polarity[1];
+ vep->bus.mipi_csi1.lane_polarity[1];
dev_dbg(dev, "data lane polarity %u, pos %u\n",
buscfg->bus.ccp2.lanecfg.data[0].pol,
buscfg->bus.ccp2.lanecfg.data[0].pos);
buscfg->bus.ccp2.strobe_clk_pol =
- vep.bus.mipi_csi1.clock_inv;
- buscfg->bus.ccp2.phy_layer = vep.bus.mipi_csi1.strobe;
+ vep->bus.mipi_csi1.clock_inv;
+ buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
buscfg->bus.ccp2.ccp2_mode =
- vep.bus_type == V4L2_MBUS_CCP2;
+ vep->bus_type == V4L2_MBUS_CCP2;
buscfg->bus.ccp2.vp_clk_pol = 1;
buscfg->bus.ccp2.crc = 1;
} else {
buscfg->bus.csi2.lanecfg.clk.pos =
- vep.bus.mipi_csi2.clock_lane;
+ vep->bus.mipi_csi2.clock_lane;
buscfg->bus.csi2.lanecfg.clk.pol =
- vep.bus.mipi_csi2.lane_polarities[0];
+ vep->bus.mipi_csi2.lane_polarities[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.csi2.lanecfg.clk.pol,
buscfg->bus.csi2.lanecfg.clk.pos);
buscfg->bus.csi2.num_data_lanes =
- vep.bus.mipi_csi2.num_data_lanes;
+ vep->bus.mipi_csi2.num_data_lanes;
for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
buscfg->bus.csi2.lanecfg.data[i].pos =
- vep.bus.mipi_csi2.data_lanes[i];
+ vep->bus.mipi_csi2.data_lanes[i];
buscfg->bus.csi2.lanecfg.data[i].pol =
- vep.bus.mipi_csi2.lane_polarities[i + 1];
+ vep->bus.mipi_csi2.lane_polarities[i + 1];
dev_dbg(dev,
"data lane %u polarity %u, pos %u\n", i,
buscfg->bus.csi2.lanecfg.data[i].pol,
@@ -2137,57 +2135,13 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
default:
dev_warn(dev, "%pOF: invalid interface %u\n",
- to_of_node(fwnode), vep.base.port);
+ to_of_node(vep->base.local_fwnode), vep->base.port);
return -EINVAL;
}
return 0;
}
-static int isp_fwnodes_parse(struct device *dev,
- struct v4l2_async_notifier *notifier)
-{
- struct fwnode_handle *fwnode = NULL;
-
- notifier->subdevs = devm_kcalloc(
- dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
- if (!notifier->subdevs)
- return -ENOMEM;
-
- while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
- (fwnode = fwnode_graph_get_next_endpoint(
- of_fwnode_handle(dev->of_node), fwnode))) {
- struct isp_async_subdev *isd;
-
- isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
- if (!isd)
- goto error;
-
- if (isp_fwnode_parse(dev, fwnode, isd)) {
- devm_kfree(dev, isd);
- continue;
- }
-
- notifier->subdevs[notifier->num_subdevs] = &isd->asd;
-
- isd->asd.match.fwnode.fwnode =
- fwnode_graph_get_remote_port_parent(fwnode);
- if (!isd->asd.match.fwnode.fwnode) {
- dev_warn(dev, "bad remote port parent\n");
- goto error;
- }
-
- isd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
- notifier->num_subdevs++;
- }
-
- return notifier->num_subdevs;
-
-error:
- fwnode_handle_put(fwnode);
- return -EINVAL;
-}
-
static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
{
struct isp_device *isp = container_of(async, struct isp_device,
@@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);
+ ret = v4l2_async_notifier_parse_fwnode_endpoints(
+ &pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
+ isp_fwnode_parse);
if (ret < 0)
return ret;
@@ -2407,6 +2363,7 @@ static int isp_probe(struct platform_device *pdev)
__omap3isp_put(isp, false);
error:
mutex_destroy(&isp->isp_mutex);
+ v4l2_async_notifier_release(&isp->notifier);
return ret;
}
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index e528df6efc09..8b9043db94b3 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -220,14 +220,11 @@ struct isp_device {
unsigned int sbl_resources;
unsigned int subclk_resources;
-
-#define ISP_MAX_SUBDEVS 8
- struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
};
struct isp_async_subdev {
- struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
+ struct isp_bus_cfg bus;
};
#define v4l2_subdev_to_bus_cfg(sd) \
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 07/24] rcar-vin: Use generic parser for parsing fwnode endpoints
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
Instead of using driver implementation, use
v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
of the device.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
drivers/media/platform/rcar-vin/rcar-core.c | 112 +++++++++-------------------
drivers/media/platform/rcar-vin/rcar-dma.c | 10 +--
drivers/media/platform/rcar-vin/rcar-v4l2.c | 14 ++--
drivers/media/platform/rcar-vin/rcar-vin.h | 4 +-
4 files changed, 48 insertions(+), 92 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 142de447aaaa..62b4a94f9a39 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -21,6 +21,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-fwnode.h>
#include "rcar-vin.h"
@@ -77,14 +78,14 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
int ret;
/* Verify subdevices mbus format */
- if (!rvin_mbus_supported(&vin->digital)) {
+ if (!rvin_mbus_supported(vin->digital)) {
vin_err(vin, "Unsupported media bus format for %s\n",
- vin->digital.subdev->name);
+ vin->digital->subdev->name);
return -EINVAL;
}
vin_dbg(vin, "Found media bus format for %s: %d\n",
- vin->digital.subdev->name, vin->digital.code);
+ vin->digital->subdev->name, vin->digital->code);
ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
if (ret < 0) {
@@ -103,7 +104,7 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
rvin_v4l2_remove(vin);
- vin->digital.subdev = NULL;
+ vin->digital->subdev = NULL;
}
static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
@@ -120,117 +121,70 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
if (ret < 0)
return ret;
- vin->digital.source_pad = ret;
+ vin->digital->source_pad = ret;
ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
- vin->digital.sink_pad = ret < 0 ? 0 : ret;
+ vin->digital->sink_pad = ret < 0 ? 0 : ret;
- vin->digital.subdev = subdev;
+ vin->digital->subdev = subdev;
vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
- subdev->name, vin->digital.source_pad,
- vin->digital.sink_pad);
+ subdev->name, vin->digital->source_pad,
+ vin->digital->sink_pad);
return 0;
}
-static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,
- struct device_node *ep,
- struct v4l2_mbus_config *mbus_cfg)
+static int rvin_digital_parse_v4l2(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
- struct v4l2_fwnode_endpoint v4l2_ep;
- int ret;
+ struct rvin_dev *vin = dev_get_drvdata(dev);
+ struct rvin_graph_entity *rvge =
+ container_of(asd, struct rvin_graph_entity, asd);
- ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
- if (ret) {
- vin_err(vin, "Could not parse v4l2 endpoint\n");
- return -EINVAL;
- }
+ if (vep->base.port || vep->base.id)
+ return -ENOTCONN;
- mbus_cfg->type = v4l2_ep.bus_type;
+ rvge->mbus_cfg.type = vep->bus_type;
- switch (mbus_cfg->type) {
+ switch (rvge->mbus_cfg.type) {
case V4L2_MBUS_PARALLEL:
vin_dbg(vin, "Found PARALLEL media bus\n");
- mbus_cfg->flags = v4l2_ep.bus.parallel.flags;
+ rvge->mbus_cfg.flags = vep->bus.parallel.flags;
break;
case V4L2_MBUS_BT656:
vin_dbg(vin, "Found BT656 media bus\n");
- mbus_cfg->flags = 0;
+ rvge->mbus_cfg.flags = 0;
break;
default:
vin_err(vin, "Unknown media bus type\n");
return -EINVAL;
}
- return 0;
-}
-
-static int rvin_digital_graph_parse(struct rvin_dev *vin)
-{
- struct device_node *ep, *np;
- int ret;
-
- vin->digital.asd.match.fwnode.fwnode = NULL;
- vin->digital.subdev = NULL;
-
- /*
- * Port 0 id 0 is local digital input, try to get it.
- * Not all instances can or will have this, that is OK
- */
- ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);
- if (!ep)
- return 0;
-
- np = of_graph_get_remote_port_parent(ep);
- if (!np) {
- vin_err(vin, "No remote parent for digital input\n");
- of_node_put(ep);
- return -EINVAL;
- }
- of_node_put(np);
-
- ret = rvin_digitial_parse_v4l2(vin, ep, &vin->digital.mbus_cfg);
- of_node_put(ep);
- if (ret)
- return ret;
-
- vin->digital.asd.match.fwnode.fwnode = of_fwnode_handle(np);
- vin->digital.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
+ vin->digital = rvge;
return 0;
}
static int rvin_digital_graph_init(struct rvin_dev *vin)
{
- struct v4l2_async_subdev **subdevs = NULL;
int ret;
- ret = rvin_digital_graph_parse(vin);
+ ret = v4l2_async_notifier_parse_fwnode_endpoints(
+ vin->dev, &vin->notifier,
+ sizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);
if (ret)
return ret;
- if (!vin->digital.asd.match.fwnode.fwnode) {
- vin_dbg(vin, "No digital subdevice found\n");
- return -ENODEV;
- }
-
- /* Register the subdevices notifier. */
- subdevs = devm_kzalloc(vin->dev, sizeof(*subdevs), GFP_KERNEL);
- if (subdevs == NULL)
- return -ENOMEM;
-
- subdevs[0] = &vin->digital.asd;
-
- vin_dbg(vin, "Found digital subdevice %pOF\n",
- to_of_node(subdevs[0]->match.fwnode.fwnode));
+ if (vin->digital)
+ vin_dbg(vin, "Found digital subdevice %pOF\n",
+ to_of_node(
+ vin->digital->asd.match.fwnode.fwnode));
- vin->notifier.num_subdevs = 1;
- vin->notifier.subdevs = subdevs;
vin->notifier.bound = rvin_digital_notify_bound;
vin->notifier.unbind = rvin_digital_notify_unbind;
vin->notifier.complete = rvin_digital_notify_complete;
-
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
@@ -290,6 +244,8 @@ static int rcar_vin_probe(struct platform_device *pdev)
if (ret)
return ret;
+ platform_set_drvdata(pdev, vin);
+
ret = rvin_digital_graph_init(vin);
if (ret < 0)
goto error;
@@ -297,11 +253,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
pm_suspend_ignore_children(&pdev->dev, true);
pm_runtime_enable(&pdev->dev);
- platform_set_drvdata(pdev, vin);
-
return 0;
error:
rvin_dma_remove(vin);
+ v4l2_async_notifier_release(&vin->notifier);
return ret;
}
@@ -313,6 +268,7 @@ static int rcar_vin_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
v4l2_async_notifier_unregister(&vin->notifier);
+ v4l2_async_notifier_release(&vin->notifier);
rvin_dma_remove(vin);
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index b136844499f6..23fdff7a7370 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -183,7 +183,7 @@ static int rvin_setup(struct rvin_dev *vin)
/*
* Input interface
*/
- switch (vin->digital.code) {
+ switch (vin->digital->code) {
case MEDIA_BUS_FMT_YUYV8_1X16:
/* BT.601/BT.1358 16bit YCbCr422 */
vnmc |= VNMC_INF_YUV16;
@@ -191,7 +191,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
input_is_yuv = true;
break;
@@ -200,7 +200,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY10_2X10:
/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
input_is_yuv = true;
break;
@@ -212,11 +212,11 @@ static int rvin_setup(struct rvin_dev *vin)
dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
/* Hsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+ if (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_HPS;
/* Vsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+ if (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_VPS;
/*
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index dd37ea811680..b479b882da12 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -111,7 +111,7 @@ static int rvin_reset_format(struct rvin_dev *vin)
struct v4l2_mbus_framefmt *mf = &fmt.format;
int ret;
- fmt.pad = vin->digital.source_pad;
+ fmt.pad = vin->digital->source_pad;
ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
if (ret)
@@ -172,13 +172,13 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
sd = vin_to_source(vin);
- v4l2_fill_mbus_format(&format.format, pix, vin->digital.code);
+ v4l2_fill_mbus_format(&format.format, pix, vin->digital->code);
pad_cfg = v4l2_subdev_alloc_pad_config(sd);
if (pad_cfg == NULL)
return -ENOMEM;
- format.pad = vin->digital.source_pad;
+ format.pad = vin->digital->source_pad;
field = pix->field;
@@ -555,7 +555,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
if (timings->pad)
return -EINVAL;
- timings->pad = vin->digital.sink_pad;
+ timings->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
@@ -607,7 +607,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
if (cap->pad)
return -EINVAL;
- cap->pad = vin->digital.sink_pad;
+ cap->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
@@ -625,7 +625,7 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
if (edid->pad)
return -EINVAL;
- edid->pad = vin->digital.sink_pad;
+ edid->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, get_edid, edid);
@@ -643,7 +643,7 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
if (edid->pad)
return -EINVAL;
- edid->pad = vin->digital.sink_pad;
+ edid->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, set_edid, edid);
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 9bfb5a7c4dc4..5382078143fb 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -126,7 +126,7 @@ struct rvin_dev {
struct v4l2_device v4l2_dev;
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_async_notifier notifier;
- struct rvin_graph_entity digital;
+ struct rvin_graph_entity *digital;
struct mutex lock;
struct vb2_queue queue;
@@ -145,7 +145,7 @@ struct rvin_dev {
struct v4l2_rect compose;
};
-#define vin_to_source(vin) vin->digital.subdev
+#define vin_to_source(vin) ((vin)->digital->subdev)
/* Debug */
#define vin_dbg(d, fmt, arg...) dev_dbg(d->dev, fmt, ##arg)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 07/24] rcar-vin: Use generic parser for parsing fwnode endpoints
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Instead of using driver implementation, use
v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
of the device.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/rcar-vin/rcar-core.c | 112 +++++++++-------------------
drivers/media/platform/rcar-vin/rcar-dma.c | 10 +--
drivers/media/platform/rcar-vin/rcar-v4l2.c | 14 ++--
drivers/media/platform/rcar-vin/rcar-vin.h | 4 +-
4 files changed, 48 insertions(+), 92 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 142de447aaaa..62b4a94f9a39 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -21,6 +21,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-fwnode.h>
#include "rcar-vin.h"
@@ -77,14 +78,14 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
int ret;
/* Verify subdevices mbus format */
- if (!rvin_mbus_supported(&vin->digital)) {
+ if (!rvin_mbus_supported(vin->digital)) {
vin_err(vin, "Unsupported media bus format for %s\n",
- vin->digital.subdev->name);
+ vin->digital->subdev->name);
return -EINVAL;
}
vin_dbg(vin, "Found media bus format for %s: %d\n",
- vin->digital.subdev->name, vin->digital.code);
+ vin->digital->subdev->name, vin->digital->code);
ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
if (ret < 0) {
@@ -103,7 +104,7 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
rvin_v4l2_remove(vin);
- vin->digital.subdev = NULL;
+ vin->digital->subdev = NULL;
}
static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
@@ -120,117 +121,70 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
if (ret < 0)
return ret;
- vin->digital.source_pad = ret;
+ vin->digital->source_pad = ret;
ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
- vin->digital.sink_pad = ret < 0 ? 0 : ret;
+ vin->digital->sink_pad = ret < 0 ? 0 : ret;
- vin->digital.subdev = subdev;
+ vin->digital->subdev = subdev;
vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
- subdev->name, vin->digital.source_pad,
- vin->digital.sink_pad);
+ subdev->name, vin->digital->source_pad,
+ vin->digital->sink_pad);
return 0;
}
-static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,
- struct device_node *ep,
- struct v4l2_mbus_config *mbus_cfg)
+static int rvin_digital_parse_v4l2(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
- struct v4l2_fwnode_endpoint v4l2_ep;
- int ret;
+ struct rvin_dev *vin = dev_get_drvdata(dev);
+ struct rvin_graph_entity *rvge =
+ container_of(asd, struct rvin_graph_entity, asd);
- ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
- if (ret) {
- vin_err(vin, "Could not parse v4l2 endpoint\n");
- return -EINVAL;
- }
+ if (vep->base.port || vep->base.id)
+ return -ENOTCONN;
- mbus_cfg->type = v4l2_ep.bus_type;
+ rvge->mbus_cfg.type = vep->bus_type;
- switch (mbus_cfg->type) {
+ switch (rvge->mbus_cfg.type) {
case V4L2_MBUS_PARALLEL:
vin_dbg(vin, "Found PARALLEL media bus\n");
- mbus_cfg->flags = v4l2_ep.bus.parallel.flags;
+ rvge->mbus_cfg.flags = vep->bus.parallel.flags;
break;
case V4L2_MBUS_BT656:
vin_dbg(vin, "Found BT656 media bus\n");
- mbus_cfg->flags = 0;
+ rvge->mbus_cfg.flags = 0;
break;
default:
vin_err(vin, "Unknown media bus type\n");
return -EINVAL;
}
- return 0;
-}
-
-static int rvin_digital_graph_parse(struct rvin_dev *vin)
-{
- struct device_node *ep, *np;
- int ret;
-
- vin->digital.asd.match.fwnode.fwnode = NULL;
- vin->digital.subdev = NULL;
-
- /*
- * Port 0 id 0 is local digital input, try to get it.
- * Not all instances can or will have this, that is OK
- */
- ep = of_graph_get_endpoint_by_regs(vin->dev->of_node, 0, 0);
- if (!ep)
- return 0;
-
- np = of_graph_get_remote_port_parent(ep);
- if (!np) {
- vin_err(vin, "No remote parent for digital input\n");
- of_node_put(ep);
- return -EINVAL;
- }
- of_node_put(np);
-
- ret = rvin_digitial_parse_v4l2(vin, ep, &vin->digital.mbus_cfg);
- of_node_put(ep);
- if (ret)
- return ret;
-
- vin->digital.asd.match.fwnode.fwnode = of_fwnode_handle(np);
- vin->digital.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
+ vin->digital = rvge;
return 0;
}
static int rvin_digital_graph_init(struct rvin_dev *vin)
{
- struct v4l2_async_subdev **subdevs = NULL;
int ret;
- ret = rvin_digital_graph_parse(vin);
+ ret = v4l2_async_notifier_parse_fwnode_endpoints(
+ vin->dev, &vin->notifier,
+ sizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);
if (ret)
return ret;
- if (!vin->digital.asd.match.fwnode.fwnode) {
- vin_dbg(vin, "No digital subdevice found\n");
- return -ENODEV;
- }
-
- /* Register the subdevices notifier. */
- subdevs = devm_kzalloc(vin->dev, sizeof(*subdevs), GFP_KERNEL);
- if (subdevs == NULL)
- return -ENOMEM;
-
- subdevs[0] = &vin->digital.asd;
-
- vin_dbg(vin, "Found digital subdevice %pOF\n",
- to_of_node(subdevs[0]->match.fwnode.fwnode));
+ if (vin->digital)
+ vin_dbg(vin, "Found digital subdevice %pOF\n",
+ to_of_node(
+ vin->digital->asd.match.fwnode.fwnode));
- vin->notifier.num_subdevs = 1;
- vin->notifier.subdevs = subdevs;
vin->notifier.bound = rvin_digital_notify_bound;
vin->notifier.unbind = rvin_digital_notify_unbind;
vin->notifier.complete = rvin_digital_notify_complete;
-
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
@@ -290,6 +244,8 @@ static int rcar_vin_probe(struct platform_device *pdev)
if (ret)
return ret;
+ platform_set_drvdata(pdev, vin);
+
ret = rvin_digital_graph_init(vin);
if (ret < 0)
goto error;
@@ -297,11 +253,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
pm_suspend_ignore_children(&pdev->dev, true);
pm_runtime_enable(&pdev->dev);
- platform_set_drvdata(pdev, vin);
-
return 0;
error:
rvin_dma_remove(vin);
+ v4l2_async_notifier_release(&vin->notifier);
return ret;
}
@@ -313,6 +268,7 @@ static int rcar_vin_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
v4l2_async_notifier_unregister(&vin->notifier);
+ v4l2_async_notifier_release(&vin->notifier);
rvin_dma_remove(vin);
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index b136844499f6..23fdff7a7370 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -183,7 +183,7 @@ static int rvin_setup(struct rvin_dev *vin)
/*
* Input interface
*/
- switch (vin->digital.code) {
+ switch (vin->digital->code) {
case MEDIA_BUS_FMT_YUYV8_1X16:
/* BT.601/BT.1358 16bit YCbCr422 */
vnmc |= VNMC_INF_YUV16;
@@ -191,7 +191,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
input_is_yuv = true;
break;
@@ -200,7 +200,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY10_2X10:
/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->digital->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
input_is_yuv = true;
break;
@@ -212,11 +212,11 @@ static int rvin_setup(struct rvin_dev *vin)
dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
/* Hsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+ if (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_HPS;
/* Vsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+ if (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_VPS;
/*
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index dd37ea811680..b479b882da12 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -111,7 +111,7 @@ static int rvin_reset_format(struct rvin_dev *vin)
struct v4l2_mbus_framefmt *mf = &fmt.format;
int ret;
- fmt.pad = vin->digital.source_pad;
+ fmt.pad = vin->digital->source_pad;
ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
if (ret)
@@ -172,13 +172,13 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
sd = vin_to_source(vin);
- v4l2_fill_mbus_format(&format.format, pix, vin->digital.code);
+ v4l2_fill_mbus_format(&format.format, pix, vin->digital->code);
pad_cfg = v4l2_subdev_alloc_pad_config(sd);
if (pad_cfg == NULL)
return -ENOMEM;
- format.pad = vin->digital.source_pad;
+ format.pad = vin->digital->source_pad;
field = pix->field;
@@ -555,7 +555,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
if (timings->pad)
return -EINVAL;
- timings->pad = vin->digital.sink_pad;
+ timings->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
@@ -607,7 +607,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
if (cap->pad)
return -EINVAL;
- cap->pad = vin->digital.sink_pad;
+ cap->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
@@ -625,7 +625,7 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
if (edid->pad)
return -EINVAL;
- edid->pad = vin->digital.sink_pad;
+ edid->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, get_edid, edid);
@@ -643,7 +643,7 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
if (edid->pad)
return -EINVAL;
- edid->pad = vin->digital.sink_pad;
+ edid->pad = vin->digital->sink_pad;
ret = v4l2_subdev_call(sd, pad, set_edid, edid);
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 9bfb5a7c4dc4..5382078143fb 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -126,7 +126,7 @@ struct rvin_dev {
struct v4l2_device v4l2_dev;
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_async_notifier notifier;
- struct rvin_graph_entity digital;
+ struct rvin_graph_entity *digital;
struct mutex lock;
struct vb2_queue queue;
@@ -145,7 +145,7 @@ struct rvin_dev {
struct v4l2_rect compose;
};
-#define vin_to_source(vin) vin->digital.subdev
+#define vin_to_source(vin) ((vin)->digital->subdev)
/* Debug */
#define vin_dbg(d, fmt, arg...) dev_dbg(d->dev, fmt, ##arg)
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 10/24] v4l: async: Move async subdev notifier operations to a separate structure
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
From: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
The async subdev notifier .bound(), .unbind() and .complete() operations
are function pointers stored directly in the v4l2_async_subdev
structure. As the structure isn't immutable, this creates a potential
security risk as the function pointers are mutable.
To fix this, move the function pointers to a new
v4l2_async_subdev_operations structure that can be made const in
drivers.
Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
---
drivers/media/platform/am437x/am437x-vpfe.c | 8 +++++--
drivers/media/platform/atmel/atmel-isc.c | 10 ++++++---
drivers/media/platform/atmel/atmel-isi.c | 10 ++++++---
drivers/media/platform/davinci/vpif_capture.c | 8 +++++--
drivers/media/platform/davinci/vpif_display.c | 8 +++++--
drivers/media/platform/exynos4-is/media-dev.c | 8 +++++--
drivers/media/platform/omap3isp/isp.c | 6 +++++-
drivers/media/platform/pxa_camera.c | 8 +++++--
drivers/media/platform/qcom/camss-8x16/camss.c | 8 +++++--
drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++---
drivers/media/platform/rcar_drif.c | 10 ++++++---
drivers/media/platform/soc_camera/soc_camera.c | 14 +++++++------
drivers/media/platform/stm32/stm32-dcmi.c | 10 ++++++---
drivers/media/platform/ti-vpe/cal.c | 8 +++++--
drivers/media/platform/xilinx/xilinx-vipp.c | 8 +++++--
drivers/media/v4l2-core/v4l2-async.c | 20 +++++++++---------
drivers/staging/media/imx/imx-media-dev.c | 8 +++++--
include/media/v4l2-async.h | 29 +++++++++++++++++---------
18 files changed, 131 insertions(+), 60 deletions(-)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index dfcc484cab89..0997c640191d 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2417,6 +2417,11 @@ static int vpfe_async_complete(struct v4l2_async_notifier *notifier)
return vpfe_probe_complete(vpfe);
}
+static const struct v4l2_async_notifier_operations vpfe_async_ops = {
+ .bound = vpfe_async_bound,
+ .complete = vpfe_async_complete,
+};
+
static struct vpfe_config *
vpfe_get_pdata(struct platform_device *pdev)
{
@@ -2590,8 +2595,7 @@ static int vpfe_probe(struct platform_device *pdev)
vpfe->notifier.subdevs = vpfe->cfg->asd;
vpfe->notifier.num_subdevs = ARRAY_SIZE(vpfe->cfg->asd);
- vpfe->notifier.bound = vpfe_async_bound;
- vpfe->notifier.complete = vpfe_async_complete;
+ vpfe->notifier.ops = &vpfe_async_ops;
ret = v4l2_async_notifier_register(&vpfe->v4l2_dev,
&vpfe->notifier);
if (ret) {
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index d7103c5f92c3..48544c4137cb 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1639,6 +1639,12 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations isc_async_ops = {
+ .bound = isc_async_bound,
+ .unbind = isc_async_unbind,
+ .complete = isc_async_complete,
+};
+
static void isc_subdev_cleanup(struct isc_device *isc)
{
struct isc_subdev_entity *subdev_entity;
@@ -1851,9 +1857,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
list_for_each_entry(subdev_entity, &isc->subdev_entities, list) {
subdev_entity->notifier.subdevs = &subdev_entity->asd;
subdev_entity->notifier.num_subdevs = 1;
- subdev_entity->notifier.bound = isc_async_bound;
- subdev_entity->notifier.unbind = isc_async_unbind;
- subdev_entity->notifier.complete = isc_async_complete;
+ subdev_entity->notifier.ops = &isc_async_ops;
ret = v4l2_async_notifier_register(&isc->v4l2_dev,
&subdev_entity->notifier);
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..eadbf9def358 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1105,6 +1105,12 @@ static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations isi_graph_notify_ops = {
+ .bound = isi_graph_notify_bound,
+ .unbind = isi_graph_notify_unbind,
+ .complete = isi_graph_notify_complete,
+};
+
static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
{
struct device_node *ep = NULL;
@@ -1152,9 +1158,7 @@ static int isi_graph_init(struct atmel_isi *isi)
isi->notifier.subdevs = subdevs;
isi->notifier.num_subdevs = 1;
- isi->notifier.bound = isi_graph_notify_bound;
- isi->notifier.unbind = isi_graph_notify_unbind;
- isi->notifier.complete = isi_graph_notify_complete;
+ isi->notifier.ops = &isi_graph_notify_ops;
ret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);
if (ret < 0) {
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 0ef36cec21d1..a89367ab1e06 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1500,6 +1500,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
return vpif_probe_complete();
}
+static const struct v4l2_async_notifier_operations vpif_async_ops = {
+ .bound = vpif_async_bound,
+ .complete = vpif_async_complete,
+};
+
static struct vpif_capture_config *
vpif_capture_get_pdata(struct platform_device *pdev)
{
@@ -1691,8 +1696,7 @@ static __init int vpif_probe(struct platform_device *pdev)
} else {
vpif_obj.notifier.subdevs = vpif_obj.config->asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
- vpif_obj.notifier.bound = vpif_async_bound;
- vpif_obj.notifier.complete = vpif_async_complete;
+ vpif_obj.notifier.ops = &vpif_async_ops;
err = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,
&vpif_obj.notifier);
if (err) {
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 56fe4e5b396e..ff2f75a328c9 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1232,6 +1232,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
return vpif_probe_complete();
}
+static const struct v4l2_async_notifier_operations vpif_async_ops = {
+ .bound = vpif_async_bound,
+ .complete = vpif_async_complete,
+};
+
/*
* vpif_probe: This function creates device entries by register itself to the
* V4L2 driver and initializes fields of each channel objects
@@ -1313,8 +1318,7 @@ static __init int vpif_probe(struct platform_device *pdev)
} else {
vpif_obj.notifier.subdevs = vpif_obj.config->asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
- vpif_obj.notifier.bound = vpif_async_bound;
- vpif_obj.notifier.complete = vpif_async_complete;
+ vpif_obj.notifier.ops = &vpif_async_ops;
err = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,
&vpif_obj.notifier);
if (err) {
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index d4656d5175d7..c15596b56dc9 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1405,6 +1405,11 @@ static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
return media_device_register(&fmd->media_dev);
}
+static const struct v4l2_async_notifier_operations subdev_notifier_ops = {
+ .bound = subdev_notifier_bound,
+ .complete = subdev_notifier_complete,
+};
+
static int fimc_md_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1479,8 +1484,7 @@ static int fimc_md_probe(struct platform_device *pdev)
if (fmd->num_sensors > 0) {
fmd->subdev_notifier.subdevs = fmd->async_subdevs;
fmd->subdev_notifier.num_subdevs = fmd->num_sensors;
- fmd->subdev_notifier.bound = subdev_notifier_bound;
- fmd->subdev_notifier.complete = subdev_notifier_complete;
+ fmd->subdev_notifier.ops = &subdev_notifier_ops;
fmd->num_sensors = 0;
ret = v4l2_async_notifier_register(&fmd->v4l2_dev,
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 9a694924e46e..0f08d602f756 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2171,6 +2171,10 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return media_device_register(&isp->media_dev);
}
+static const struct v4l2_async_notifier_operations isp_subdev_notifier_ops = {
+ .complete = isp_subdev_notifier_complete,
+};
+
/*
* isp_probe - Probe ISP platform device
* @pdev: Pointer to ISP platform device
@@ -2341,7 +2345,7 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.complete = isp_subdev_notifier_complete;
+ isp->notifier.ops = &isp_subdev_notifier_ops;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
if (ret)
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index edca993c2b1f..9d3f0cb1d95a 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2221,6 +2221,11 @@ static void pxa_camera_sensor_unbind(struct v4l2_async_notifier *notifier,
mutex_unlock(&pcdev->mlock);
}
+static const struct v4l2_async_notifier_operations pxa_camera_sensor_ops = {
+ .bound = pxa_camera_sensor_bound,
+ .unbind = pxa_camera_sensor_unbind,
+};
+
/*
* Driver probe, remove, suspend and resume operations
*/
@@ -2489,8 +2494,7 @@ static int pxa_camera_probe(struct platform_device *pdev)
pcdev->asds[0] = &pcdev->asd;
pcdev->notifier.subdevs = pcdev->asds;
pcdev->notifier.num_subdevs = 1;
- pcdev->notifier.bound = pxa_camera_sensor_bound;
- pcdev->notifier.unbind = pxa_camera_sensor_unbind;
+ pcdev->notifier.ops = &pxa_camera_sensor_ops;
if (!of_have_populated_dt())
pcdev->asd.match_type = V4L2_ASYNC_MATCH_I2C;
diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c b/drivers/media/platform/qcom/camss-8x16/camss.c
index a3760b5dd1d1..390a42c17b66 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss.c
@@ -601,6 +601,11 @@ static int camss_subdev_notifier_complete(struct v4l2_async_notifier *async)
return media_device_register(&camss->media_dev);
}
+static const struct v4l2_async_notifier_operations camss_subdev_notifier_ops = {
+ .bound = camss_subdev_notifier_bound,
+ .complete = camss_subdev_notifier_complete,
+};
+
static const struct media_device_ops camss_media_ops = {
.link_notify = v4l2_pipeline_link_notify,
};
@@ -655,8 +660,7 @@ static int camss_probe(struct platform_device *pdev)
goto err_register_entities;
if (camss->notifier.num_subdevs) {
- camss->notifier.bound = camss_subdev_notifier_bound;
- camss->notifier.complete = camss_subdev_notifier_complete;
+ camss->notifier.ops = &camss_subdev_notifier_ops;
ret = v4l2_async_notifier_register(&camss->v4l2_dev,
&camss->notifier);
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 62b4a94f9a39..55acab4f2905 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -134,6 +134,12 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations rvin_digital_notify_ops = {
+ .bound = rvin_digital_notify_bound,
+ .unbind = rvin_digital_notify_unbind,
+ .complete = rvin_digital_notify_complete,
+};
+
static int rvin_digital_parse_v4l2(struct device *dev,
struct v4l2_fwnode_endpoint *vep,
@@ -182,9 +188,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
to_of_node(
vin->digital->asd.match.fwnode.fwnode));
- vin->notifier.bound = rvin_digital_notify_bound;
- vin->notifier.unbind = rvin_digital_notify_unbind;
- vin->notifier.complete = rvin_digital_notify_complete;
+ vin->notifier.ops = &rvin_digital_notify_ops;
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index 522364ff0d5d..0b2214d6d621 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -1185,6 +1185,12 @@ static int rcar_drif_notify_complete(struct v4l2_async_notifier *notifier)
return ret;
}
+static const struct v4l2_async_notifier_operations rcar_drif_notify_ops = {
+ .bound = rcar_drif_notify_bound,
+ .unbind = rcar_drif_notify_unbind,
+ .complete = rcar_drif_notify_complete,
+};
+
/* Read endpoint properties */
static void rcar_drif_get_ep_properties(struct rcar_drif_sdr *sdr,
struct fwnode_handle *fwnode)
@@ -1347,9 +1353,7 @@ static int rcar_drif_sdr_probe(struct rcar_drif_sdr *sdr)
if (ret)
goto error;
- sdr->notifier.bound = rcar_drif_notify_bound;
- sdr->notifier.unbind = rcar_drif_notify_unbind;
- sdr->notifier.complete = rcar_drif_notify_complete;
+ sdr->notifier.ops = &rcar_drif_notify_ops;
/* Register notifier */
ret = v4l2_async_notifier_register(&sdr->v4l2_dev, &sdr->notifier);
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 1f3c450c7a69..916ff68b73d4 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1391,6 +1391,12 @@ static int soc_camera_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations soc_camera_async_ops = {
+ .bound = soc_camera_async_bound,
+ .unbind = soc_camera_async_unbind,
+ .complete = soc_camera_async_complete,
+};
+
static int scan_async_group(struct soc_camera_host *ici,
struct v4l2_async_subdev **asd, unsigned int size)
{
@@ -1437,9 +1443,7 @@ static int scan_async_group(struct soc_camera_host *ici,
sasc->notifier.subdevs = asd;
sasc->notifier.num_subdevs = size;
- sasc->notifier.bound = soc_camera_async_bound;
- sasc->notifier.unbind = soc_camera_async_unbind;
- sasc->notifier.complete = soc_camera_async_complete;
+ sasc->notifier.ops = &soc_camera_async_ops;
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
@@ -1537,9 +1541,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
sasc->notifier.subdevs = &info->subdev;
sasc->notifier.num_subdevs = 1;
- sasc->notifier.bound = soc_camera_async_bound;
- sasc->notifier.unbind = soc_camera_async_unbind;
- sasc->notifier.complete = soc_camera_async_complete;
+ sasc->notifier.ops = &soc_camera_async_ops;
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 35ba6f211b79..ac4c450a6c7d 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1495,6 +1495,12 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = {
+ .bound = dcmi_graph_notify_bound,
+ .unbind = dcmi_graph_notify_unbind,
+ .complete = dcmi_graph_notify_complete,
+};
+
static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
{
struct device_node *ep = NULL;
@@ -1542,9 +1548,7 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
dcmi->notifier.subdevs = subdevs;
dcmi->notifier.num_subdevs = 1;
- dcmi->notifier.bound = dcmi_graph_notify_bound;
- dcmi->notifier.unbind = dcmi_graph_notify_unbind;
- dcmi->notifier.complete = dcmi_graph_notify_complete;
+ dcmi->notifier.ops = &dcmi_graph_notify_ops;
ret = v4l2_async_notifier_register(&dcmi->v4l2_dev, &dcmi->notifier);
if (ret < 0) {
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 42e383a48ffe..8b586c864524 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1522,6 +1522,11 @@ static int cal_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations cal_async_ops = {
+ .bound = cal_async_bound,
+ .complete = cal_async_complete,
+};
+
static int cal_complete_ctx(struct cal_ctx *ctx)
{
struct video_device *vfd;
@@ -1736,8 +1741,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
ctx->asd_list[0] = asd;
ctx->notifier.subdevs = ctx->asd_list;
ctx->notifier.num_subdevs = 1;
- ctx->notifier.bound = cal_async_bound;
- ctx->notifier.complete = cal_async_complete;
+ ctx->notifier.ops = &cal_async_ops;
ret = v4l2_async_notifier_register(&ctx->v4l2_dev,
&ctx->notifier);
if (ret) {
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index ebfdf334d99c..d881cf09876d 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -351,6 +351,11 @@ static int xvip_graph_notify_bound(struct v4l2_async_notifier *notifier,
return -EINVAL;
}
+static const struct v4l2_async_notifier_operations xvip_graph_notify_ops = {
+ .bound = xvip_graph_notify_bound,
+ .complete = xvip_graph_notify_complete,
+};
+
static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
struct device_node *node)
{
@@ -548,8 +553,7 @@ static int xvip_graph_init(struct xvip_composite_device *xdev)
xdev->notifier.subdevs = subdevs;
xdev->notifier.num_subdevs = num_subdevs;
- xdev->notifier.bound = xvip_graph_notify_bound;
- xdev->notifier.complete = xvip_graph_notify_complete;
+ xdev->notifier.ops = &xvip_graph_notify_ops;
ret = v4l2_async_notifier_register(&xdev->v4l2_dev, &xdev->notifier);
if (ret < 0) {
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index ad099b9bb71d..a2df85ea00f4 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -102,16 +102,16 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
{
int ret;
- if (notifier->bound) {
- ret = notifier->bound(notifier, sd, asd);
+ if (notifier->ops->bound) {
+ ret = notifier->ops->bound(notifier, sd, asd);
if (ret < 0)
return ret;
}
ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
if (ret < 0) {
- if (notifier->unbind)
- notifier->unbind(notifier, sd, asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, asd);
return ret;
}
@@ -123,8 +123,8 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
/* Move from the global subdevice list to notifier's done */
list_move(&sd->async_list, ¬ifier->done);
- if (list_empty(¬ifier->waiting) && notifier->complete)
- return notifier->complete(notifier);
+ if (list_empty(¬ifier->waiting) && notifier->ops->complete)
+ return notifier->ops->complete(notifier);
return 0;
}
@@ -210,8 +210,8 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
v4l2_async_cleanup(sd);
- if (notifier->unbind)
- notifier->unbind(notifier, sd, sd->asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, sd->asd);
}
mutex_unlock(&list_lock);
@@ -300,8 +300,8 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
v4l2_async_cleanup(sd);
- if (notifier->unbind)
- notifier->unbind(notifier, sd, sd->asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, sd->asd);
mutex_unlock(&list_lock);
}
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index d96f4512224f..e8dc2c93658a 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -440,6 +440,11 @@ static int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
return media_device_register(&imxmd->md);
}
+static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {
+ .bound = imx_media_subdev_bound,
+ .complete = imx_media_probe_complete,
+};
+
/*
* adds controls to a video device from an entity subdevice.
* Continues upstream from the entity's sink pads.
@@ -608,8 +613,7 @@ static int imx_media_probe(struct platform_device *pdev)
/* prepare the async subdev notifier and register it */
imxmd->subdev_notifier.subdevs = imxmd->async_ptrs;
- imxmd->subdev_notifier.bound = imx_media_subdev_bound;
- imxmd->subdev_notifier.complete = imx_media_probe_complete;
+ imxmd->subdev_notifier.ops = &imx_media_subdev_ops;
ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
&imxmd->subdev_notifier);
if (ret) {
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 96fa1afc00dd..3c48f8b66d12 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -18,6 +18,7 @@ struct device;
struct device_node;
struct v4l2_device;
struct v4l2_subdev;
+struct v4l2_async_notifier;
/* A random max subdevice number, used to allocate an array on stack */
#define V4L2_MAX_SUBDEVS 128U
@@ -79,8 +80,25 @@ struct v4l2_async_subdev {
};
/**
+ * struct v4l2_async_notifier_operations - Asynchronous V4L2 notifier operations
+ * @bound: a subdevice driver has successfully probed one of the subdevices
+ * @complete: all subdevices have been probed successfully
+ * @unbind: a subdevice is leaving
+ */
+struct v4l2_async_notifier_operations {
+ int (*bound)(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd);
+ int (*complete)(struct v4l2_async_notifier *notifier);
+ void (*unbind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd);
+};
+
+/**
* struct v4l2_async_notifier - v4l2_device notifier data
*
+ * @ops: notifier operations
* @num_subdevs: number of subdevices used in the subdevs array
* @max_subdevs: number of subdevices allocated in the subdevs array
* @subdevs: array of pointers to subdevice descriptors
@@ -88,11 +106,9 @@ struct v4l2_async_subdev {
* @waiting: list of struct v4l2_async_subdev, waiting for their drivers
* @done: list of struct v4l2_subdev, already probed
* @list: member in a global list of notifiers
- * @bound: a subdevice driver has successfully probed one of subdevices
- * @complete: all subdevices have been probed successfully
- * @unbind: a subdevice is leaving
*/
struct v4l2_async_notifier {
+ const struct v4l2_async_notifier_operations *ops;
unsigned int num_subdevs;
unsigned int max_subdevs;
struct v4l2_async_subdev **subdevs;
@@ -100,13 +116,6 @@ struct v4l2_async_notifier {
struct list_head waiting;
struct list_head done;
struct list_head list;
- int (*bound)(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd);
- int (*complete)(struct v4l2_async_notifier *notifier);
- void (*unbind)(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd);
};
/**
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 10/24] v4l: async: Move async subdev notifier operations to a separate structure
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The async subdev notifier .bound(), .unbind() and .complete() operations
are function pointers stored directly in the v4l2_async_subdev
structure. As the structure isn't immutable, this creates a potential
security risk as the function pointers are mutable.
To fix this, move the function pointers to a new
v4l2_async_subdev_operations structure that can be made const in
drivers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/am437x/am437x-vpfe.c | 8 +++++--
drivers/media/platform/atmel/atmel-isc.c | 10 ++++++---
drivers/media/platform/atmel/atmel-isi.c | 10 ++++++---
drivers/media/platform/davinci/vpif_capture.c | 8 +++++--
drivers/media/platform/davinci/vpif_display.c | 8 +++++--
drivers/media/platform/exynos4-is/media-dev.c | 8 +++++--
drivers/media/platform/omap3isp/isp.c | 6 +++++-
drivers/media/platform/pxa_camera.c | 8 +++++--
drivers/media/platform/qcom/camss-8x16/camss.c | 8 +++++--
drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++---
drivers/media/platform/rcar_drif.c | 10 ++++++---
drivers/media/platform/soc_camera/soc_camera.c | 14 +++++++------
drivers/media/platform/stm32/stm32-dcmi.c | 10 ++++++---
drivers/media/platform/ti-vpe/cal.c | 8 +++++--
drivers/media/platform/xilinx/xilinx-vipp.c | 8 +++++--
drivers/media/v4l2-core/v4l2-async.c | 20 +++++++++---------
drivers/staging/media/imx/imx-media-dev.c | 8 +++++--
include/media/v4l2-async.h | 29 +++++++++++++++++---------
18 files changed, 131 insertions(+), 60 deletions(-)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index dfcc484cab89..0997c640191d 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2417,6 +2417,11 @@ static int vpfe_async_complete(struct v4l2_async_notifier *notifier)
return vpfe_probe_complete(vpfe);
}
+static const struct v4l2_async_notifier_operations vpfe_async_ops = {
+ .bound = vpfe_async_bound,
+ .complete = vpfe_async_complete,
+};
+
static struct vpfe_config *
vpfe_get_pdata(struct platform_device *pdev)
{
@@ -2590,8 +2595,7 @@ static int vpfe_probe(struct platform_device *pdev)
vpfe->notifier.subdevs = vpfe->cfg->asd;
vpfe->notifier.num_subdevs = ARRAY_SIZE(vpfe->cfg->asd);
- vpfe->notifier.bound = vpfe_async_bound;
- vpfe->notifier.complete = vpfe_async_complete;
+ vpfe->notifier.ops = &vpfe_async_ops;
ret = v4l2_async_notifier_register(&vpfe->v4l2_dev,
&vpfe->notifier);
if (ret) {
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index d7103c5f92c3..48544c4137cb 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1639,6 +1639,12 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations isc_async_ops = {
+ .bound = isc_async_bound,
+ .unbind = isc_async_unbind,
+ .complete = isc_async_complete,
+};
+
static void isc_subdev_cleanup(struct isc_device *isc)
{
struct isc_subdev_entity *subdev_entity;
@@ -1851,9 +1857,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
list_for_each_entry(subdev_entity, &isc->subdev_entities, list) {
subdev_entity->notifier.subdevs = &subdev_entity->asd;
subdev_entity->notifier.num_subdevs = 1;
- subdev_entity->notifier.bound = isc_async_bound;
- subdev_entity->notifier.unbind = isc_async_unbind;
- subdev_entity->notifier.complete = isc_async_complete;
+ subdev_entity->notifier.ops = &isc_async_ops;
ret = v4l2_async_notifier_register(&isc->v4l2_dev,
&subdev_entity->notifier);
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..eadbf9def358 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1105,6 +1105,12 @@ static int isi_graph_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations isi_graph_notify_ops = {
+ .bound = isi_graph_notify_bound,
+ .unbind = isi_graph_notify_unbind,
+ .complete = isi_graph_notify_complete,
+};
+
static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
{
struct device_node *ep = NULL;
@@ -1152,9 +1158,7 @@ static int isi_graph_init(struct atmel_isi *isi)
isi->notifier.subdevs = subdevs;
isi->notifier.num_subdevs = 1;
- isi->notifier.bound = isi_graph_notify_bound;
- isi->notifier.unbind = isi_graph_notify_unbind;
- isi->notifier.complete = isi_graph_notify_complete;
+ isi->notifier.ops = &isi_graph_notify_ops;
ret = v4l2_async_notifier_register(&isi->v4l2_dev, &isi->notifier);
if (ret < 0) {
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 0ef36cec21d1..a89367ab1e06 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1500,6 +1500,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
return vpif_probe_complete();
}
+static const struct v4l2_async_notifier_operations vpif_async_ops = {
+ .bound = vpif_async_bound,
+ .complete = vpif_async_complete,
+};
+
static struct vpif_capture_config *
vpif_capture_get_pdata(struct platform_device *pdev)
{
@@ -1691,8 +1696,7 @@ static __init int vpif_probe(struct platform_device *pdev)
} else {
vpif_obj.notifier.subdevs = vpif_obj.config->asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
- vpif_obj.notifier.bound = vpif_async_bound;
- vpif_obj.notifier.complete = vpif_async_complete;
+ vpif_obj.notifier.ops = &vpif_async_ops;
err = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,
&vpif_obj.notifier);
if (err) {
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 56fe4e5b396e..ff2f75a328c9 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1232,6 +1232,11 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
return vpif_probe_complete();
}
+static const struct v4l2_async_notifier_operations vpif_async_ops = {
+ .bound = vpif_async_bound,
+ .complete = vpif_async_complete,
+};
+
/*
* vpif_probe: This function creates device entries by register itself to the
* V4L2 driver and initializes fields of each channel objects
@@ -1313,8 +1318,7 @@ static __init int vpif_probe(struct platform_device *pdev)
} else {
vpif_obj.notifier.subdevs = vpif_obj.config->asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
- vpif_obj.notifier.bound = vpif_async_bound;
- vpif_obj.notifier.complete = vpif_async_complete;
+ vpif_obj.notifier.ops = &vpif_async_ops;
err = v4l2_async_notifier_register(&vpif_obj.v4l2_dev,
&vpif_obj.notifier);
if (err) {
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index d4656d5175d7..c15596b56dc9 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1405,6 +1405,11 @@ static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
return media_device_register(&fmd->media_dev);
}
+static const struct v4l2_async_notifier_operations subdev_notifier_ops = {
+ .bound = subdev_notifier_bound,
+ .complete = subdev_notifier_complete,
+};
+
static int fimc_md_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1479,8 +1484,7 @@ static int fimc_md_probe(struct platform_device *pdev)
if (fmd->num_sensors > 0) {
fmd->subdev_notifier.subdevs = fmd->async_subdevs;
fmd->subdev_notifier.num_subdevs = fmd->num_sensors;
- fmd->subdev_notifier.bound = subdev_notifier_bound;
- fmd->subdev_notifier.complete = subdev_notifier_complete;
+ fmd->subdev_notifier.ops = &subdev_notifier_ops;
fmd->num_sensors = 0;
ret = v4l2_async_notifier_register(&fmd->v4l2_dev,
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 9a694924e46e..0f08d602f756 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2171,6 +2171,10 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
return media_device_register(&isp->media_dev);
}
+static const struct v4l2_async_notifier_operations isp_subdev_notifier_ops = {
+ .complete = isp_subdev_notifier_complete,
+};
+
/*
* isp_probe - Probe ISP platform device
* @pdev: Pointer to ISP platform device
@@ -2341,7 +2345,7 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
- isp->notifier.complete = isp_subdev_notifier_complete;
+ isp->notifier.ops = &isp_subdev_notifier_ops;
ret = v4l2_async_notifier_register(&isp->v4l2_dev, &isp->notifier);
if (ret)
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index edca993c2b1f..9d3f0cb1d95a 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2221,6 +2221,11 @@ static void pxa_camera_sensor_unbind(struct v4l2_async_notifier *notifier,
mutex_unlock(&pcdev->mlock);
}
+static const struct v4l2_async_notifier_operations pxa_camera_sensor_ops = {
+ .bound = pxa_camera_sensor_bound,
+ .unbind = pxa_camera_sensor_unbind,
+};
+
/*
* Driver probe, remove, suspend and resume operations
*/
@@ -2489,8 +2494,7 @@ static int pxa_camera_probe(struct platform_device *pdev)
pcdev->asds[0] = &pcdev->asd;
pcdev->notifier.subdevs = pcdev->asds;
pcdev->notifier.num_subdevs = 1;
- pcdev->notifier.bound = pxa_camera_sensor_bound;
- pcdev->notifier.unbind = pxa_camera_sensor_unbind;
+ pcdev->notifier.ops = &pxa_camera_sensor_ops;
if (!of_have_populated_dt())
pcdev->asd.match_type = V4L2_ASYNC_MATCH_I2C;
diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c b/drivers/media/platform/qcom/camss-8x16/camss.c
index a3760b5dd1d1..390a42c17b66 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss.c
@@ -601,6 +601,11 @@ static int camss_subdev_notifier_complete(struct v4l2_async_notifier *async)
return media_device_register(&camss->media_dev);
}
+static const struct v4l2_async_notifier_operations camss_subdev_notifier_ops = {
+ .bound = camss_subdev_notifier_bound,
+ .complete = camss_subdev_notifier_complete,
+};
+
static const struct media_device_ops camss_media_ops = {
.link_notify = v4l2_pipeline_link_notify,
};
@@ -655,8 +660,7 @@ static int camss_probe(struct platform_device *pdev)
goto err_register_entities;
if (camss->notifier.num_subdevs) {
- camss->notifier.bound = camss_subdev_notifier_bound;
- camss->notifier.complete = camss_subdev_notifier_complete;
+ camss->notifier.ops = &camss_subdev_notifier_ops;
ret = v4l2_async_notifier_register(&camss->v4l2_dev,
&camss->notifier);
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 62b4a94f9a39..55acab4f2905 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -134,6 +134,12 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations rvin_digital_notify_ops = {
+ .bound = rvin_digital_notify_bound,
+ .unbind = rvin_digital_notify_unbind,
+ .complete = rvin_digital_notify_complete,
+};
+
static int rvin_digital_parse_v4l2(struct device *dev,
struct v4l2_fwnode_endpoint *vep,
@@ -182,9 +188,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
to_of_node(
vin->digital->asd.match.fwnode.fwnode));
- vin->notifier.bound = rvin_digital_notify_bound;
- vin->notifier.unbind = rvin_digital_notify_unbind;
- vin->notifier.complete = rvin_digital_notify_complete;
+ vin->notifier.ops = &rvin_digital_notify_ops;
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index 522364ff0d5d..0b2214d6d621 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -1185,6 +1185,12 @@ static int rcar_drif_notify_complete(struct v4l2_async_notifier *notifier)
return ret;
}
+static const struct v4l2_async_notifier_operations rcar_drif_notify_ops = {
+ .bound = rcar_drif_notify_bound,
+ .unbind = rcar_drif_notify_unbind,
+ .complete = rcar_drif_notify_complete,
+};
+
/* Read endpoint properties */
static void rcar_drif_get_ep_properties(struct rcar_drif_sdr *sdr,
struct fwnode_handle *fwnode)
@@ -1347,9 +1353,7 @@ static int rcar_drif_sdr_probe(struct rcar_drif_sdr *sdr)
if (ret)
goto error;
- sdr->notifier.bound = rcar_drif_notify_bound;
- sdr->notifier.unbind = rcar_drif_notify_unbind;
- sdr->notifier.complete = rcar_drif_notify_complete;
+ sdr->notifier.ops = &rcar_drif_notify_ops;
/* Register notifier */
ret = v4l2_async_notifier_register(&sdr->v4l2_dev, &sdr->notifier);
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 1f3c450c7a69..916ff68b73d4 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1391,6 +1391,12 @@ static int soc_camera_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations soc_camera_async_ops = {
+ .bound = soc_camera_async_bound,
+ .unbind = soc_camera_async_unbind,
+ .complete = soc_camera_async_complete,
+};
+
static int scan_async_group(struct soc_camera_host *ici,
struct v4l2_async_subdev **asd, unsigned int size)
{
@@ -1437,9 +1443,7 @@ static int scan_async_group(struct soc_camera_host *ici,
sasc->notifier.subdevs = asd;
sasc->notifier.num_subdevs = size;
- sasc->notifier.bound = soc_camera_async_bound;
- sasc->notifier.unbind = soc_camera_async_unbind;
- sasc->notifier.complete = soc_camera_async_complete;
+ sasc->notifier.ops = &soc_camera_async_ops;
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
@@ -1537,9 +1541,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
sasc->notifier.subdevs = &info->subdev;
sasc->notifier.num_subdevs = 1;
- sasc->notifier.bound = soc_camera_async_bound;
- sasc->notifier.unbind = soc_camera_async_unbind;
- sasc->notifier.complete = soc_camera_async_complete;
+ sasc->notifier.ops = &soc_camera_async_ops;
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 35ba6f211b79..ac4c450a6c7d 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1495,6 +1495,12 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
return 0;
}
+static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = {
+ .bound = dcmi_graph_notify_bound,
+ .unbind = dcmi_graph_notify_unbind,
+ .complete = dcmi_graph_notify_complete,
+};
+
static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
{
struct device_node *ep = NULL;
@@ -1542,9 +1548,7 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
dcmi->notifier.subdevs = subdevs;
dcmi->notifier.num_subdevs = 1;
- dcmi->notifier.bound = dcmi_graph_notify_bound;
- dcmi->notifier.unbind = dcmi_graph_notify_unbind;
- dcmi->notifier.complete = dcmi_graph_notify_complete;
+ dcmi->notifier.ops = &dcmi_graph_notify_ops;
ret = v4l2_async_notifier_register(&dcmi->v4l2_dev, &dcmi->notifier);
if (ret < 0) {
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 42e383a48ffe..8b586c864524 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1522,6 +1522,11 @@ static int cal_async_complete(struct v4l2_async_notifier *notifier)
return 0;
}
+static const struct v4l2_async_notifier_operations cal_async_ops = {
+ .bound = cal_async_bound,
+ .complete = cal_async_complete,
+};
+
static int cal_complete_ctx(struct cal_ctx *ctx)
{
struct video_device *vfd;
@@ -1736,8 +1741,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
ctx->asd_list[0] = asd;
ctx->notifier.subdevs = ctx->asd_list;
ctx->notifier.num_subdevs = 1;
- ctx->notifier.bound = cal_async_bound;
- ctx->notifier.complete = cal_async_complete;
+ ctx->notifier.ops = &cal_async_ops;
ret = v4l2_async_notifier_register(&ctx->v4l2_dev,
&ctx->notifier);
if (ret) {
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index ebfdf334d99c..d881cf09876d 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -351,6 +351,11 @@ static int xvip_graph_notify_bound(struct v4l2_async_notifier *notifier,
return -EINVAL;
}
+static const struct v4l2_async_notifier_operations xvip_graph_notify_ops = {
+ .bound = xvip_graph_notify_bound,
+ .complete = xvip_graph_notify_complete,
+};
+
static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
struct device_node *node)
{
@@ -548,8 +553,7 @@ static int xvip_graph_init(struct xvip_composite_device *xdev)
xdev->notifier.subdevs = subdevs;
xdev->notifier.num_subdevs = num_subdevs;
- xdev->notifier.bound = xvip_graph_notify_bound;
- xdev->notifier.complete = xvip_graph_notify_complete;
+ xdev->notifier.ops = &xvip_graph_notify_ops;
ret = v4l2_async_notifier_register(&xdev->v4l2_dev, &xdev->notifier);
if (ret < 0) {
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index ad099b9bb71d..a2df85ea00f4 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -102,16 +102,16 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
{
int ret;
- if (notifier->bound) {
- ret = notifier->bound(notifier, sd, asd);
+ if (notifier->ops->bound) {
+ ret = notifier->ops->bound(notifier, sd, asd);
if (ret < 0)
return ret;
}
ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
if (ret < 0) {
- if (notifier->unbind)
- notifier->unbind(notifier, sd, asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, asd);
return ret;
}
@@ -123,8 +123,8 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
/* Move from the global subdevice list to notifier's done */
list_move(&sd->async_list, ¬ifier->done);
- if (list_empty(¬ifier->waiting) && notifier->complete)
- return notifier->complete(notifier);
+ if (list_empty(¬ifier->waiting) && notifier->ops->complete)
+ return notifier->ops->complete(notifier);
return 0;
}
@@ -210,8 +210,8 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
v4l2_async_cleanup(sd);
- if (notifier->unbind)
- notifier->unbind(notifier, sd, sd->asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, sd->asd);
}
mutex_unlock(&list_lock);
@@ -300,8 +300,8 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
v4l2_async_cleanup(sd);
- if (notifier->unbind)
- notifier->unbind(notifier, sd, sd->asd);
+ if (notifier->ops->unbind)
+ notifier->ops->unbind(notifier, sd, sd->asd);
mutex_unlock(&list_lock);
}
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index d96f4512224f..e8dc2c93658a 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -440,6 +440,11 @@ static int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
return media_device_register(&imxmd->md);
}
+static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {
+ .bound = imx_media_subdev_bound,
+ .complete = imx_media_probe_complete,
+};
+
/*
* adds controls to a video device from an entity subdevice.
* Continues upstream from the entity's sink pads.
@@ -608,8 +613,7 @@ static int imx_media_probe(struct platform_device *pdev)
/* prepare the async subdev notifier and register it */
imxmd->subdev_notifier.subdevs = imxmd->async_ptrs;
- imxmd->subdev_notifier.bound = imx_media_subdev_bound;
- imxmd->subdev_notifier.complete = imx_media_probe_complete;
+ imxmd->subdev_notifier.ops = &imx_media_subdev_ops;
ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
&imxmd->subdev_notifier);
if (ret) {
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 96fa1afc00dd..3c48f8b66d12 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -18,6 +18,7 @@ struct device;
struct device_node;
struct v4l2_device;
struct v4l2_subdev;
+struct v4l2_async_notifier;
/* A random max subdevice number, used to allocate an array on stack */
#define V4L2_MAX_SUBDEVS 128U
@@ -79,8 +80,25 @@ struct v4l2_async_subdev {
};
/**
+ * struct v4l2_async_notifier_operations - Asynchronous V4L2 notifier operations
+ * @bound: a subdevice driver has successfully probed one of the subdevices
+ * @complete: all subdevices have been probed successfully
+ * @unbind: a subdevice is leaving
+ */
+struct v4l2_async_notifier_operations {
+ int (*bound)(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd);
+ int (*complete)(struct v4l2_async_notifier *notifier);
+ void (*unbind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd);
+};
+
+/**
* struct v4l2_async_notifier - v4l2_device notifier data
*
+ * @ops: notifier operations
* @num_subdevs: number of subdevices used in the subdevs array
* @max_subdevs: number of subdevices allocated in the subdevs array
* @subdevs: array of pointers to subdevice descriptors
@@ -88,11 +106,9 @@ struct v4l2_async_subdev {
* @waiting: list of struct v4l2_async_subdev, waiting for their drivers
* @done: list of struct v4l2_subdev, already probed
* @list: member in a global list of notifiers
- * @bound: a subdevice driver has successfully probed one of subdevices
- * @complete: all subdevices have been probed successfully
- * @unbind: a subdevice is leaving
*/
struct v4l2_async_notifier {
+ const struct v4l2_async_notifier_operations *ops;
unsigned int num_subdevs;
unsigned int max_subdevs;
struct v4l2_async_subdev **subdevs;
@@ -100,13 +116,6 @@ struct v4l2_async_notifier {
struct list_head waiting;
struct list_head done;
struct list_head list;
- int (*bound)(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd);
- int (*complete)(struct v4l2_async_notifier *notifier);
- void (*unbind)(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *subdev,
- struct v4l2_async_subdev *asd);
};
/**
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 13/24] v4l: async: Allow async notifier register call succeed with no subdevs
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
The information on how many async sub-devices would be bindable to a
notifier is typically dependent on information from platform firmware and
it's not driver's business to be aware of that.
Many V4L2 main drivers are perfectly usable (and useful) without async
sub-devices and so if there aren't any around, just proceed call the
notifier's complete callback immediately without registering the notifier
itself.
If a driver needs to check whether there are async sub-devices available,
it can be done by inspecting the notifier's num_subdevs field which tells
the number of async sub-devices.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/media/v4l2-core/v4l2-async.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 7b396ff4302b..9ebc2e079d03 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -170,10 +170,12 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
struct v4l2_async_subdev *asd;
int i;
- if (!v4l2_dev || !notifier->num_subdevs ||
- notifier->num_subdevs > V4L2_MAX_SUBDEVS)
+ if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
+ if (!notifier->num_subdevs)
+ return v4l2_async_notifier_call_complete(notifier);
+
notifier->v4l2_dev = v4l2_dev;
INIT_LIST_HEAD(¬ifier->waiting);
INIT_LIST_HEAD(¬ifier->done);
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 13/24] v4l: async: Allow async notifier register call succeed with no subdevs
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
The information on how many async sub-devices would be bindable to a
notifier is typically dependent on information from platform firmware and
it's not driver's business to be aware of that.
Many V4L2 main drivers are perfectly usable (and useful) without async
sub-devices and so if there aren't any around, just proceed call the
notifier's complete callback immediately without registering the notifier
itself.
If a driver needs to check whether there are async sub-devices available,
it can be done by inspecting the notifier's num_subdevs field which tells
the number of async sub-devices.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-async.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 7b396ff4302b..9ebc2e079d03 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -170,10 +170,12 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
struct v4l2_async_subdev *asd;
int i;
- if (!v4l2_dev || !notifier->num_subdevs ||
- notifier->num_subdevs > V4L2_MAX_SUBDEVS)
+ if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
+ if (!notifier->num_subdevs)
+ return v4l2_async_notifier_call_complete(notifier);
+
notifier->v4l2_dev = v4l2_dev;
INIT_LIST_HEAD(¬ifier->waiting);
INIT_LIST_HEAD(¬ifier->done);
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 14/24] v4l: async: Allow binding notifiers to sub-devices
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 7:59 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
Registering a notifier has required the knowledge of struct v4l2_device
for the reason that sub-devices generally are registered to the
v4l2_device (as well as the media device, also available through
v4l2_device).
This information is not available for sub-device drivers at probe time.
What this patch does is that it allows registering notifiers without
having v4l2_device around. Instead the sub-device pointer is stored in the
notifier. Once the sub-device of the driver that registered the notifier
is registered, the notifier will gain the knowledge of the v4l2_device,
and the binding of async sub-devices from the sub-device driver's notifier
may proceed.
The root notifier's complete callback is only called when all sub-device
notifiers are completed.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----
include/media/v4l2-async.h | 16 ++-
2 files changed, 202 insertions(+), 31 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 9ebc2e079d03..6f788b2e922a 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)
return n->ops->complete(n);
}
+static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd);
+
static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
{
#if IS_ENABLED(CONFIG_I2C)
@@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
return NULL;
}
+/* Get the sub-device notifier registered by a sub-device driver. */
+static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(
+ struct v4l2_subdev *sd)
+{
+ struct v4l2_async_notifier *n;
+
+ list_for_each_entry(n, ¬ifier_list, list)
+ if (n->sd == sd)
+ return n;
+
+ return NULL;
+}
+
+/* Return true if all sub-device notifiers are complete, false otherwise. */
+static bool v4l2_async_subdev_notifiers_complete(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd;
+
+ if (!list_empty(¬ifier->waiting))
+ return false;
+
+ list_for_each_entry(sd, ¬ifier->done, async_list) {
+ struct v4l2_async_notifier *subdev_notifier =
+ v4l2_async_get_subdev_notifier(sd);
+
+ if (!subdev_notifier)
+ continue;
+
+ if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
+ return false;
+ }
+
+ return true;
+}
+
+/* Get v4l2_device related to the notifier if one can be found. */
+static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev(
+ struct v4l2_async_notifier *notifier)
+{
+ while (notifier->parent)
+ notifier = notifier->parent;
+
+ return notifier->v4l2_dev;
+}
+
+/* Test all async sub-devices in a notifier for a match. */
+static int v4l2_async_notifier_try_all_subdevs(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd;
+
+ if (!v4l2_async_notifier_get_v4l2_dev(notifier))
+ return 0;
+
+again:
+ list_for_each_entry(sd, &subdev_list, async_list) {
+ struct v4l2_async_subdev *asd;
+ int ret;
+
+ asd = v4l2_async_find_match(notifier, sd);
+ if (!asd)
+ continue;
+
+ ret = v4l2_async_match_notify(notifier, sd, asd);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * v4l2_async_match_notify() may lead to registering a
+ * new notifier and thus changing the async subdevs
+ * list. In order to proceed safely from here, restart
+ * parsing the list from the beginning.
+ */
+ goto again;
+ }
+
+ return 0;
+}
+
+/* Try completing a notifier. */
+static int v4l2_async_notifier_try_complete(
+ struct v4l2_async_notifier *notifier)
+{
+ do {
+ int ret;
+
+ /* Any local async sub-devices left? */
+ if (!list_empty(¬ifier->waiting))
+ return 0;
+
+ /*
+ * Any sub-device notifiers waiting for async subdevs
+ * to be bound?
+ */
+ if (!v4l2_async_subdev_notifiers_complete(notifier))
+ return 0;
+
+ /* Proceed completing the notifier */
+ ret = v4l2_async_notifier_call_complete(notifier);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Obtain notifier's parent. If there is one, repeat
+ * the process, otherwise we're done here.
+ */
+ } while ((notifier = notifier->parent));
+
+ return 0;
+}
+
static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
struct v4l2_subdev *sd,
struct v4l2_async_subdev *asd)
{
+ struct v4l2_async_notifier *subdev_notifier;
int ret;
- ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
- if (ret < 0)
+ ret = v4l2_device_register_subdev(
+ v4l2_async_notifier_get_v4l2_dev(notifier), sd);
+ if (ret)
return ret;
ret = v4l2_async_notifier_call_bound(notifier, sd, asd);
@@ -148,10 +266,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
/* Move from the global subdevice list to notifier's done */
list_move(&sd->async_list, ¬ifier->done);
- if (list_empty(¬ifier->waiting))
- return v4l2_async_notifier_call_complete(notifier);
+ /*
+ * See if the sub-device has a notifier. If it does, proceed
+ * with checking for its async sub-devices.
+ */
+ subdev_notifier = v4l2_async_get_subdev_notifier(sd);
+ if (subdev_notifier && !subdev_notifier->parent) {
+ subdev_notifier->parent = notifier;
+ ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier);
+ if (ret)
+ return ret;
+ }
- return 0;
+ /* Try completing the notifier and its parent(s). */
+ return v4l2_async_notifier_try_complete(notifier);
}
static void v4l2_async_cleanup(struct v4l2_subdev *sd)
@@ -163,20 +291,18 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
sd->dev = NULL;
}
-int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
- struct v4l2_async_notifier *notifier)
+static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
{
- struct v4l2_subdev *sd, *tmp;
struct v4l2_async_subdev *asd;
+ int ret;
int i;
- if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
+ if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
if (!notifier->num_subdevs)
return v4l2_async_notifier_call_complete(notifier);
- notifier->v4l2_dev = v4l2_dev;
INIT_LIST_HEAD(¬ifier->waiting);
INIT_LIST_HEAD(¬ifier->done);
@@ -200,18 +326,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
mutex_lock(&list_lock);
- list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
- int ret;
-
- asd = v4l2_async_find_match(notifier, sd);
- if (!asd)
- continue;
-
- ret = v4l2_async_match_notify(notifier, sd, asd);
- if (ret < 0) {
- mutex_unlock(&list_lock);
- return ret;
- }
+ ret = v4l2_async_notifier_try_all_subdevs(notifier);
+ if (ret) {
+ mutex_unlock(&list_lock);
+ return ret;
}
/* Keep also completed notifiers on the list */
@@ -221,28 +339,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
return 0;
}
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier)
+{
+ if (!v4l2_dev || notifier->sd)
+ return -EINVAL;
+
+ notifier->v4l2_dev = v4l2_dev;
+
+ return __v4l2_async_notifier_register(notifier);
+}
EXPORT_SYMBOL(v4l2_async_notifier_register);
-void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier)
{
- struct v4l2_subdev *sd, *tmp;
+ if (!sd || notifier->v4l2_dev)
+ return -EINVAL;
- if (!notifier->v4l2_dev)
- return;
+ notifier->sd = sd;
- mutex_lock(&list_lock);
+ return __v4l2_async_notifier_register(notifier);
+}
+EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
- list_del(¬ifier->list);
+/* Unbind all sub-devices in the notifier tree. */
+static void v4l2_async_notifier_unbind_all_subdevs(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd, *tmp;
list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
+ struct v4l2_async_notifier *subdev_notifier =
+ v4l2_async_get_subdev_notifier(sd);
+
+ if (subdev_notifier)
+ v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
+
v4l2_async_cleanup(sd);
v4l2_async_notifier_call_unbind(notifier, sd, sd->asd);
+
+ list_del(&sd->async_list);
+ list_add(&sd->async_list, &subdev_list);
}
- mutex_unlock(&list_lock);
+ notifier->parent = NULL;
+}
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+ if (!notifier->v4l2_dev && !notifier->sd)
+ return;
- notifier->v4l2_dev = NULL;
+ mutex_lock(&list_lock);
+
+ v4l2_async_notifier_unbind_all_subdevs(notifier);
+
+ list_del(¬ifier->list);
+
+ mutex_unlock(&list_lock);
}
EXPORT_SYMBOL(v4l2_async_notifier_unregister);
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 3bc8a7c0d83f..cf409d45208c 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {
* @num_subdevs: number of subdevices used in the subdevs array
* @max_subdevs: number of subdevices allocated in the subdevs array
* @subdevs: array of pointers to subdevice descriptors
- * @v4l2_dev: pointer to struct v4l2_device
+ * @v4l2_dev: v4l2_device of the root notifier, NULL otherwise
+ * @sd: sub-device that registered the notifier, NULL otherwise
+ * @parent: parent notifier carrying @v4l2_dev
* @waiting: list of struct v4l2_async_subdev, waiting for their drivers
* @done: list of struct v4l2_subdev, already probed
* @list: member in a global list of notifiers
@@ -113,6 +115,8 @@ struct v4l2_async_notifier {
unsigned int max_subdevs;
struct v4l2_async_subdev **subdevs;
struct v4l2_device *v4l2_dev;
+ struct v4l2_subdev *sd;
+ struct v4l2_async_notifier *parent;
struct list_head waiting;
struct list_head done;
struct list_head list;
@@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
struct v4l2_async_notifier *notifier);
/**
+ * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous
+ * notifier for a sub-device
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ * @notifier: pointer to &struct v4l2_async_notifier
+ */
+int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier);
+
+/**
* v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier
*
* @notifier: pointer to &struct v4l2_async_notifier
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 14/24] v4l: async: Allow binding notifiers to sub-devices
@ 2017-09-11 7:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 7:59 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Registering a notifier has required the knowledge of struct v4l2_device
for the reason that sub-devices generally are registered to the
v4l2_device (as well as the media device, also available through
v4l2_device).
This information is not available for sub-device drivers at probe time.
What this patch does is that it allows registering notifiers without
having v4l2_device around. Instead the sub-device pointer is stored in the
notifier. Once the sub-device of the driver that registered the notifier
is registered, the notifier will gain the knowledge of the v4l2_device,
and the binding of async sub-devices from the sub-device driver's notifier
may proceed.
The root notifier's complete callback is only called when all sub-device
notifiers are completed.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----
include/media/v4l2-async.h | 16 ++-
2 files changed, 202 insertions(+), 31 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 9ebc2e079d03..6f788b2e922a 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)
return n->ops->complete(n);
}
+static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd);
+
static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
{
#if IS_ENABLED(CONFIG_I2C)
@@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
return NULL;
}
+/* Get the sub-device notifier registered by a sub-device driver. */
+static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(
+ struct v4l2_subdev *sd)
+{
+ struct v4l2_async_notifier *n;
+
+ list_for_each_entry(n, ¬ifier_list, list)
+ if (n->sd == sd)
+ return n;
+
+ return NULL;
+}
+
+/* Return true if all sub-device notifiers are complete, false otherwise. */
+static bool v4l2_async_subdev_notifiers_complete(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd;
+
+ if (!list_empty(¬ifier->waiting))
+ return false;
+
+ list_for_each_entry(sd, ¬ifier->done, async_list) {
+ struct v4l2_async_notifier *subdev_notifier =
+ v4l2_async_get_subdev_notifier(sd);
+
+ if (!subdev_notifier)
+ continue;
+
+ if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
+ return false;
+ }
+
+ return true;
+}
+
+/* Get v4l2_device related to the notifier if one can be found. */
+static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev(
+ struct v4l2_async_notifier *notifier)
+{
+ while (notifier->parent)
+ notifier = notifier->parent;
+
+ return notifier->v4l2_dev;
+}
+
+/* Test all async sub-devices in a notifier for a match. */
+static int v4l2_async_notifier_try_all_subdevs(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd;
+
+ if (!v4l2_async_notifier_get_v4l2_dev(notifier))
+ return 0;
+
+again:
+ list_for_each_entry(sd, &subdev_list, async_list) {
+ struct v4l2_async_subdev *asd;
+ int ret;
+
+ asd = v4l2_async_find_match(notifier, sd);
+ if (!asd)
+ continue;
+
+ ret = v4l2_async_match_notify(notifier, sd, asd);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * v4l2_async_match_notify() may lead to registering a
+ * new notifier and thus changing the async subdevs
+ * list. In order to proceed safely from here, restart
+ * parsing the list from the beginning.
+ */
+ goto again;
+ }
+
+ return 0;
+}
+
+/* Try completing a notifier. */
+static int v4l2_async_notifier_try_complete(
+ struct v4l2_async_notifier *notifier)
+{
+ do {
+ int ret;
+
+ /* Any local async sub-devices left? */
+ if (!list_empty(¬ifier->waiting))
+ return 0;
+
+ /*
+ * Any sub-device notifiers waiting for async subdevs
+ * to be bound?
+ */
+ if (!v4l2_async_subdev_notifiers_complete(notifier))
+ return 0;
+
+ /* Proceed completing the notifier */
+ ret = v4l2_async_notifier_call_complete(notifier);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Obtain notifier's parent. If there is one, repeat
+ * the process, otherwise we're done here.
+ */
+ } while ((notifier = notifier->parent));
+
+ return 0;
+}
+
static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
struct v4l2_subdev *sd,
struct v4l2_async_subdev *asd)
{
+ struct v4l2_async_notifier *subdev_notifier;
int ret;
- ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
- if (ret < 0)
+ ret = v4l2_device_register_subdev(
+ v4l2_async_notifier_get_v4l2_dev(notifier), sd);
+ if (ret)
return ret;
ret = v4l2_async_notifier_call_bound(notifier, sd, asd);
@@ -148,10 +266,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
/* Move from the global subdevice list to notifier's done */
list_move(&sd->async_list, ¬ifier->done);
- if (list_empty(¬ifier->waiting))
- return v4l2_async_notifier_call_complete(notifier);
+ /*
+ * See if the sub-device has a notifier. If it does, proceed
+ * with checking for its async sub-devices.
+ */
+ subdev_notifier = v4l2_async_get_subdev_notifier(sd);
+ if (subdev_notifier && !subdev_notifier->parent) {
+ subdev_notifier->parent = notifier;
+ ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier);
+ if (ret)
+ return ret;
+ }
- return 0;
+ /* Try completing the notifier and its parent(s). */
+ return v4l2_async_notifier_try_complete(notifier);
}
static void v4l2_async_cleanup(struct v4l2_subdev *sd)
@@ -163,20 +291,18 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
sd->dev = NULL;
}
-int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
- struct v4l2_async_notifier *notifier)
+static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
{
- struct v4l2_subdev *sd, *tmp;
struct v4l2_async_subdev *asd;
+ int ret;
int i;
- if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
+ if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
if (!notifier->num_subdevs)
return v4l2_async_notifier_call_complete(notifier);
- notifier->v4l2_dev = v4l2_dev;
INIT_LIST_HEAD(¬ifier->waiting);
INIT_LIST_HEAD(¬ifier->done);
@@ -200,18 +326,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
mutex_lock(&list_lock);
- list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
- int ret;
-
- asd = v4l2_async_find_match(notifier, sd);
- if (!asd)
- continue;
-
- ret = v4l2_async_match_notify(notifier, sd, asd);
- if (ret < 0) {
- mutex_unlock(&list_lock);
- return ret;
- }
+ ret = v4l2_async_notifier_try_all_subdevs(notifier);
+ if (ret) {
+ mutex_unlock(&list_lock);
+ return ret;
}
/* Keep also completed notifiers on the list */
@@ -221,28 +339,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
return 0;
}
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier)
+{
+ if (!v4l2_dev || notifier->sd)
+ return -EINVAL;
+
+ notifier->v4l2_dev = v4l2_dev;
+
+ return __v4l2_async_notifier_register(notifier);
+}
EXPORT_SYMBOL(v4l2_async_notifier_register);
-void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier)
{
- struct v4l2_subdev *sd, *tmp;
+ if (!sd || notifier->v4l2_dev)
+ return -EINVAL;
- if (!notifier->v4l2_dev)
- return;
+ notifier->sd = sd;
- mutex_lock(&list_lock);
+ return __v4l2_async_notifier_register(notifier);
+}
+EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
- list_del(¬ifier->list);
+/* Unbind all sub-devices in the notifier tree. */
+static void v4l2_async_notifier_unbind_all_subdevs(
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd, *tmp;
list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
+ struct v4l2_async_notifier *subdev_notifier =
+ v4l2_async_get_subdev_notifier(sd);
+
+ if (subdev_notifier)
+ v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
+
v4l2_async_cleanup(sd);
v4l2_async_notifier_call_unbind(notifier, sd, sd->asd);
+
+ list_del(&sd->async_list);
+ list_add(&sd->async_list, &subdev_list);
}
- mutex_unlock(&list_lock);
+ notifier->parent = NULL;
+}
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+ if (!notifier->v4l2_dev && !notifier->sd)
+ return;
- notifier->v4l2_dev = NULL;
+ mutex_lock(&list_lock);
+
+ v4l2_async_notifier_unbind_all_subdevs(notifier);
+
+ list_del(¬ifier->list);
+
+ mutex_unlock(&list_lock);
}
EXPORT_SYMBOL(v4l2_async_notifier_unregister);
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 3bc8a7c0d83f..cf409d45208c 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {
* @num_subdevs: number of subdevices used in the subdevs array
* @max_subdevs: number of subdevices allocated in the subdevs array
* @subdevs: array of pointers to subdevice descriptors
- * @v4l2_dev: pointer to struct v4l2_device
+ * @v4l2_dev: v4l2_device of the root notifier, NULL otherwise
+ * @sd: sub-device that registered the notifier, NULL otherwise
+ * @parent: parent notifier carrying @v4l2_dev
* @waiting: list of struct v4l2_async_subdev, waiting for their drivers
* @done: list of struct v4l2_subdev, already probed
* @list: member in a global list of notifiers
@@ -113,6 +115,8 @@ struct v4l2_async_notifier {
unsigned int max_subdevs;
struct v4l2_async_subdev **subdevs;
struct v4l2_device *v4l2_dev;
+ struct v4l2_subdev *sd;
+ struct v4l2_async_notifier *parent;
struct list_head waiting;
struct list_head done;
struct list_head list;
@@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
struct v4l2_async_notifier *notifier);
/**
+ * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous
+ * notifier for a sub-device
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ * @notifier: pointer to &struct v4l2_async_notifier
+ */
+int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier);
+
+/**
* v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier
*
* @notifier: pointer to &struct v4l2_async_notifier
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v10 14/24] v4l: async: Allow binding notifiers to sub-devices
2017-09-11 7:59 ` Sakari Ailus
(?)
@ 2017-09-11 8:57 ` Hans Verkuil
2017-09-11 9:30 ` Sakari Ailus
-1 siblings, 1 reply; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 8:57 UTC (permalink / raw)
To: Sakari Ailus, linux-media
Cc: niklas.soderlund, robh, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
On 09/11/2017 09:59 AM, Sakari Ailus wrote:
> Registering a notifier has required the knowledge of struct v4l2_device
> for the reason that sub-devices generally are registered to the
> v4l2_device (as well as the media device, also available through
> v4l2_device).
>
> This information is not available for sub-device drivers at probe time.
>
> What this patch does is that it allows registering notifiers without
> having v4l2_device around. Instead the sub-device pointer is stored in the
> notifier. Once the sub-device of the driver that registered the notifier
> is registered, the notifier will gain the knowledge of the v4l2_device,
> and the binding of async sub-devices from the sub-device driver's notifier
> may proceed.
>
> The root notifier's complete callback is only called when all sub-device
> notifiers are completed.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----
> include/media/v4l2-async.h | 16 ++-
> 2 files changed, 202 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 9ebc2e079d03..6f788b2e922a 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)
> return n->ops->complete(n);
> }
>
> +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *sd,
> + struct v4l2_async_subdev *asd);
> +
> static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> #if IS_ENABLED(CONFIG_I2C)
> @@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
> return NULL;
> }
>
> +/* Get the sub-device notifier registered by a sub-device driver. */
> +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(
I prefer to call this v4l2_async_find_subdev_notifier(). 'get' suggests
a getter function, but this actually has to find it. I think this may have
confused me during an earlier review of this code. The comment also needs
updating: "Find the sub-device...".
> + struct v4l2_subdev *sd)
> +{
> + struct v4l2_async_notifier *n;
> +
> + list_for_each_entry(n, ¬ifier_list, list)
> + if (n->sd == sd)
> + return n;
> +
> + return NULL;
> +}
> +
> +/* Return true if all sub-device notifiers are complete, false otherwise. */
> +static bool v4l2_async_subdev_notifiers_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!list_empty(¬ifier->waiting))
> + return false;
> +
> + list_for_each_entry(sd, ¬ifier->done, async_list) {
> + struct v4l2_async_notifier *subdev_notifier =
> + v4l2_async_get_subdev_notifier(sd);
Would it make sense to add a 'struct v4l2_async_notifier *subdev_notifier'
field to struct v4l2_subdev? It's set when a subdev registers a notifier.
That way you can just use sd->subdev_notifier here.
I wonder if v4l2_async_get_subdev_notifier() is needed at all if you do
this.
> +
> + if (!subdev_notifier)
> + continue;
> +
> + if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/* Get v4l2_device related to the notifier if one can be found. */
> +static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev(
> + struct v4l2_async_notifier *notifier)
> +{
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + return notifier->v4l2_dev;
> +}
> +
> +/* Test all async sub-devices in a notifier for a match. */
> +static int v4l2_async_notifier_try_all_subdevs(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!v4l2_async_notifier_get_v4l2_dev(notifier))
> + return 0;
> +
> +again:
> + list_for_each_entry(sd, &subdev_list, async_list) {
> + struct v4l2_async_subdev *asd;
> + int ret;
> +
> + asd = v4l2_async_find_match(notifier, sd);
> + if (!asd)
> + continue;
> +
> + ret = v4l2_async_match_notify(notifier, sd, asd);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * v4l2_async_match_notify() may lead to registering a
> + * new notifier and thus changing the async subdevs
> + * list. In order to proceed safely from here, restart
> + * parsing the list from the beginning.
> + */
> + goto again;
> + }
> +
> + return 0;
> +}
> +
> +/* Try completing a notifier. */
> +static int v4l2_async_notifier_try_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + do {
> + int ret;
> +
> + /* Any local async sub-devices left? */
> + if (!list_empty(¬ifier->waiting))
> + return 0;
> +
> + /*
> + * Any sub-device notifiers waiting for async subdevs
> + * to be bound?
> + */
> + if (!v4l2_async_subdev_notifiers_complete(notifier))
> + return 0;
> +
> + /* Proceed completing the notifier */
> + ret = v4l2_async_notifier_call_complete(notifier);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * Obtain notifier's parent. If there is one, repeat
> + * the process, otherwise we're done here.
> + */
> + } while ((notifier = notifier->parent));
> +
> + return 0;
> +}
> +
> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> struct v4l2_subdev *sd,
> struct v4l2_async_subdev *asd)
> {
> + struct v4l2_async_notifier *subdev_notifier;
> int ret;
>
> - ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
> - if (ret < 0)
> + ret = v4l2_device_register_subdev(
> + v4l2_async_notifier_get_v4l2_dev(notifier), sd);
> + if (ret)
> return ret;
>
> ret = v4l2_async_notifier_call_bound(notifier, sd, asd);
> @@ -148,10 +266,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> /* Move from the global subdevice list to notifier's done */
> list_move(&sd->async_list, ¬ifier->done);
>
> - if (list_empty(¬ifier->waiting))
> - return v4l2_async_notifier_call_complete(notifier);
> + /*
> + * See if the sub-device has a notifier. If it does, proceed
> + * with checking for its async sub-devices.
> + */
> + subdev_notifier = v4l2_async_get_subdev_notifier(sd);
> + if (subdev_notifier && !subdev_notifier->parent) {
> + subdev_notifier->parent = notifier;
> + ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier);
> + if (ret)
> + return ret;
> + }
>
> - return 0;
> + /* Try completing the notifier and its parent(s). */
> + return v4l2_async_notifier_try_complete(notifier);
> }
>
> static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> @@ -163,20 +291,18 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> sd->dev = NULL;
> }
>
> -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> - struct v4l2_async_notifier *notifier)
> +static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
> {
> - struct v4l2_subdev *sd, *tmp;
> struct v4l2_async_subdev *asd;
> + int ret;
> int i;
>
> - if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> + if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> return -EINVAL;
>
> if (!notifier->num_subdevs)
> return v4l2_async_notifier_call_complete(notifier);
>
> - notifier->v4l2_dev = v4l2_dev;
> INIT_LIST_HEAD(¬ifier->waiting);
> INIT_LIST_HEAD(¬ifier->done);
>
> @@ -200,18 +326,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
>
> mutex_lock(&list_lock);
>
> - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
> - int ret;
> -
> - asd = v4l2_async_find_match(notifier, sd);
> - if (!asd)
> - continue;
> -
> - ret = v4l2_async_match_notify(notifier, sd, asd);
> - if (ret < 0) {
> - mutex_unlock(&list_lock);
> - return ret;
> - }
> + ret = v4l2_async_notifier_try_all_subdevs(notifier);
> + if (ret) {
> + mutex_unlock(&list_lock);
> + return ret;
> }
>
> /* Keep also completed notifiers on the list */
> @@ -221,28 +339,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
>
> return 0;
> }
> +
> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> + struct v4l2_async_notifier *notifier)
> +{
> + if (!v4l2_dev || notifier->sd)
Should this be a WARN_ON?
> + return -EINVAL;
> +
> + notifier->v4l2_dev = v4l2_dev;
> +
> + return __v4l2_async_notifier_register(notifier);
> +}
> EXPORT_SYMBOL(v4l2_async_notifier_register);
>
> -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
> + struct v4l2_async_notifier *notifier)
> {
> - struct v4l2_subdev *sd, *tmp;
> + if (!sd || notifier->v4l2_dev)
Ditto.
> + return -EINVAL;
>
> - if (!notifier->v4l2_dev)
> - return;
> + notifier->sd = sd;
>
> - mutex_lock(&list_lock);
> + return __v4l2_async_notifier_register(notifier);
> +}
> +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
>
> - list_del(¬ifier->list);
> +/* Unbind all sub-devices in the notifier tree. */
> +static void v4l2_async_notifier_unbind_all_subdevs(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd, *tmp;
>
> list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
> + struct v4l2_async_notifier *subdev_notifier =
> + v4l2_async_get_subdev_notifier(sd);
> +
> + if (subdev_notifier)
> + v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
> +
> v4l2_async_cleanup(sd);
>
> v4l2_async_notifier_call_unbind(notifier, sd, sd->asd);
> +
> + list_del(&sd->async_list);
> + list_add(&sd->async_list, &subdev_list);
> }
>
> - mutex_unlock(&list_lock);
> + notifier->parent = NULL;
Shouldn't notifier->v4l2_dev and notifier->sd be set to NULL as well?
I can't really tell.
> +}
> +
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> +{
> + if (!notifier->v4l2_dev && !notifier->sd)
> + return;
>
> - notifier->v4l2_dev = NULL;
> + mutex_lock(&list_lock);
> +
> + v4l2_async_notifier_unbind_all_subdevs(notifier);
> +
> + list_del(¬ifier->list);
> +
> + mutex_unlock(&list_lock);
> }
> EXPORT_SYMBOL(v4l2_async_notifier_unregister);
>
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 3bc8a7c0d83f..cf409d45208c 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {
> * @num_subdevs: number of subdevices used in the subdevs array
> * @max_subdevs: number of subdevices allocated in the subdevs array
> * @subdevs: array of pointers to subdevice descriptors
> - * @v4l2_dev: pointer to struct v4l2_device
> + * @v4l2_dev: v4l2_device of the root notifier, NULL otherwise
> + * @sd: sub-device that registered the notifier, NULL otherwise
> + * @parent: parent notifier carrying @v4l2_dev
That's not correct, it only carries v4l2_dev if it is the root notifier.
I think just 'parent notifier' is sufficient here.
> * @waiting: list of struct v4l2_async_subdev, waiting for their drivers
> * @done: list of struct v4l2_subdev, already probed
> * @list: member in a global list of notifiers
> @@ -113,6 +115,8 @@ struct v4l2_async_notifier {
> unsigned int max_subdevs;
> struct v4l2_async_subdev **subdevs;
> struct v4l2_device *v4l2_dev;
> + struct v4l2_subdev *sd;
> + struct v4l2_async_notifier *parent;
> struct list_head waiting;
> struct list_head done;
> struct list_head list;
> @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> struct v4l2_async_notifier *notifier);
>
> /**
> + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous
> + * notifier for a sub-device
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + * @notifier: pointer to &struct v4l2_async_notifier
> + */
> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
> + struct v4l2_async_notifier *notifier);
> +
> +/**
> * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier
> *
> * @notifier: pointer to &struct v4l2_async_notifier
>
Regards,
Hans
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 14/24] v4l: async: Allow binding notifiers to sub-devices
2017-09-11 8:57 ` Hans Verkuil
@ 2017-09-11 9:30 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 9:30 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
Hi Hans,
Thanks for the review!
On Mon, Sep 11, 2017 at 10:57:16AM +0200, Hans Verkuil wrote:
> On 09/11/2017 09:59 AM, Sakari Ailus wrote:
> > Registering a notifier has required the knowledge of struct v4l2_device
> > for the reason that sub-devices generally are registered to the
> > v4l2_device (as well as the media device, also available through
> > v4l2_device).
> >
> > This information is not available for sub-device drivers at probe time.
> >
> > What this patch does is that it allows registering notifiers without
> > having v4l2_device around. Instead the sub-device pointer is stored in the
> > notifier. Once the sub-device of the driver that registered the notifier
> > is registered, the notifier will gain the knowledge of the v4l2_device,
> > and the binding of async sub-devices from the sub-device driver's notifier
> > may proceed.
> >
> > The root notifier's complete callback is only called when all sub-device
> > notifiers are completed.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > drivers/media/v4l2-core/v4l2-async.c | 217 ++++++++++++++++++++++++++++++-----
> > include/media/v4l2-async.h | 16 ++-
> > 2 files changed, 202 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 9ebc2e079d03..6f788b2e922a 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n)
> > return n->ops->complete(n);
> > }
> >
> > +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> > + struct v4l2_subdev *sd,
> > + struct v4l2_async_subdev *asd);
> > +
> > static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > #if IS_ENABLED(CONFIG_I2C)
> > @@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
> > return NULL;
> > }
> >
> > +/* Get the sub-device notifier registered by a sub-device driver. */
> > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(
>
> I prefer to call this v4l2_async_find_subdev_notifier(). 'get' suggests
> a getter function, but this actually has to find it. I think this may have
> confused me during an earlier review of this code. The comment also needs
> updating: "Find the sub-device...".
Yes, makes sense. Get also suggests that there would be reference counting
which is not the case here.
I made the corresponding change to v4l2_async_notifier_find_v4l2_dev() as
well.
>
> > + struct v4l2_subdev *sd)
> > +{
> > + struct v4l2_async_notifier *n;
> > +
> > + list_for_each_entry(n, ¬ifier_list, list)
> > + if (n->sd == sd)
> > + return n;
> > +
> > + return NULL;
> > +}
> > +
> > +/* Return true if all sub-device notifiers are complete, false otherwise. */
> > +static bool v4l2_async_subdev_notifiers_complete(
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + struct v4l2_subdev *sd;
> > +
> > + if (!list_empty(¬ifier->waiting))
> > + return false;
> > +
> > + list_for_each_entry(sd, ¬ifier->done, async_list) {
> > + struct v4l2_async_notifier *subdev_notifier =
> > + v4l2_async_get_subdev_notifier(sd);
>
> Would it make sense to add a 'struct v4l2_async_notifier *subdev_notifier'
> field to struct v4l2_subdev? It's set when a subdev registers a notifier.
>
> That way you can just use sd->subdev_notifier here.
>
> I wonder if v4l2_async_get_subdev_notifier() is needed at all if you do
> this.
I thought of that, but ended up keeping the information in the notifier. As
the information is already available elsewhere, I didn't end up adding a
new field for the purpose. This is certainly not performance critical
either.
>
> > +
> > + if (!subdev_notifier)
> > + continue;
> > +
> > + if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +/* Get v4l2_device related to the notifier if one can be found. */
> > +static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev(
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + while (notifier->parent)
> > + notifier = notifier->parent;
> > +
> > + return notifier->v4l2_dev;
> > +}
> > +
> > +/* Test all async sub-devices in a notifier for a match. */
> > +static int v4l2_async_notifier_try_all_subdevs(
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + struct v4l2_subdev *sd;
> > +
> > + if (!v4l2_async_notifier_get_v4l2_dev(notifier))
> > + return 0;
> > +
> > +again:
> > + list_for_each_entry(sd, &subdev_list, async_list) {
> > + struct v4l2_async_subdev *asd;
> > + int ret;
> > +
> > + asd = v4l2_async_find_match(notifier, sd);
> > + if (!asd)
> > + continue;
> > +
> > + ret = v4l2_async_match_notify(notifier, sd, asd);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * v4l2_async_match_notify() may lead to registering a
> > + * new notifier and thus changing the async subdevs
> > + * list. In order to proceed safely from here, restart
> > + * parsing the list from the beginning.
> > + */
> > + goto again;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Try completing a notifier. */
> > +static int v4l2_async_notifier_try_complete(
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + do {
> > + int ret;
> > +
> > + /* Any local async sub-devices left? */
> > + if (!list_empty(¬ifier->waiting))
> > + return 0;
> > +
> > + /*
> > + * Any sub-device notifiers waiting for async subdevs
> > + * to be bound?
> > + */
> > + if (!v4l2_async_subdev_notifiers_complete(notifier))
> > + return 0;
> > +
> > + /* Proceed completing the notifier */
> > + ret = v4l2_async_notifier_call_complete(notifier);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * Obtain notifier's parent. If there is one, repeat
> > + * the process, otherwise we're done here.
> > + */
> > + } while ((notifier = notifier->parent));
> > +
> > + return 0;
> > +}
> > +
> > static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> > struct v4l2_subdev *sd,
> > struct v4l2_async_subdev *asd)
> > {
> > + struct v4l2_async_notifier *subdev_notifier;
> > int ret;
> >
> > - ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
> > - if (ret < 0)
> > + ret = v4l2_device_register_subdev(
> > + v4l2_async_notifier_get_v4l2_dev(notifier), sd);
> > + if (ret)
> > return ret;
> >
> > ret = v4l2_async_notifier_call_bound(notifier, sd, asd);
> > @@ -148,10 +266,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> > /* Move from the global subdevice list to notifier's done */
> > list_move(&sd->async_list, ¬ifier->done);
> >
> > - if (list_empty(¬ifier->waiting))
> > - return v4l2_async_notifier_call_complete(notifier);
> > + /*
> > + * See if the sub-device has a notifier. If it does, proceed
> > + * with checking for its async sub-devices.
> > + */
> > + subdev_notifier = v4l2_async_get_subdev_notifier(sd);
> > + if (subdev_notifier && !subdev_notifier->parent) {
> > + subdev_notifier->parent = notifier;
> > + ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier);
> > + if (ret)
> > + return ret;
> > + }
> >
> > - return 0;
> > + /* Try completing the notifier and its parent(s). */
> > + return v4l2_async_notifier_try_complete(notifier);
> > }
> >
> > static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> > @@ -163,20 +291,18 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> > sd->dev = NULL;
> > }
> >
> > -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > - struct v4l2_async_notifier *notifier)
> > +static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
> > {
> > - struct v4l2_subdev *sd, *tmp;
> > struct v4l2_async_subdev *asd;
> > + int ret;
> > int i;
> >
> > - if (!v4l2_dev || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> > + if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> > return -EINVAL;
> >
> > if (!notifier->num_subdevs)
> > return v4l2_async_notifier_call_complete(notifier);
> >
> > - notifier->v4l2_dev = v4l2_dev;
> > INIT_LIST_HEAD(¬ifier->waiting);
> > INIT_LIST_HEAD(¬ifier->done);
> >
> > @@ -200,18 +326,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> >
> > mutex_lock(&list_lock);
> >
> > - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
> > - int ret;
> > -
> > - asd = v4l2_async_find_match(notifier, sd);
> > - if (!asd)
> > - continue;
> > -
> > - ret = v4l2_async_match_notify(notifier, sd, asd);
> > - if (ret < 0) {
> > - mutex_unlock(&list_lock);
> > - return ret;
> > - }
> > + ret = v4l2_async_notifier_try_all_subdevs(notifier);
> > + if (ret) {
> > + mutex_unlock(&list_lock);
> > + return ret;
> > }
> >
> > /* Keep also completed notifiers on the list */
> > @@ -221,28 +339,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> >
> > return 0;
> > }
> > +
> > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + if (!v4l2_dev || notifier->sd)
>
> Should this be a WARN_ON?
Added WARN_ON().
>
> > + return -EINVAL;
> > +
> > + notifier->v4l2_dev = v4l2_dev;
> > +
> > + return __v4l2_async_notifier_register(notifier);
> > +}
> > EXPORT_SYMBOL(v4l2_async_notifier_register);
> >
> > -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
> > + struct v4l2_async_notifier *notifier)
> > {
> > - struct v4l2_subdev *sd, *tmp;
> > + if (!sd || notifier->v4l2_dev)
>
> Ditto.
Indeed.
>
> > + return -EINVAL;
> >
> > - if (!notifier->v4l2_dev)
> > - return;
> > + notifier->sd = sd;
> >
> > - mutex_lock(&list_lock);
> > + return __v4l2_async_notifier_register(notifier);
> > +}
> > +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
> >
> > - list_del(¬ifier->list);
> > +/* Unbind all sub-devices in the notifier tree. */
> > +static void v4l2_async_notifier_unbind_all_subdevs(
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + struct v4l2_subdev *sd, *tmp;
> >
> > list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
> > + struct v4l2_async_notifier *subdev_notifier =
> > + v4l2_async_get_subdev_notifier(sd);
> > +
> > + if (subdev_notifier)
> > + v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
> > +
> > v4l2_async_cleanup(sd);
> >
> > v4l2_async_notifier_call_unbind(notifier, sd, sd->asd);
> > +
> > + list_del(&sd->async_list);
> > + list_add(&sd->async_list, &subdev_list);
> > }
> >
> > - mutex_unlock(&list_lock);
> > + notifier->parent = NULL;
>
> Shouldn't notifier->v4l2_dev and notifier->sd be set to NULL as well?
> I can't really tell.
Well spotted. Yes. This was actually broken by the patch; v4l2_dev used to
be set NULL. Will fix.
>
> > +}
> > +
> > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> > +{
> > + if (!notifier->v4l2_dev && !notifier->sd)
> > + return;
> >
> > - notifier->v4l2_dev = NULL;
> > + mutex_lock(&list_lock);
> > +
> > + v4l2_async_notifier_unbind_all_subdevs(notifier);
> > +
> > + list_del(¬ifier->list);
> > +
> > + mutex_unlock(&list_lock);
> > }
> > EXPORT_SYMBOL(v4l2_async_notifier_unregister);
> >
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 3bc8a7c0d83f..cf409d45208c 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {
> > * @num_subdevs: number of subdevices used in the subdevs array
> > * @max_subdevs: number of subdevices allocated in the subdevs array
> > * @subdevs: array of pointers to subdevice descriptors
> > - * @v4l2_dev: pointer to struct v4l2_device
> > + * @v4l2_dev: v4l2_device of the root notifier, NULL otherwise
> > + * @sd: sub-device that registered the notifier, NULL otherwise
> > + * @parent: parent notifier carrying @v4l2_dev
>
> That's not correct, it only carries v4l2_dev if it is the root notifier.
> I think just 'parent notifier' is sufficient here.
Will change.
>
> > * @waiting: list of struct v4l2_async_subdev, waiting for their drivers
> > * @done: list of struct v4l2_subdev, already probed
> > * @list: member in a global list of notifiers
> > @@ -113,6 +115,8 @@ struct v4l2_async_notifier {
> > unsigned int max_subdevs;
> > struct v4l2_async_subdev **subdevs;
> > struct v4l2_device *v4l2_dev;
> > + struct v4l2_subdev *sd;
> > + struct v4l2_async_notifier *parent;
> > struct list_head waiting;
> > struct list_head done;
> > struct list_head list;
> > @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > struct v4l2_async_notifier *notifier);
> >
> > /**
> > + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous
> > + * notifier for a sub-device
> > + *
> > + * @sd: pointer to &struct v4l2_subdev
> > + * @notifier: pointer to &struct v4l2_async_notifier
> > + */
> > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
> > + struct v4l2_async_notifier *notifier);
> > +
> > +/**
> > * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier
> > *
> > * @notifier: pointer to &struct v4l2_async_notifier
> >
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing generic references
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 8:00 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
Add function v4l2_fwnode_reference_count() for counting external
references and v4l2_fwnode_reference_parse() for parsing them as async
sub-devices.
This can be done on e.g. flash or lens async sub-devices that are not part
of but are associated with a sensor.
struct v4l2_async_notifier.max_subdevs field is added to contain the
maximum number of sub-devices in a notifier to reflect the memory
allocated for the subdevs array.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index d978f2d714ca..4821c4989119 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
}
EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
+static int v4l2_fwnode_reference_parse(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ const char *prop)
+{
+ struct fwnode_reference_args args;
+ unsigned int index = 0;
+ int ret;
+
+ for (; !fwnode_property_get_reference_args(
+ dev_fwnode(dev), prop, NULL, 0, index, &args); index++)
+ fwnode_handle_put(args.fwnode);
+
+ ret = v4l2_async_notifier_realloc(notifier,
+ notifier->num_subdevs + index);
+ if (ret)
+ return -ENOMEM;
+
+ for (ret = -ENOENT, index = 0;
+ !fwnode_property_get_reference_args(
+ dev_fwnode(dev), prop, NULL, 0, index, &args);
+ index++) {
+ struct v4l2_async_subdev *asd;
+
+ if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ asd = kzalloc(sizeof(*asd), GFP_KERNEL);
+ if (!asd) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ notifier->subdevs[notifier->num_subdevs] = asd;
+ asd->match.fwnode.fwnode = args.fwnode;
+ asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+ notifier->num_subdevs++;
+ }
+
+ return 0;
+
+error:
+ fwnode_handle_put(args.fwnode);
+ return ret;
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>");
MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing generic references
@ 2017-09-11 8:00 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Add function v4l2_fwnode_reference_count() for counting external
references and v4l2_fwnode_reference_parse() for parsing them as async
sub-devices.
This can be done on e.g. flash or lens async sub-devices that are not part
of but are associated with a sensor.
struct v4l2_async_notifier.max_subdevs field is added to contain the
maximum number of sub-devices in a notifier to reflect the memory
allocated for the subdevs array.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index d978f2d714ca..4821c4989119 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
}
EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
+static int v4l2_fwnode_reference_parse(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ const char *prop)
+{
+ struct fwnode_reference_args args;
+ unsigned int index = 0;
+ int ret;
+
+ for (; !fwnode_property_get_reference_args(
+ dev_fwnode(dev), prop, NULL, 0, index, &args); index++)
+ fwnode_handle_put(args.fwnode);
+
+ ret = v4l2_async_notifier_realloc(notifier,
+ notifier->num_subdevs + index);
+ if (ret)
+ return -ENOMEM;
+
+ for (ret = -ENOENT, index = 0;
+ !fwnode_property_get_reference_args(
+ dev_fwnode(dev), prop, NULL, 0, index, &args);
+ index++) {
+ struct v4l2_async_subdev *asd;
+
+ if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ asd = kzalloc(sizeof(*asd), GFP_KERNEL);
+ if (!asd) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ notifier->subdevs[notifier->num_subdevs] = asd;
+ asd->match.fwnode.fwnode = args.fwnode;
+ asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+ notifier->num_subdevs++;
+ }
+
+ return 0;
+
+error:
+ fwnode_handle_put(args.fwnode);
+ return ret;
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing generic references
2017-09-11 8:00 ` Sakari Ailus
(?)
@ 2017-09-11 9:14 ` Hans Verkuil
2017-09-11 9:59 ` Sakari Ailus
-1 siblings, 1 reply; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 9:14 UTC (permalink / raw)
To: Sakari Ailus, linux-media
Cc: niklas.soderlund, robh, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> Add function v4l2_fwnode_reference_count() for counting external
> references and v4l2_fwnode_reference_parse() for parsing them as async
> sub-devices.
>
> This can be done on e.g. flash or lens async sub-devices that are not part
> of but are associated with a sensor.
>
> struct v4l2_async_notifier.max_subdevs field is added to contain the
> maximum number of sub-devices in a notifier to reflect the memory
> allocated for the subdevs array.
This paragraph appears to be out-of-date.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index d978f2d714ca..4821c4989119 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
> }
> EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
>
> +static int v4l2_fwnode_reference_parse(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + const char *prop)
> +{
> + struct fwnode_reference_args args;
> + unsigned int index = 0;
> + int ret;
> +
> + for (; !fwnode_property_get_reference_args(
> + dev_fwnode(dev), prop, NULL, 0, index, &args); index++)
> + fwnode_handle_put(args.fwnode);
> +
If nothing is found (i.e. index == 0), shouldn't you just return here?
> + ret = v4l2_async_notifier_realloc(notifier,
> + notifier->num_subdevs + index);
> + if (ret)
> + return -ENOMEM;
> +
> + for (ret = -ENOENT, index = 0;
There is no reason for the 'ret = -ENOENT' to be in the for(), just set it before
the 'for' statement.
> + !fwnode_property_get_reference_args(
> + dev_fwnode(dev), prop, NULL, 0, index, &args);
> + index++) {
> + struct v4l2_async_subdev *asd;
> +
> + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + asd = kzalloc(sizeof(*asd), GFP_KERNEL);
> + if (!asd) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + notifier->subdevs[notifier->num_subdevs] = asd;
> + asd->match.fwnode.fwnode = args.fwnode;
> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> + notifier->num_subdevs++;
> + }
If the loop doesn't find anything, then it still returns 0, not -ENOENT.
So why set ret to ENOENT? Something weird going on here.
I think you should also add a comment explaining this function.
> +
> + return 0;
> +
> +error:
> + fwnode_handle_put(args.fwnode);
> + return ret;
> +}
> +
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
> MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
>
Regards,
Hans
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 17/24] v4l: fwnode: Add a helper function for parsing generic references
2017-09-11 9:14 ` Hans Verkuil
@ 2017-09-11 9:59 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 9:59 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
Hi Hans,
Thanks for the review!
On Mon, Sep 11, 2017 at 11:14:03AM +0200, Hans Verkuil wrote:
> On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> > Add function v4l2_fwnode_reference_count() for counting external
> > references and v4l2_fwnode_reference_parse() for parsing them as async
> > sub-devices.
> >
> > This can be done on e.g. flash or lens async sub-devices that are not part
> > of but are associated with a sensor.
> >
> > struct v4l2_async_notifier.max_subdevs field is added to contain the
> > maximum number of sub-devices in a notifier to reflect the memory
> > allocated for the subdevs array.
>
> This paragraph appears to be out-of-date.
Will remove.
>
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 47 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index d978f2d714ca..4821c4989119 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -449,6 +449,53 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
> > }
> > EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
> >
> > +static int v4l2_fwnode_reference_parse(
> > + struct device *dev, struct v4l2_async_notifier *notifier,
> > + const char *prop)
> > +{
> > + struct fwnode_reference_args args;
> > + unsigned int index = 0;
> > + int ret;
> > +
> > + for (; !fwnode_property_get_reference_args(
> > + dev_fwnode(dev), prop, NULL, 0, index, &args); index++)
> > + fwnode_handle_put(args.fwnode);
> > +
>
> If nothing is found (i.e. index == 0), shouldn't you just return here?
You could. yes. It would actually simplify the code, I could just return
-ENOENT here.
>
> > + ret = v4l2_async_notifier_realloc(notifier,
> > + notifier->num_subdevs + index);
> > + if (ret)
> > + return -ENOMEM;
> > +
> > + for (ret = -ENOENT, index = 0;
>
> There is no reason for the 'ret = -ENOENT' to be in the for(), just set it before
> the 'for' statement.
>
> > + !fwnode_property_get_reference_args(
> > + dev_fwnode(dev), prop, NULL, 0, index, &args);
> > + index++) {
> > + struct v4l2_async_subdev *asd;
> > +
> > + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> > + ret = -EINVAL;
> > + goto error;
> > + }
> > +
> > + asd = kzalloc(sizeof(*asd), GFP_KERNEL);
> > + if (!asd) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > +
> > + notifier->subdevs[notifier->num_subdevs] = asd;
> > + asd->match.fwnode.fwnode = args.fwnode;
> > + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> > + notifier->num_subdevs++;
> > + }
>
> If the loop doesn't find anything, then it still returns 0, not -ENOENT.
> So why set ret to ENOENT? Something weird going on here.
:-)
I think -ENOENT would make sense indeed if there are no entries found.
>
> I think you should also add a comment explaining this function.
Yes. I had that in the header when it was exported, I'll add the same
comments here.
>
> > +
> > + return 0;
> > +
> > +error:
> > + fwnode_handle_put(args.fwnode);
> > + return ret;
> > +}
> > +
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
> > MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
> >
>
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 8:00 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
the device's own fwnode, it will follow child fwnodes with the given
property -- value pair and return the resulting fwnode.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 4821c4989119..56eee5bbd3b5 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
return ret;
}
+static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
+ struct fwnode_handle *fwnode, const char *prop, unsigned int index,
+ const char **props, unsigned int nprops)
+{
+ struct fwnode_reference_args fwnode_args;
+ unsigned int *args = fwnode_args.args;
+ struct fwnode_handle *child;
+ int ret;
+
+ ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
+ index, &fwnode_args);
+ if (ret)
+ return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
+
+ for (fwnode = fwnode_args.fwnode;
+ nprops; nprops--, fwnode = child, props++, args++) {
+ u32 val;
+
+ fwnode_for_each_child_node(fwnode, child) {
+ if (fwnode_property_read_u32(child, *props, &val))
+ continue;
+
+ if (val == *args)
+ break;
+ }
+
+ fwnode_handle_put(fwnode);
+
+ if (!child) {
+ fwnode = ERR_PTR(-ENOENT);
+ break;
+ }
+ }
+
+ return fwnode;
+}
+
+static int v4l2_fwnode_reference_parse_int_props(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ const char *prop, const char **props, unsigned int nprops)
+{
+ struct fwnode_handle *fwnode;
+ unsigned int index = 0;
+ int ret;
+
+ while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
+ dev_fwnode(dev), prop, index, props,
+ nprops)))) {
+ fwnode_handle_put(fwnode);
+ index++;
+ }
+
+ if (PTR_ERR(fwnode) != -ENOENT)
+ return PTR_ERR(fwnode);
+
+ ret = v4l2_async_notifier_realloc(notifier,
+ notifier->num_subdevs + index);
+ if (ret)
+ return -ENOMEM;
+
+ for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
+ dev_fwnode(dev), prop, index, props,
+ nprops))); ) {
+ struct v4l2_async_subdev *asd;
+
+ if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
+ if (!asd) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ notifier->subdevs[notifier->num_subdevs] = asd;
+ asd->match.fwnode.fwnode = fwnode;
+ asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+ notifier->num_subdevs++;
+
+ fwnode_handle_put(fwnode);
+
+ index++;
+ }
+
+ return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
+
+error:
+ fwnode_handle_put(fwnode);
+ return ret;
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>");
MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
@ 2017-09-11 8:00 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
the device's own fwnode, it will follow child fwnodes with the given
property -- value pair and return the resulting fwnode.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 4821c4989119..56eee5bbd3b5 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
return ret;
}
+static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
+ struct fwnode_handle *fwnode, const char *prop, unsigned int index,
+ const char **props, unsigned int nprops)
+{
+ struct fwnode_reference_args fwnode_args;
+ unsigned int *args = fwnode_args.args;
+ struct fwnode_handle *child;
+ int ret;
+
+ ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
+ index, &fwnode_args);
+ if (ret)
+ return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
+
+ for (fwnode = fwnode_args.fwnode;
+ nprops; nprops--, fwnode = child, props++, args++) {
+ u32 val;
+
+ fwnode_for_each_child_node(fwnode, child) {
+ if (fwnode_property_read_u32(child, *props, &val))
+ continue;
+
+ if (val == *args)
+ break;
+ }
+
+ fwnode_handle_put(fwnode);
+
+ if (!child) {
+ fwnode = ERR_PTR(-ENOENT);
+ break;
+ }
+ }
+
+ return fwnode;
+}
+
+static int v4l2_fwnode_reference_parse_int_props(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ const char *prop, const char **props, unsigned int nprops)
+{
+ struct fwnode_handle *fwnode;
+ unsigned int index = 0;
+ int ret;
+
+ while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
+ dev_fwnode(dev), prop, index, props,
+ nprops)))) {
+ fwnode_handle_put(fwnode);
+ index++;
+ }
+
+ if (PTR_ERR(fwnode) != -ENOENT)
+ return PTR_ERR(fwnode);
+
+ ret = v4l2_async_notifier_realloc(notifier,
+ notifier->num_subdevs + index);
+ if (ret)
+ return -ENOMEM;
+
+ for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
+ dev_fwnode(dev), prop, index, props,
+ nprops))); ) {
+ struct v4l2_async_subdev *asd;
+
+ if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
+ if (!asd) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ notifier->subdevs[notifier->num_subdevs] = asd;
+ asd->match.fwnode.fwnode = fwnode;
+ asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+ notifier->num_subdevs++;
+
+ fwnode_handle_put(fwnode);
+
+ index++;
+ }
+
+ return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
+
+error:
+ fwnode_handle_put(fwnode);
+ return ret;
+}
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 8:00 ` Sakari Ailus
(?)
@ 2017-09-11 9:38 ` Hans Verkuil
2017-09-11 12:28 ` Sakari Ailus
-1 siblings, 1 reply; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 9:38 UTC (permalink / raw)
To: Sakari Ailus, linux-media
Cc: niklas.soderlund, robh, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Typo in subject: interger -> integer
On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
> the device's own fwnode,
Sorry, you lost me here. Which device are we talking about?
> it will follow child fwnodes with the given
> property -- value pair and return the resulting fwnode.
property-value pair (easier readable that way).
You only describe v4l2_fwnode_reference_parse_int_prop(), not
v4l2_fwnode_reference_parse_int_props().
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 4821c4989119..56eee5bbd3b5 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
> return ret;
> }
>
> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
> + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
> + const char **props, unsigned int nprops)
Need comments describing what this does.
> +{
> + struct fwnode_reference_args fwnode_args;
> + unsigned int *args = fwnode_args.args;
> + struct fwnode_handle *child;
> + int ret;
> +
> + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
> + index, &fwnode_args);
> + if (ret)
> + return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.
> +
> + for (fwnode = fwnode_args.fwnode;
> + nprops; nprops--, fwnode = child, props++, args++) {
I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...
It's hard to parse.
I would make this a 'while (nprops)' and write out all the other assignments,
increments and decrements.
> + u32 val;
> +
> + fwnode_for_each_child_node(fwnode, child) {
> + if (fwnode_property_read_u32(child, *props, &val))
> + continue;
> +
> + if (val == *args)
> + break;
I'm lost. This really needs comments and perhaps even an DT or ACPI example
so you can see what exactly it is we're doing here.
> + }
> +
> + fwnode_handle_put(fwnode);
> +
> + if (!child) {
> + fwnode = ERR_PTR(-ENOENT);
> + break;
> + }
> + }
> +
> + return fwnode;
> +}
> +
> +static int v4l2_fwnode_reference_parse_int_props(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + const char *prop, const char **props, unsigned int nprops)
Needs comments describing what this does.
> +{
> + struct fwnode_handle *fwnode;
> + unsigned int index = 0;
> + int ret;
> +
> + while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> + dev_fwnode(dev), prop, index, props,
> + nprops)))) {
> + fwnode_handle_put(fwnode);
> + index++;
> + }
> +
> + if (PTR_ERR(fwnode) != -ENOENT)
> + return PTR_ERR(fwnode);
Missing 'if (index == 0)'?
> +
> + ret = v4l2_async_notifier_realloc(notifier,
> + notifier->num_subdevs + index);
> + if (ret)
> + return -ENOMEM;
> +
> + for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> + dev_fwnode(dev), prop, index, props,
> + nprops))); ) {
I'd add 'index++' in this for-loop. It's weird that it is missing.
> + struct v4l2_async_subdev *asd;
> +
> + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
> + if (!asd) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + notifier->subdevs[notifier->num_subdevs] = asd;
> + asd->match.fwnode.fwnode = fwnode;
> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> + notifier->num_subdevs++;
> +
> + fwnode_handle_put(fwnode);
> +
> + index++;
> + }
> +
> + return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
> +
> +error:
> + fwnode_handle_put(fwnode);
> + return ret;
> +}
> +
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
> MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
>
Regards,
Hans
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 9:38 ` Hans Verkuil
@ 2017-09-11 12:28 ` Sakari Ailus
2017-09-11 12:38 ` Hans Verkuil
0 siblings, 1 reply; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 12:28 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
Hi Hans,
Thanks for the review.
On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:
> Typo in subject: interger -> integer
>
> On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> > v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
> > the device's own fwnode,
>
> Sorry, you lost me here. Which device are we talking about?
The fwnode related a struct device, in other words what dev_fwnode(dev)
gives you. This is either struct device.fwnode or struct
device.of_node.fwnode, depending on which firmware interface was used to
create the device.
I'll add a note of this.
>
> > it will follow child fwnodes with the given
> > property -- value pair and return the resulting fwnode.
>
> property-value pair (easier readable that way).
>
> You only describe v4l2_fwnode_reference_parse_int_prop(), not
> v4l2_fwnode_reference_parse_int_props().
Yes, I think I changed the naming but forgot to update the commit. I'll do
that now.
>
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 93 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 4821c4989119..56eee5bbd3b5 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
> > return ret;
> > }
> >
> > +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
> > + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
> > + const char **props, unsigned int nprops)
>
> Need comments describing what this does.
Yes. I'll also rename it (get -> read) for consistency with the async
changes.
>
> > +{
> > + struct fwnode_reference_args fwnode_args;
> > + unsigned int *args = fwnode_args.args;
> > + struct fwnode_handle *child;
> > + int ret;
> > +
> > + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
> > + index, &fwnode_args);
> > + if (ret)
> > + return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
>
> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.
fwnode_property_get_reference_args() returns currently a little bit
different error codes in ACPI / DT. This is worth documenting there and
fixing as well.
>
> > +
> > + for (fwnode = fwnode_args.fwnode;
> > + nprops; nprops--, fwnode = child, props++, args++) {
>
> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...
> It's hard to parse.
Hmm. I'm not sure if that really helps; the function is just handling each
entry in the array and related array pointers are changed accordingly. The
fwnode = child assignment is there to move to the child node. I.e. what you
need for handling the loop itself.
I can change this though if you think it really makes a difference for
better.
>
> I would make this a 'while (nprops)' and write out all the other assignments,
> increments and decrements.
>
> > + u32 val;
> > +
> > + fwnode_for_each_child_node(fwnode, child) {
> > + if (fwnode_property_read_u32(child, *props, &val))
> > + continue;
> > +
> > + if (val == *args)
> > + break;
>
> I'm lost. This really needs comments and perhaps even an DT or ACPI example
> so you can see what exactly it is we're doing here.
I'll add comments to the code. A good example will be ACPI documentation
for LEDs, see 17th patch in v9. That will go through the linux-pm tree so
it won't be available in the same tree for a while.
>
> > + }
> > +
> > + fwnode_handle_put(fwnode);
> > +
> > + if (!child) {
> > + fwnode = ERR_PTR(-ENOENT);
> > + break;
> > + }
> > + }
> > +
> > + return fwnode;
> > +}
> > +
> > +static int v4l2_fwnode_reference_parse_int_props(
> > + struct device *dev, struct v4l2_async_notifier *notifier,
> > + const char *prop, const char **props, unsigned int nprops)
>
> Needs comments describing what this does.
Will add.
>
> > +{
> > + struct fwnode_handle *fwnode;
> > + unsigned int index = 0;
> > + int ret;
> > +
> > + while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> > + dev_fwnode(dev), prop, index, props,
> > + nprops)))) {
> > + fwnode_handle_put(fwnode);
> > + index++;
> > + }
> > +
> > + if (PTR_ERR(fwnode) != -ENOENT)
> > + return PTR_ERR(fwnode);
>
> Missing 'if (index == 0)'?
Yes, will add.
>
> > +
> > + ret = v4l2_async_notifier_realloc(notifier,
> > + notifier->num_subdevs + index);
> > + if (ret)
> > + return -ENOMEM;
> > +
> > + for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> > + dev_fwnode(dev), prop, index, props,
> > + nprops))); ) {
>
> I'd add 'index++' in this for-loop. It's weird that it is missing.
Agreed, I'll move it there.
>
> > + struct v4l2_async_subdev *asd;
> > +
> > + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> > + ret = -EINVAL;
> > + goto error;
> > + }
> > +
> > + asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
> > + if (!asd) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > +
> > + notifier->subdevs[notifier->num_subdevs] = asd;
> > + asd->match.fwnode.fwnode = fwnode;
> > + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> > + notifier->num_subdevs++;
> > +
> > + fwnode_handle_put(fwnode);
> > +
> > + index++;
> > + }
> > +
> > + return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
> > +
> > +error:
> > + fwnode_handle_put(fwnode);
> > + return ret;
> > +}
> > +
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
> > MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
> >
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 12:28 ` Sakari Ailus
@ 2017-09-11 12:38 ` Hans Verkuil
2017-09-11 13:27 ` Sakari Ailus
0 siblings, 1 reply; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 12:38 UTC (permalink / raw)
To: Sakari Ailus
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
On 09/11/2017 02:28 PM, Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for the review.
>
> On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:
>> Typo in subject: interger -> integer
>>
>> On 09/11/2017 10:00 AM, Sakari Ailus wrote:
>>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
>>> the device's own fwnode,
>>
>> Sorry, you lost me here. Which device are we talking about?
>
> The fwnode related a struct device, in other words what dev_fwnode(dev)
> gives you. This is either struct device.fwnode or struct
> device.of_node.fwnode, depending on which firmware interface was used to
> create the device.
>
> I'll add a note of this.
>
>>
>>> it will follow child fwnodes with the given
>>> property -- value pair and return the resulting fwnode.
>>
>> property-value pair (easier readable that way).
>>
>> You only describe v4l2_fwnode_reference_parse_int_prop(), not
>> v4l2_fwnode_reference_parse_int_props().
>
> Yes, I think I changed the naming but forgot to update the commit. I'll do
> that now.
>
>>
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 93 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> index 4821c4989119..56eee5bbd3b5 100644
>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
>>> return ret;
>>> }
>>>
>>> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
>>> + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
>>> + const char **props, unsigned int nprops)
>>
>> Need comments describing what this does.
>
> Yes. I'll also rename it (get -> read) for consistency with the async
> changes.
Which async changes? Since the fwnode_handle that's returned is refcounted
I wonder if 'get' isn't the right name in this case.
>
>>
>>> +{
>>> + struct fwnode_reference_args fwnode_args;
>>> + unsigned int *args = fwnode_args.args;
>>> + struct fwnode_handle *child;
>>> + int ret;
>>> +
>>> + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
>>> + index, &fwnode_args);
>>> + if (ret)
>>> + return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
>>
>> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.
>
> fwnode_property_get_reference_args() returns currently a little bit
> different error codes in ACPI / DT. This is worth documenting there and
> fixing as well.
>
>>
>>> +
>>> + for (fwnode = fwnode_args.fwnode;
>>> + nprops; nprops--, fwnode = child, props++, args++) {
>>
>> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...
>> It's hard to parse.
>
> Hmm. I'm not sure if that really helps; the function is just handling each
> entry in the array and related array pointers are changed accordingly. The
> fwnode = child assignment is there to move to the child node. I.e. what you
> need for handling the loop itself.
>
> I can change this though if you think it really makes a difference for
> better.
I think so, yes. I noticed you like complex for-loops :-)
>
>>
>> I would make this a 'while (nprops)' and write out all the other assignments,
>> increments and decrements.
>>
>>> + u32 val;
>>> +
>>> + fwnode_for_each_child_node(fwnode, child) {
>>> + if (fwnode_property_read_u32(child, *props, &val))
>>> + continue;
>>> +
>>> + if (val == *args)
>>> + break;
>>
>> I'm lost. This really needs comments and perhaps even an DT or ACPI example
>> so you can see what exactly it is we're doing here.
>
> I'll add comments to the code. A good example will be ACPI documentation
> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so
> it won't be available in the same tree for a while.
Ideally an ACPI and an equivalent DT example would be nice to have, but I might
be asking too much. I'm not that familiar with ACPI, so for me a DT example
is easier.
>
>>
>>> + }
>>> +
>>> + fwnode_handle_put(fwnode);
>>> +
>>> + if (!child) {
>>> + fwnode = ERR_PTR(-ENOENT);
>>> + break;
>>> + }
>>> + }
>>> +
>>> + return fwnode;
>>> +}
>>> +
>>> +static int v4l2_fwnode_reference_parse_int_props(
>>> + struct device *dev, struct v4l2_async_notifier *notifier,
>>> + const char *prop, const char **props, unsigned int nprops)
>>
>> Needs comments describing what this does.
>
> Will add.
>
>>
>>> +{
>>> + struct fwnode_handle *fwnode;
>>> + unsigned int index = 0;
>>> + int ret;
>>> +
>>> + while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
>>> + dev_fwnode(dev), prop, index, props,
>>> + nprops)))) {
>>> + fwnode_handle_put(fwnode);
>>> + index++;
>>> + }
>>> +
>>> + if (PTR_ERR(fwnode) != -ENOENT)
>>> + return PTR_ERR(fwnode);
>>
>> Missing 'if (index == 0)'?
>
> Yes, will add.
>
>>
>>> +
>>> + ret = v4l2_async_notifier_realloc(notifier,
>>> + notifier->num_subdevs + index);
>>> + if (ret)
>>> + return -ENOMEM;
>>> +
>>> + for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
>>> + dev_fwnode(dev), prop, index, props,
>>> + nprops))); ) {
>>
>> I'd add 'index++' in this for-loop. It's weird that it is missing.
>
> Agreed, I'll move it there.
>
>>
>>> + struct v4l2_async_subdev *asd;
>>> +
>>> + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
>>> + ret = -EINVAL;
>>> + goto error;
>>> + }
>>> +
>>> + asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
>>> + if (!asd) {
>>> + ret = -ENOMEM;
>>> + goto error;
>>> + }
>>> +
>>> + notifier->subdevs[notifier->num_subdevs] = asd;
>>> + asd->match.fwnode.fwnode = fwnode;
>>> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
>>> + notifier->num_subdevs++;
>>> +
>>> + fwnode_handle_put(fwnode);
>>> +
>>> + index++;
>>> + }
>>> +
>>> + return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
>>> +
>>> +error:
>>> + fwnode_handle_put(fwnode);
>>> + return ret;
>>> +}
>>> +
>>> MODULE_LICENSE("GPL");
>>> MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
>>> MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
>>>
>
Regards,
Hans
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 12:38 ` Hans Verkuil
@ 2017-09-11 13:27 ` Sakari Ailus
2017-09-11 13:34 ` Hans Verkuil
0 siblings, 1 reply; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 13:27 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
Hi Hans,
On Mon, Sep 11, 2017 at 02:38:23PM +0200, Hans Verkuil wrote:
> On 09/11/2017 02:28 PM, Sakari Ailus wrote:
> > Hi Hans,
> >
> > Thanks for the review.
> >
> > On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:
> >> Typo in subject: interger -> integer
> >>
> >> On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> >>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
> >>> the device's own fwnode,
> >>
> >> Sorry, you lost me here. Which device are we talking about?
> >
> > The fwnode related a struct device, in other words what dev_fwnode(dev)
> > gives you. This is either struct device.fwnode or struct
> > device.of_node.fwnode, depending on which firmware interface was used to
> > create the device.
> >
> > I'll add a note of this.
> >
> >>
> >>> it will follow child fwnodes with the given
> >>> property -- value pair and return the resulting fwnode.
> >>
> >> property-value pair (easier readable that way).
> >>
> >> You only describe v4l2_fwnode_reference_parse_int_prop(), not
> >> v4l2_fwnode_reference_parse_int_props().
> >
> > Yes, I think I changed the naming but forgot to update the commit. I'll do
> > that now.
> >
> >>
> >>>
> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> ---
> >>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 93 insertions(+)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> index 4821c4989119..56eee5bbd3b5 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
> >>> return ret;
> >>> }
> >>>
> >>> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
> >>> + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
> >>> + const char **props, unsigned int nprops)
> >>
> >> Need comments describing what this does.
> >
> > Yes. I'll also rename it (get -> read) for consistency with the async
> > changes.
>
> Which async changes? Since the fwnode_handle that's returned is refcounted
> I wonder if 'get' isn't the right name in this case.
Right. True. I'll leave that as-is then.
>
> >
> >>
> >>> +{
> >>> + struct fwnode_reference_args fwnode_args;
> >>> + unsigned int *args = fwnode_args.args;
> >>> + struct fwnode_handle *child;
> >>> + int ret;
> >>> +
> >>> + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
> >>> + index, &fwnode_args);
> >>> + if (ret)
> >>> + return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
> >>
> >> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.
> >
> > fwnode_property_get_reference_args() returns currently a little bit
> > different error codes in ACPI / DT. This is worth documenting there and
> > fixing as well.
> >
> >>
> >>> +
> >>> + for (fwnode = fwnode_args.fwnode;
> >>> + nprops; nprops--, fwnode = child, props++, args++) {
> >>
> >> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...
> >> It's hard to parse.
> >
> > Hmm. I'm not sure if that really helps; the function is just handling each
> > entry in the array and related array pointers are changed accordingly. The
> > fwnode = child assignment is there to move to the child node. I.e. what you
> > need for handling the loop itself.
> >
> > I can change this though if you think it really makes a difference for
> > better.
>
> I think so, yes. I noticed you like complex for-loops :-)
I don't really see a difference. The loop increment will just move at the
end of the block inside the loop.
>
> >
> >>
> >> I would make this a 'while (nprops)' and write out all the other assignments,
> >> increments and decrements.
> >>
> >>> + u32 val;
> >>> +
> >>> + fwnode_for_each_child_node(fwnode, child) {
> >>> + if (fwnode_property_read_u32(child, *props, &val))
> >>> + continue;
> >>> +
> >>> + if (val == *args)
> >>> + break;
> >>
> >> I'm lost. This really needs comments and perhaps even an DT or ACPI example
> >> so you can see what exactly it is we're doing here.
> >
> > I'll add comments to the code. A good example will be ACPI documentation
> > for LEDs, see 17th patch in v9. That will go through the linux-pm tree so
> > it won't be available in the same tree for a while.
>
> Ideally an ACPI and an equivalent DT example would be nice to have, but I might
> be asking too much. I'm not that familiar with ACPI, so for me a DT example
> is easier.
This won't be useful on DT although you could technically use it. In DT you
can directly refer to any node but on ACPI you can just refer to devices,
hence this.
Would you be happy with the leds.txt example? I think it's a good example
as it's directly related to this.
>
> >
> >>
> >>> + }
> >>> +
> >>> + fwnode_handle_put(fwnode);
> >>> +
> >>> + if (!child) {
> >>> + fwnode = ERR_PTR(-ENOENT);
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + return fwnode;
> >>> +}
> >>> +
> >>> +static int v4l2_fwnode_reference_parse_int_props(
> >>> + struct device *dev, struct v4l2_async_notifier *notifier,
> >>> + const char *prop, const char **props, unsigned int nprops)
> >>
> >> Needs comments describing what this does.
> >
> > Will add.
> >
> >>
> >>> +{
> >>> + struct fwnode_handle *fwnode;
> >>> + unsigned int index = 0;
> >>> + int ret;
> >>> +
> >>> + while (!IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> >>> + dev_fwnode(dev), prop, index, props,
> >>> + nprops)))) {
> >>> + fwnode_handle_put(fwnode);
> >>> + index++;
> >>> + }
> >>> +
> >>> + if (PTR_ERR(fwnode) != -ENOENT)
> >>> + return PTR_ERR(fwnode);
> >>
> >> Missing 'if (index == 0)'?
> >
> > Yes, will add.
> >
> >>
> >>> +
> >>> + ret = v4l2_async_notifier_realloc(notifier,
> >>> + notifier->num_subdevs + index);
> >>> + if (ret)
> >>> + return -ENOMEM;
> >>> +
> >>> + for (index = 0; !IS_ERR((fwnode = v4l2_fwnode_reference_get_int_prop(
> >>> + dev_fwnode(dev), prop, index, props,
> >>> + nprops))); ) {
> >>
> >> I'd add 'index++' in this for-loop. It's weird that it is missing.
> >
> > Agreed, I'll move it there.
> >
> >>
> >>> + struct v4l2_async_subdev *asd;
> >>> +
> >>> + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> + }
> >>> +
> >>> + asd = kzalloc(sizeof(struct v4l2_async_subdev), GFP_KERNEL);
> >>> + if (!asd) {
> >>> + ret = -ENOMEM;
> >>> + goto error;
> >>> + }
> >>> +
> >>> + notifier->subdevs[notifier->num_subdevs] = asd;
> >>> + asd->match.fwnode.fwnode = fwnode;
> >>> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> >>> + notifier->num_subdevs++;
> >>> +
> >>> + fwnode_handle_put(fwnode);
> >>> +
> >>> + index++;
> >>> + }
> >>> +
> >>> + return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
> >>> +
> >>> +error:
> >>> + fwnode_handle_put(fwnode);
> >>> + return ret;
> >>> +}
> >>> +
> >>> MODULE_LICENSE("GPL");
> >>> MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
> >>> MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
> >>>
> >
>
> Regards,
>
> Hans
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 13:27 ` Sakari Ailus
@ 2017-09-11 13:34 ` Hans Verkuil
2017-09-11 14:12 ` Sakari Ailus
0 siblings, 1 reply; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 13:34 UTC (permalink / raw)
To: Sakari Ailus
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
On 09/11/2017 03:27 PM, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Sep 11, 2017 at 02:38:23PM +0200, Hans Verkuil wrote:
>> On 09/11/2017 02:28 PM, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> Thanks for the review.
>>>
>>> On Mon, Sep 11, 2017 at 11:38:58AM +0200, Hans Verkuil wrote:
>>>> Typo in subject: interger -> integer
>>>>
>>>> On 09/11/2017 10:00 AM, Sakari Ailus wrote:
>>>>> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
>>>>> the device's own fwnode,
>>>>
>>>> Sorry, you lost me here. Which device are we talking about?
>>>
>>> The fwnode related a struct device, in other words what dev_fwnode(dev)
>>> gives you. This is either struct device.fwnode or struct
>>> device.of_node.fwnode, depending on which firmware interface was used to
>>> create the device.
>>>
>>> I'll add a note of this.
>>>
>>>>
>>>>> it will follow child fwnodes with the given
>>>>> property -- value pair and return the resulting fwnode.
>>>>
>>>> property-value pair (easier readable that way).
>>>>
>>>> You only describe v4l2_fwnode_reference_parse_int_prop(), not
>>>> v4l2_fwnode_reference_parse_int_props().
>>>
>>> Yes, I think I changed the naming but forgot to update the commit. I'll do
>>> that now.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>>> ---
>>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 93 +++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 93 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
>>>>> index 4821c4989119..56eee5bbd3b5 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>>>> @@ -496,6 +496,99 @@ static int v4l2_fwnode_reference_parse(
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
>>>>> + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
>>>>> + const char **props, unsigned int nprops)
>>>>
>>>> Need comments describing what this does.
>>>
>>> Yes. I'll also rename it (get -> read) for consistency with the async
>>> changes.
>>
>> Which async changes? Since the fwnode_handle that's returned is refcounted
>> I wonder if 'get' isn't the right name in this case.
>
> Right. True. I'll leave that as-is then.
>
>>
>>>
>>>>
>>>>> +{
>>>>> + struct fwnode_reference_args fwnode_args;
>>>>> + unsigned int *args = fwnode_args.args;
>>>>> + struct fwnode_handle *child;
>>>>> + int ret;
>>>>> +
>>>>> + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
>>>>> + index, &fwnode_args);
>>>>> + if (ret)
>>>>> + return ERR_PTR(ret == -EINVAL ? -ENOENT : ret);
>>>>
>>>> Why map EINVAL to ENOENT? Needs a comment, either here or in the function description.
>>>
>>> fwnode_property_get_reference_args() returns currently a little bit
>>> different error codes in ACPI / DT. This is worth documenting there and
>>> fixing as well.
>>>
>>>>
>>>>> +
>>>>> + for (fwnode = fwnode_args.fwnode;
>>>>> + nprops; nprops--, fwnode = child, props++, args++) {
>>>>
>>>> I think you cram too much in this for-loop: fwnode, nprops, fwnode, props, args...
>>>> It's hard to parse.
>>>
>>> Hmm. I'm not sure if that really helps; the function is just handling each
>>> entry in the array and related array pointers are changed accordingly. The
>>> fwnode = child assignment is there to move to the child node. I.e. what you
>>> need for handling the loop itself.
>>>
>>> I can change this though if you think it really makes a difference for
>>> better.
>>
>> I think so, yes. I noticed you like complex for-loops :-)
>
> I don't really see a difference. The loop increment will just move at the
> end of the block inside the loop.
>
>>
>>>
>>>>
>>>> I would make this a 'while (nprops)' and write out all the other assignments,
>>>> increments and decrements.
>>>>
>>>>> + u32 val;
>>>>> +
>>>>> + fwnode_for_each_child_node(fwnode, child) {
>>>>> + if (fwnode_property_read_u32(child, *props, &val))
>>>>> + continue;
>>>>> +
>>>>> + if (val == *args)
>>>>> + break;
>>>>
>>>> I'm lost. This really needs comments and perhaps even an DT or ACPI example
>>>> so you can see what exactly it is we're doing here.
>>>
>>> I'll add comments to the code. A good example will be ACPI documentation
>>> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so
>>> it won't be available in the same tree for a while.
>>
>> Ideally an ACPI and an equivalent DT example would be nice to have, but I might
>> be asking too much. I'm not that familiar with ACPI, so for me a DT example
>> is easier.
>
> This won't be useful on DT although you could technically use it. In DT you
> can directly refer to any node but on ACPI you can just refer to devices,
> hence this.
So this function will effectively only be used with acpi? That should be
documented. I think that explains some of my confusion since I was trying
to map this code to a device tree, without much success.
> Would you be happy with the leds.txt example? I think it's a good example
> as it's directly related to this.
Yes, that will work.
Regards,
Hans
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH v10 18/24] v4l: fwnode: Add a helper function to obtain device / interger references
2017-09-11 13:34 ` Hans Verkuil
@ 2017-09-11 14:12 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 14:12 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sakari Ailus, linux-media, niklas.soderlund, robh,
laurent.pinchart, linux-acpi, mika.westerberg, devicetree, pavel,
sre
On Mon, Sep 11, 2017 at 03:34:16PM +0200, Hans Verkuil wrote:
> >>>>> + u32 val;
> >>>>> +
> >>>>> + fwnode_for_each_child_node(fwnode, child) {
> >>>>> + if (fwnode_property_read_u32(child, *props, &val))
> >>>>> + continue;
> >>>>> +
> >>>>> + if (val == *args)
> >>>>> + break;
> >>>>
> >>>> I'm lost. This really needs comments and perhaps even an DT or ACPI example
> >>>> so you can see what exactly it is we're doing here.
> >>>
> >>> I'll add comments to the code. A good example will be ACPI documentation
> >>> for LEDs, see 17th patch in v9. That will go through the linux-pm tree so
> >>> it won't be available in the same tree for a while.
> >>
> >> Ideally an ACPI and an equivalent DT example would be nice to have, but I might
> >> be asking too much. I'm not that familiar with ACPI, so for me a DT example
> >> is easier.
> >
> > This won't be useful on DT although you could technically use it. In DT you
> > can directly refer to any node but on ACPI you can just refer to devices,
> > hence this.
>
> So this function will effectively only be used with acpi? That should be
> documented. I think that explains some of my confusion since I was trying
> to map this code to a device tree, without much success.
I'll add to the documentation of the function:
* While it is technically possible to use this function on DT, it is only
* meaningful on ACPI. On Device tree you can refer to any node in the tree but
* on ACPI the references are limited to devices.
>
> > Would you be happy with the leds.txt example? I think it's a good example
> > as it's directly related to this.
>
> Yes, that will work.
I'll add a separate patch that I'll post later on. The ACPI documentation
should get merged first.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH v10 21/24] smiapp: Add support for flash and lens devices
2017-09-11 7:59 ` Sakari Ailus
@ 2017-09-11 8:00 ` Sakari Ailus
-1 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw,
robh-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
mika.westerberg-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I,
sre-DgEjT+Ai2ygdnm+yROfE0A
Parse async sub-devices by using
v4l2_subdev_fwnode_reference_parse_sensor_common().
These types devices aren't directly related to the sensor, but are
nevertheless handled by the smiapp driver due to the relationship of these
component to the main part of the camera module --- the sensor.
This does not yet address providing the user space with information on how
to associate the sensor or lens devices but the kernel now has the
necessary information to do that.
Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/media/i2c/smiapp/smiapp-core.c | 38 +++++++++++++++++++++++++++-------
drivers/media/i2c/smiapp/smiapp.h | 4 +++-
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 700f433261d0..a65a839135d2 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -31,7 +31,7 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/smiapp.h>
-#include <linux/v4l2-mediabus.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-device.h>
@@ -2887,17 +2887,24 @@ static int smiapp_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&sensor->src->sd, client, &smiapp_ops);
sensor->src->sd.internal_ops = &smiapp_internal_src_ops;
+ rval = v4l2_fwnode_reference_parse_sensor_common(
+ &client->dev, &sensor->notifier);
+ if (rval < 0)
+ return rval;
+
sensor->vana = devm_regulator_get(&client->dev, "vana");
if (IS_ERR(sensor->vana)) {
dev_err(&client->dev, "could not get regulator for vana\n");
- return PTR_ERR(sensor->vana);
+ rval = PTR_ERR(sensor->vana);
+ goto out_release_async_notifier;
}
sensor->ext_clk = devm_clk_get(&client->dev, NULL);
if (IS_ERR(sensor->ext_clk)) {
dev_err(&client->dev, "could not get clock (%ld)\n",
PTR_ERR(sensor->ext_clk));
- return -EPROBE_DEFER;
+ rval = -EPROBE_DEFER;
+ goto out_release_async_notifier;
}
rval = clk_set_rate(sensor->ext_clk, sensor->hwcfg->ext_clk);
@@ -2905,17 +2912,19 @@ static int smiapp_probe(struct i2c_client *client,
dev_err(&client->dev,
"unable to set clock freq to %u\n",
sensor->hwcfg->ext_clk);
- return rval;
+ goto out_release_async_notifier;
}
sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
GPIOD_OUT_LOW);
- if (IS_ERR(sensor->xshutdown))
- return PTR_ERR(sensor->xshutdown);
+ if (IS_ERR(sensor->xshutdown)) {
+ rval = PTR_ERR(sensor->xshutdown);
+ goto out_release_async_notifier;
+ }
rval = smiapp_power_on(&client->dev);
if (rval < 0)
- return rval;
+ goto out_release_async_notifier;
rval = smiapp_identify_module(sensor);
if (rval) {
@@ -3092,9 +3101,14 @@ static int smiapp_probe(struct i2c_client *client,
if (rval < 0)
goto out_media_entity_cleanup;
+ rval = v4l2_async_subdev_notifier_register(&sensor->src->sd,
+ &sensor->notifier);
+ if (rval)
+ goto out_media_entity_cleanup;
+
rval = v4l2_async_register_subdev(&sensor->src->sd);
if (rval < 0)
- goto out_media_entity_cleanup;
+ goto out_unregister_async_notifier;
pm_runtime_set_active(&client->dev);
pm_runtime_get_noresume(&client->dev);
@@ -3105,6 +3119,9 @@ static int smiapp_probe(struct i2c_client *client,
return 0;
+out_unregister_async_notifier:
+ v4l2_async_notifier_unregister(&sensor->notifier);
+
out_media_entity_cleanup:
media_entity_cleanup(&sensor->src->sd.entity);
@@ -3114,6 +3131,9 @@ static int smiapp_probe(struct i2c_client *client,
out_power_off:
smiapp_power_off(&client->dev);
+out_release_async_notifier:
+ v4l2_async_notifier_release(&sensor->notifier);
+
return rval;
}
@@ -3124,6 +3144,8 @@ static int smiapp_remove(struct i2c_client *client)
unsigned int i;
v4l2_async_unregister_subdev(subdev);
+ v4l2_async_notifier_unregister(&sensor->notifier);
+ v4l2_async_notifier_release(&sensor->notifier);
pm_runtime_disable(&client->dev);
if (!pm_runtime_status_suspended(&client->dev))
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index f74d695018b9..be92cb5713f4 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -20,9 +20,10 @@
#define __SMIAPP_PRIV_H_
#include <linux/mutex.h>
+#include <media/i2c/smiapp.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-subdev.h>
-#include <media/i2c/smiapp.h>
#include "smiapp-pll.h"
#include "smiapp-reg.h"
@@ -172,6 +173,7 @@ struct smiapp_subdev {
* struct smiapp_sensor - Main device structure
*/
struct smiapp_sensor {
+ struct v4l2_async_notifier notifier;
/*
* "mutex" is used to serialise access to all fields here
* except v4l2_ctrls at the end of the struct. "mutex" is also
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH v10 21/24] smiapp: Add support for flash and lens devices
@ 2017-09-11 8:00 ` Sakari Ailus
0 siblings, 0 replies; 68+ messages in thread
From: Sakari Ailus @ 2017-09-11 8:00 UTC (permalink / raw)
To: linux-media
Cc: niklas.soderlund, robh, hverkuil, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
Parse async sub-devices by using
v4l2_subdev_fwnode_reference_parse_sensor_common().
These types devices aren't directly related to the sensor, but are
nevertheless handled by the smiapp driver due to the relationship of these
component to the main part of the camera module --- the sensor.
This does not yet address providing the user space with information on how
to associate the sensor or lens devices but the kernel now has the
necessary information to do that.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/smiapp/smiapp-core.c | 38 +++++++++++++++++++++++++++-------
drivers/media/i2c/smiapp/smiapp.h | 4 +++-
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 700f433261d0..a65a839135d2 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -31,7 +31,7 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/smiapp.h>
-#include <linux/v4l2-mediabus.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-device.h>
@@ -2887,17 +2887,24 @@ static int smiapp_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&sensor->src->sd, client, &smiapp_ops);
sensor->src->sd.internal_ops = &smiapp_internal_src_ops;
+ rval = v4l2_fwnode_reference_parse_sensor_common(
+ &client->dev, &sensor->notifier);
+ if (rval < 0)
+ return rval;
+
sensor->vana = devm_regulator_get(&client->dev, "vana");
if (IS_ERR(sensor->vana)) {
dev_err(&client->dev, "could not get regulator for vana\n");
- return PTR_ERR(sensor->vana);
+ rval = PTR_ERR(sensor->vana);
+ goto out_release_async_notifier;
}
sensor->ext_clk = devm_clk_get(&client->dev, NULL);
if (IS_ERR(sensor->ext_clk)) {
dev_err(&client->dev, "could not get clock (%ld)\n",
PTR_ERR(sensor->ext_clk));
- return -EPROBE_DEFER;
+ rval = -EPROBE_DEFER;
+ goto out_release_async_notifier;
}
rval = clk_set_rate(sensor->ext_clk, sensor->hwcfg->ext_clk);
@@ -2905,17 +2912,19 @@ static int smiapp_probe(struct i2c_client *client,
dev_err(&client->dev,
"unable to set clock freq to %u\n",
sensor->hwcfg->ext_clk);
- return rval;
+ goto out_release_async_notifier;
}
sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
GPIOD_OUT_LOW);
- if (IS_ERR(sensor->xshutdown))
- return PTR_ERR(sensor->xshutdown);
+ if (IS_ERR(sensor->xshutdown)) {
+ rval = PTR_ERR(sensor->xshutdown);
+ goto out_release_async_notifier;
+ }
rval = smiapp_power_on(&client->dev);
if (rval < 0)
- return rval;
+ goto out_release_async_notifier;
rval = smiapp_identify_module(sensor);
if (rval) {
@@ -3092,9 +3101,14 @@ static int smiapp_probe(struct i2c_client *client,
if (rval < 0)
goto out_media_entity_cleanup;
+ rval = v4l2_async_subdev_notifier_register(&sensor->src->sd,
+ &sensor->notifier);
+ if (rval)
+ goto out_media_entity_cleanup;
+
rval = v4l2_async_register_subdev(&sensor->src->sd);
if (rval < 0)
- goto out_media_entity_cleanup;
+ goto out_unregister_async_notifier;
pm_runtime_set_active(&client->dev);
pm_runtime_get_noresume(&client->dev);
@@ -3105,6 +3119,9 @@ static int smiapp_probe(struct i2c_client *client,
return 0;
+out_unregister_async_notifier:
+ v4l2_async_notifier_unregister(&sensor->notifier);
+
out_media_entity_cleanup:
media_entity_cleanup(&sensor->src->sd.entity);
@@ -3114,6 +3131,9 @@ static int smiapp_probe(struct i2c_client *client,
out_power_off:
smiapp_power_off(&client->dev);
+out_release_async_notifier:
+ v4l2_async_notifier_release(&sensor->notifier);
+
return rval;
}
@@ -3124,6 +3144,8 @@ static int smiapp_remove(struct i2c_client *client)
unsigned int i;
v4l2_async_unregister_subdev(subdev);
+ v4l2_async_notifier_unregister(&sensor->notifier);
+ v4l2_async_notifier_release(&sensor->notifier);
pm_runtime_disable(&client->dev);
if (!pm_runtime_status_suspended(&client->dev))
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index f74d695018b9..be92cb5713f4 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -20,9 +20,10 @@
#define __SMIAPP_PRIV_H_
#include <linux/mutex.h>
+#include <media/i2c/smiapp.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-subdev.h>
-#include <media/i2c/smiapp.h>
#include "smiapp-pll.h"
#include "smiapp-reg.h"
@@ -172,6 +173,7 @@ struct smiapp_subdev {
* struct smiapp_sensor - Main device structure
*/
struct smiapp_sensor {
+ struct v4l2_async_notifier notifier;
/*
* "mutex" is used to serialise access to all fields here
* except v4l2_ctrls at the end of the struct. "mutex" is also
--
2.11.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH v10 21/24] smiapp: Add support for flash and lens devices
2017-09-11 8:00 ` Sakari Ailus
(?)
@ 2017-09-11 9:48 ` Hans Verkuil
-1 siblings, 0 replies; 68+ messages in thread
From: Hans Verkuil @ 2017-09-11 9:48 UTC (permalink / raw)
To: Sakari Ailus, linux-media
Cc: niklas.soderlund, robh, laurent.pinchart, linux-acpi,
mika.westerberg, devicetree, pavel, sre
On 09/11/2017 10:00 AM, Sakari Ailus wrote:
> Parse async sub-devices by using
> v4l2_subdev_fwnode_reference_parse_sensor_common().
>
> These types devices aren't directly related to the sensor, but are
> nevertheless handled by the smiapp driver due to the relationship of these
> component to the main part of the camera module --- the sensor.
>
> This does not yet address providing the user space with information on how
> to associate the sensor or lens devices but the kernel now has the
> necessary information to do that.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
> ---
> drivers/media/i2c/smiapp/smiapp-core.c | 38 +++++++++++++++++++++++++++-------
> drivers/media/i2c/smiapp/smiapp.h | 4 +++-
> 2 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 700f433261d0..a65a839135d2 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -31,7 +31,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> #include <linux/smiapp.h>
> -#include <linux/v4l2-mediabus.h>
> +#include <media/v4l2-async.h>
> #include <media/v4l2-fwnode.h>
> #include <media/v4l2-device.h>
>
> @@ -2887,17 +2887,24 @@ static int smiapp_probe(struct i2c_client *client,
> v4l2_i2c_subdev_init(&sensor->src->sd, client, &smiapp_ops);
> sensor->src->sd.internal_ops = &smiapp_internal_src_ops;
>
> + rval = v4l2_fwnode_reference_parse_sensor_common(
> + &client->dev, &sensor->notifier);
> + if (rval < 0)
> + return rval;
> +
> sensor->vana = devm_regulator_get(&client->dev, "vana");
> if (IS_ERR(sensor->vana)) {
> dev_err(&client->dev, "could not get regulator for vana\n");
> - return PTR_ERR(sensor->vana);
> + rval = PTR_ERR(sensor->vana);
> + goto out_release_async_notifier;
> }
>
> sensor->ext_clk = devm_clk_get(&client->dev, NULL);
> if (IS_ERR(sensor->ext_clk)) {
> dev_err(&client->dev, "could not get clock (%ld)\n",
> PTR_ERR(sensor->ext_clk));
> - return -EPROBE_DEFER;
> + rval = -EPROBE_DEFER;
> + goto out_release_async_notifier;
> }
>
> rval = clk_set_rate(sensor->ext_clk, sensor->hwcfg->ext_clk);
> @@ -2905,17 +2912,19 @@ static int smiapp_probe(struct i2c_client *client,
> dev_err(&client->dev,
> "unable to set clock freq to %u\n",
> sensor->hwcfg->ext_clk);
> - return rval;
> + goto out_release_async_notifier;
> }
>
> sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
> GPIOD_OUT_LOW);
> - if (IS_ERR(sensor->xshutdown))
> - return PTR_ERR(sensor->xshutdown);
> + if (IS_ERR(sensor->xshutdown)) {
> + rval = PTR_ERR(sensor->xshutdown);
> + goto out_release_async_notifier;
> + }
>
> rval = smiapp_power_on(&client->dev);
> if (rval < 0)
> - return rval;
> + goto out_release_async_notifier;
>
> rval = smiapp_identify_module(sensor);
> if (rval) {
> @@ -3092,9 +3101,14 @@ static int smiapp_probe(struct i2c_client *client,
> if (rval < 0)
> goto out_media_entity_cleanup;
>
> + rval = v4l2_async_subdev_notifier_register(&sensor->src->sd,
> + &sensor->notifier);
> + if (rval)
> + goto out_media_entity_cleanup;
> +
> rval = v4l2_async_register_subdev(&sensor->src->sd);
> if (rval < 0)
> - goto out_media_entity_cleanup;
> + goto out_unregister_async_notifier;
>
> pm_runtime_set_active(&client->dev);
> pm_runtime_get_noresume(&client->dev);
> @@ -3105,6 +3119,9 @@ static int smiapp_probe(struct i2c_client *client,
>
> return 0;
>
> +out_unregister_async_notifier:
> + v4l2_async_notifier_unregister(&sensor->notifier);
> +
> out_media_entity_cleanup:
> media_entity_cleanup(&sensor->src->sd.entity);
>
> @@ -3114,6 +3131,9 @@ static int smiapp_probe(struct i2c_client *client,
> out_power_off:
> smiapp_power_off(&client->dev);
>
> +out_release_async_notifier:
> + v4l2_async_notifier_release(&sensor->notifier);
> +
> return rval;
> }
>
> @@ -3124,6 +3144,8 @@ static int smiapp_remove(struct i2c_client *client)
> unsigned int i;
>
> v4l2_async_unregister_subdev(subdev);
> + v4l2_async_notifier_unregister(&sensor->notifier);
> + v4l2_async_notifier_release(&sensor->notifier);
>
> pm_runtime_disable(&client->dev);
> if (!pm_runtime_status_suspended(&client->dev))
> diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
> index f74d695018b9..be92cb5713f4 100644
> --- a/drivers/media/i2c/smiapp/smiapp.h
> +++ b/drivers/media/i2c/smiapp/smiapp.h
> @@ -20,9 +20,10 @@
> #define __SMIAPP_PRIV_H_
>
> #include <linux/mutex.h>
> +#include <media/i2c/smiapp.h>
> +#include <media/v4l2-async.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-subdev.h>
> -#include <media/i2c/smiapp.h>
>
> #include "smiapp-pll.h"
> #include "smiapp-reg.h"
> @@ -172,6 +173,7 @@ struct smiapp_subdev {
> * struct smiapp_sensor - Main device structure
> */
> struct smiapp_sensor {
> + struct v4l2_async_notifier notifier;
> /*
> * "mutex" is used to serialise access to all fields here
> * except v4l2_ctrls at the end of the struct. "mutex" is also
>
^ permalink raw reply [flat|nested] 68+ messages in thread