linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] exynos4-is driver fixes
@ 2013-04-22 14:03 Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources Sylwester Nawrocki
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

This patch series includes fixes for several issues found during
testing all exynos4-is device drivers build as modules. The exynos4-is
build with all sub-drivers as 'M' is hopefully now free of all serious
issues, but one. I.e. the requirement now is to have all sub-device
drivers, including the sensor subdev drivers, built as modules.

The problem when some of the sub-device drivers is statically linked
is that the media links of a media entity just unregistered from
the media device are not fully cleaned up in the media controller
API. This means other entities can have dangling pointers to the links
array owned by en entity just removed and freed. The problem is not
existent when all media entites are registered/unregistred together.
In such a case it doesn't hurt that media_entity_cleanup() function
just frees the links array.

I will post a separate RFC patch to address this issue, since it is
not trivial where the link references should be removed from all
involved media entities.

I verified that adding a call to media_entity_remove_links() as in
patch [1] to the v4l2_sdubdev_unregister_function() eliminates all
weird crashes present before, when inserting/removing all the host
driver modules while the sensor driver stays loaded.

[1] http://git.linuxtv.org/snawrocki/samsung.git/commitdiff/f7007880a37c28beef845aa0787696aa8cead1cd

Sylwester Nawrocki (12):
  s5c73m3: Fix remove() callback to free requested resources
  s5c73m3: Add missing subdev .unregistered callback
  exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries
  exynos4-is: Fix initialization of subdev 'flags' field
  exynos4-is: Fix regulator/gpio resource releasing on the driver
    removal
  exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver
  exynos4-is: Unregister fimc-is subdevs from the media device properly
  exynos4-is: Set fimc-lite subdev subdev owner module
  exynos4-is: Remove redundant module_put() for MIPI-CSIS module
  exynos4-is: Remove debugfs entries properly
  exynos4-is: Change function call order in fimc_is_module_exit()
  exynos4-is: Fix runtime PM handling on fimc-is probe error path

 drivers/media/i2c/s5c73m3/s5c73m3-core.c           |   21 +++++++++---
 drivers/media/platform/exynos4-is/fimc-capture.c   |    2 +-
 drivers/media/platform/exynos4-is/fimc-is-i2c.c    |    3 --
 drivers/media/platform/exynos4-is/fimc-is-sensor.c |   35 +++++---------------
 drivers/media/platform/exynos4-is/fimc-is-sensor.h |    6 ++++
 drivers/media/platform/exynos4-is/fimc-is.c        |   15 ++++-----
 drivers/media/platform/exynos4-is/fimc-isp.c       |    2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c      |    3 +-
 drivers/media/platform/exynos4-is/media-dev.c      |    5 ++-
 9 files changed, 46 insertions(+), 46 deletions(-)

--
1.7.9.5


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

* [PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 02/12] s5c73m3: Add missing subdev .unregistered callback Sylwester Nawrocki
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Make sure v4l2_device_unregister_subdev() is called for both:
oif and sensor subdev and both media entities are freed on
driver removal.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index b353c50..ce8fcf2 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1668,13 +1668,17 @@ out_err1:
 
 static int s5c73m3_remove(struct i2c_client *client)
 {
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct s5c73m3 *state = sensor_sd_to_s5c73m3(sd);
+	struct v4l2_subdev *oif_sd = i2c_get_clientdata(client);
+	struct s5c73m3 *state = oif_sd_to_s5c73m3(oif_sd);
+	struct v4l2_subdev *sensor_sd = &state->sensor_sd;
 
-	v4l2_device_unregister_subdev(sd);
+	v4l2_device_unregister_subdev(oif_sd);
 
-	v4l2_ctrl_handler_free(sd->ctrl_handler);
-	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(oif_sd->ctrl_handler);
+	media_entity_cleanup(&oif_sd->entity);
+
+	v4l2_device_unregister_subdev(sensor_sd);
+	media_entity_cleanup(&sensor_sd->entity);
 
 	s5c73m3_unregister_spi_driver(state);
 	s5c73m3_free_gpios(state);
-- 
1.7.9.5


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

* [PATCH 02/12] s5c73m3: Add missing subdev .unregistered callback
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 03/12] exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries Sylwester Nawrocki
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

This is needed to free any resources requested in
the .registered subdev op.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index ce8fcf2..cb52438 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1457,6 +1457,12 @@ static int s5c73m3_oif_registered(struct v4l2_subdev *sd)
 	return ret;
 }
 
+static void s5c73m3_oif_unregistered(struct v4l2_subdev *sd)
+{
+	struct s5c73m3 *state = oif_sd_to_s5c73m3(sd);
+	v4l2_device_unregister_subdev(&state->sensor_sd);
+}
+
 static const struct v4l2_subdev_internal_ops s5c73m3_internal_ops = {
 	.open		= s5c73m3_open,
 };
@@ -1474,6 +1480,7 @@ static const struct v4l2_subdev_ops s5c73m3_subdev_ops = {
 
 static const struct v4l2_subdev_internal_ops oif_internal_ops = {
 	.registered	= s5c73m3_oif_registered,
+	.unregistered	= s5c73m3_oif_unregistered,
 	.open		= s5c73m3_oif_open,
 };
 
-- 
1.7.9.5


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

* [PATCH 03/12] exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 02/12] s5c73m3: Add missing subdev .unregistered callback Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 04/12] exynos4-is: Fix initialization of subdev 'flags' field Sylwester Nawrocki
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Remove unneeded MODULE_DEVICE_TABLE(of,...) instances from files that
are linked into same module. This fixes following error when building
as a module:

LD [M]  drivers/media/platform/exynos4-is/s5p-fimc.o
drivers/media/platform/exynos4-is/fimc-is-sensor.o: In function `.LANCHOR1':
fimc-is-sensor.c:(.rodata+0x48): multiple definition of `__mod_of_device_table'
drivers/media/platform/exynos4-is/fimc-is.o:fimc-is.c:(.rodata+0x174): first defined here
drivers/media/platform/exynos4-is/fimc-is-i2c.o:(.rodata+0x5c): multiple definition of `__mod_of_device_table'
drivers/media/platform/exynos4-is/fimc-is.o:fimc-is.c:(.rodata+0x174): first defined here
make[4]: *** [drivers/media/platform/exynos4-is/exynos-fimc-is.o] Error 1

Also remove exporting fimc_is_(un)register_i2c_driver functions, it
is not needed since these functions should be called only from our
module.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is-i2c.c    |    3 ---
 drivers/media/platform/exynos4-is/fimc-is-sensor.c |    1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is-i2c.c b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
index 1ec6b3c..c397777 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-i2c.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
@@ -103,7 +103,6 @@ static const struct of_device_id fimc_is_i2c_of_match[] = {
 	{ .compatible = FIMC_IS_I2C_COMPATIBLE },
 	{ },
 };
-MODULE_DEVICE_TABLE(of, fimc_is_i2c_of_match);
 
 static struct platform_driver fimc_is_i2c_driver = {
 	.probe		= fimc_is_i2c_probe,
@@ -120,10 +119,8 @@ int fimc_is_register_i2c_driver(void)
 {
 	return platform_driver_register(&fimc_is_i2c_driver);
 }
-EXPORT_SYMBOL(fimc_is_register_i2c_driver);
 
 void fimc_is_unregister_i2c_driver(void)
 {
 	platform_driver_unregister(&fimc_is_i2c_driver);
 }
-EXPORT_SYMBOL(fimc_is_unregister_i2c_driver);
diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
index 02b2719..6b3ea54 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
@@ -294,7 +294,6 @@ static const struct of_device_id fimc_is_sensor_of_match[] = {
 	},
 	{  }
 };
-MODULE_DEVICE_TABLE(of, fimc_is_sensor_of_match);
 
 static struct i2c_driver fimc_is_sensor_driver = {
 	.driver = {
-- 
1.7.9.5


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

* [PATCH 04/12] exynos4-is: Fix initialization of subdev 'flags' field
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (2 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 03/12] exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 05/12] exynos4-is: Fix regulator/gpio resource releasing on the driver removal Sylwester Nawrocki
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Ensure the value of struct v4l2_subdev::flags field as set
in v4l2_subdev_init() is preserved when initializing it in
the subdev drivers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |    2 +-
 drivers/media/platform/exynos4-is/fimc-isp.c     |    2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c    |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 72c516a..558c528 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -1869,7 +1869,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc)
 	int ret;
 
 	v4l2_subdev_init(sd, &fimc_subdev_ops);
-	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	snprintf(sd->name, sizeof(sd->name), "FIMC.%d", fimc->id);
 
 	fimc->vid_cap.sd_pads[FIMC_SD_PAD_SINK_CAM].flags = MEDIA_PAD_FL_SINK;
diff --git a/drivers/media/platform/exynos4-is/fimc-isp.c b/drivers/media/platform/exynos4-is/fimc-isp.c
index 3b9a664..d63947f 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp.c
@@ -621,7 +621,7 @@ int fimc_isp_subdev_create(struct fimc_isp *isp)
 
 	v4l2_subdev_init(sd, &fimc_is_subdev_ops);
 	sd->grp_id = GRP_ID_FIMC_IS;
-	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	snprintf(sd->name, sizeof(sd->name), "FIMC-IS-ISP");
 
 	isp->subdev_pads[FIMC_ISP_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 661d0d1..7ecf4e7 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1377,7 +1377,7 @@ static int fimc_lite_create_capture_subdev(struct fimc_lite *fimc)
 	int ret;
 
 	v4l2_subdev_init(sd, &fimc_lite_subdev_ops);
-	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	snprintf(sd->name, sizeof(sd->name), "FIMC-LITE.%d", fimc->index);
 
 	fimc->subdev_pads[FLITE_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
-- 
1.7.9.5


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

* [PATCH 05/12] exynos4-is: Fix regulator/gpio resource releasing on the driver removal
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (3 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 04/12] exynos4-is: Fix initialization of subdev 'flags' field Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 06/12] exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver Sylwester Nawrocki
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Remove regulator_bulk_free() calls as devm_regulator_bulk_get() function
is used to get the regulators so those will be freed automatically while
the driver is removed.
Missing gpio free is fixed by requesting a gpio with the devm_* API.
All that is done now in the I2C client driver remove() callback is the
media entity cleanup call.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is-sensor.c |   26 ++++++--------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
index 6b3ea54..035fa14 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
@@ -216,7 +216,8 @@ static int fimc_is_sensor_probe(struct i2c_client *client,
 
 	gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
 	if (gpio_is_valid(gpio)) {
-		ret = gpio_request_one(gpio, GPIOF_OUT_INIT_LOW, DRIVER_NAME);
+		ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW,
+							DRIVER_NAME);
 		if (ret < 0)
 			return ret;
 	}
@@ -228,13 +229,11 @@ static int fimc_is_sensor_probe(struct i2c_client *client,
 	ret = devm_regulator_bulk_get(&client->dev, SENSOR_NUM_SUPPLIES,
 				      sensor->supplies);
 	if (ret < 0)
-		goto err_gpio;
+		return ret;
 
 	of_id = of_match_node(fimc_is_sensor_of_match, dev->of_node);
-	if (!of_id) {
-		ret = -ENODEV;
-		goto err_reg;
-	}
+	if (!of_id)
+		return -ENODEV;
 
 	sensor->drvdata = of_id->data;
 	sensor->dev = dev;
@@ -251,28 +250,19 @@ static int fimc_is_sensor_probe(struct i2c_client *client,
 	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_init(&sd->entity, 1, &sensor->pad, 0);
 	if (ret < 0)
-		goto err_reg;
+		return ret;
 
 	v4l2_set_subdevdata(sd, sensor);
 	pm_runtime_no_callbacks(dev);
 	pm_runtime_enable(dev);
 
-	return 0;
-err_reg:
-	regulator_bulk_free(SENSOR_NUM_SUPPLIES, sensor->supplies);
-err_gpio:
-	if (gpio_is_valid(sensor->gpio_reset))
-		gpio_free(sensor->gpio_reset);
 	return ret;
 }
 
 static int fimc_is_sensor_remove(struct i2c_client *client)
 {
-	struct fimc_is_sensor *sensor;
-
-	regulator_bulk_free(SENSOR_NUM_SUPPLIES, sensor->supplies);
-	media_entity_cleanup(&sensor->subdev.entity);
-
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	media_entity_cleanup(&sd->entity);
 	return 0;
 }
 
-- 
1.7.9.5


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

* [PATCH 06/12] exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (4 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 05/12] exynos4-is: Fix regulator/gpio resource releasing on the driver removal Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 07/12] exynos4-is: Unregister fimc-is subdevs from the media device properly Sylwester Nawrocki
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

It's an I2C client driver and it must not overwrite the struct v4l2_subdev
dev_priv field, which is used by the v4l2 core to store a pointer to
struct i2c_client.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is-sensor.c |    8 +-------
 drivers/media/platform/exynos4-is/fimc-is-sensor.h |    6 ++++++
 drivers/media/platform/exynos4-is/fimc-is.c        |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
index 035fa14..6647421 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
@@ -40,11 +40,6 @@ static const struct v4l2_mbus_framefmt fimc_is_sensor_formats[] = {
 	}
 };
 
-static struct fimc_is_sensor *sd_to_fimc_is_sensor(struct v4l2_subdev *sd)
-{
-	return container_of(sd, struct fimc_is_sensor, subdev);
-}
-
 static const struct v4l2_mbus_framefmt *find_sensor_format(
 	struct v4l2_mbus_framefmt *mf)
 {
@@ -147,7 +142,7 @@ static const struct v4l2_subdev_internal_ops fimc_is_sensor_sd_internal_ops = {
 
 static int fimc_is_sensor_s_power(struct v4l2_subdev *sd, int on)
 {
-	struct fimc_is_sensor *sensor = v4l2_get_subdevdata(sd);
+	struct fimc_is_sensor *sensor = sd_to_fimc_is_sensor(sd);
 	int gpio = sensor->gpio_reset;
 	int ret;
 
@@ -252,7 +247,6 @@ static int fimc_is_sensor_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	v4l2_set_subdevdata(sd, sensor);
 	pm_runtime_no_callbacks(dev);
 	pm_runtime_enable(dev);
 
diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.h b/drivers/media/platform/exynos4-is/fimc-is-sensor.h
index 50b8e4d..6036d49 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-sensor.h
+++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.h
@@ -77,6 +77,12 @@ struct fimc_is_sensor {
 	struct v4l2_mbus_framefmt format;
 };
 
+static inline
+struct fimc_is_sensor *sd_to_fimc_is_sensor(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct fimc_is_sensor, subdev);
+}
+
 int fimc_is_register_sensor_driver(void);
 void fimc_is_unregister_sensor_driver(void);
 
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 3c81c88..c4049d4 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -220,7 +220,7 @@ static int fimc_is_register_subdevs(struct fimc_is *is)
 			if (WARN_ON(is->sensor))
 				continue;
 
-			is->sensor = v4l2_get_subdevdata(sd);
+			is->sensor = sd_to_fimc_is_sensor(sd);
 
 			if (fimc_is_parse_sensor_config(is->sensor, child)) {
 				dev_warn(&is->pdev->dev, "DT parse error: %s\n",
-- 
1.7.9.5


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

* [PATCH 07/12] exynos4-is: Unregister fimc-is subdevs from the media device properly
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (5 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 06/12] exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 08/12] exynos4-is: Set fimc-lite subdev subdev owner module Sylwester Nawrocki
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Add missing v4l2_device_unregister_subdev() call for the FIMC-IS subdevs
(currently there is only the FIMC-IS-ISP subdev) so corresponding resources
are properly freed upon the media device driver module removal.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 1dbd554..a371ee5 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -823,6 +823,10 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd)
 		fimc_md_unregister_sensor(fmd->sensor[i].subdev);
 		fmd->sensor[i].subdev = NULL;
 	}
+
+	if (fmd->fimc_is)
+		v4l2_device_unregister_subdev(&fmd->fimc_is->isp.subdev);
+
 	v4l2_info(&fmd->v4l2_dev, "Unregistered all entities\n");
 }
 
-- 
1.7.9.5


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

* [PATCH 08/12] exynos4-is: Set fimc-lite subdev subdev owner module
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (6 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 07/12] exynos4-is: Unregister fimc-is subdevs from the media device properly Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 09/12] exynos4-is: Remove redundant module_put() for MIPI-CSIS module Sylwester Nawrocki
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media
  Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki, stable

The FIMC-LITE.n subdevs have currently sd->owner field not set,
the exynos-fimc-lite module can be removed at any time, regardless
it is in use by other modules. When this module is unloaded the
kernel can crash easily by accessing video or media device nodes.

Cc: stable@vger.kernel.org
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-lite.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 7ecf4e7..14bb7bc 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1399,6 +1399,7 @@ static int fimc_lite_create_capture_subdev(struct fimc_lite *fimc)
 	sd->ctrl_handler = handler;
 	sd->internal_ops = &fimc_lite_subdev_internal_ops;
 	sd->entity.ops = &fimc_lite_subdev_media_ops;
+	sd->owner = THIS_MODULE;
 	v4l2_set_subdevdata(sd, fimc);
 
 	return 0;
-- 
1.7.9.5


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

* [PATCH 09/12] exynos4-is: Remove redundant module_put() for MIPI-CSIS module
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (7 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 08/12] exynos4-is: Set fimc-lite subdev subdev owner module Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 10/12] exynos4-is: Remove debugfs entries properly Sylwester Nawrocki
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Currently there is unbalanced module_put() on the s5p-csis module
which prevents it from being unloaded. The subdev's owner module
has reference count decremented in v4l2_device_unregister_subdev()
so just remove this erroneous call.

Cc: stable@vger.kernel.org # 3.8
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index a371ee5..15ef8f2 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -814,7 +814,6 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd)
 		if (fmd->csis[i].sd == NULL)
 			continue;
 		v4l2_device_unregister_subdev(fmd->csis[i].sd);
-		module_put(fmd->csis[i].sd->owner);
 		fmd->csis[i].sd = NULL;
 	}
 	for (i = 0; i < fmd->num_sensors; i++) {
-- 
1.7.9.5


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

* [PATCH 10/12] exynos4-is: Remove debugfs entries properly
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (8 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 09/12] exynos4-is: Remove redundant module_put() for MIPI-CSIS module Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 11/12] exynos4-is: Change function call order in fimc_is_module_exit() Sylwester Nawrocki
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Ensure both debugfs: fimc_is directory and the fw_log file
are properly removed in the driver cleanup sequence.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index c4049d4..ca72b02 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -766,7 +766,7 @@ static const struct file_operations fimc_is_debugfs_fops = {
 
 static void fimc_is_debugfs_remove(struct fimc_is *is)
 {
-	debugfs_remove(is->debugfs_entry);
+	debugfs_remove_recursive(is->debugfs_entry);
 	is->debugfs_entry = NULL;
 }
 
-- 
1.7.9.5


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

* [PATCH 11/12] exynos4-is: Change function call order in fimc_is_module_exit()
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (9 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 10/12] exynos4-is: Remove debugfs entries properly Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:03 ` [PATCH 12/12] exynos4-is: Fix runtime PM handling on fimc-is probe error path Sylwester Nawrocki
  2013-04-22 14:13 ` [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Due to hardware dependencies (clocks/power domain) the I2C bus
controller needs to be unregistered before fimc-is.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index ca72b02..5e89077 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -995,9 +995,9 @@ err_sens:
 
 static void fimc_is_module_exit(void)
 {
-	platform_driver_unregister(&fimc_is_driver);
-	fimc_is_unregister_i2c_driver();
 	fimc_is_unregister_sensor_driver();
+	fimc_is_unregister_i2c_driver();
+	platform_driver_unregister(&fimc_is_driver);
 }
 
 module_init(fimc_is_module_init);
-- 
1.7.9.5


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

* [PATCH 12/12] exynos4-is: Fix runtime PM handling on fimc-is probe error path
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (10 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 11/12] exynos4-is: Change function call order in fimc_is_module_exit() Sylwester Nawrocki
@ 2013-04-22 14:03 ` Sylwester Nawrocki
  2013-04-22 14:13 ` [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:03 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda, Sylwester Nawrocki

Ensure there is no unbalanced pm_runtime_put().

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-is.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 5e89077..47c6363 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -847,16 +847,17 @@ static int fimc_is_probe(struct platform_device *pdev)
 		goto err_irq;
 
 	ret = fimc_is_setup_clocks(is);
+	pm_runtime_put_sync(dev);
+
 	if (ret < 0)
 		goto err_irq;
 
-	pm_runtime_put_sync(dev);
 	is->clk_init = true;
 
 	is->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(is->alloc_ctx)) {
 		ret = PTR_ERR(is->alloc_ctx);
-		goto err_pm;
+		goto err_irq;
 	}
 	/*
 	 * Register FIMC-IS V4L2 subdevs to this driver. The video nodes
@@ -885,8 +886,6 @@ err_sd:
 	fimc_is_unregister_subdevs(is);
 err_irq:
 	free_irq(is->irq, is);
-err_pm:
-	pm_runtime_put(dev);
 err_clk:
 	fimc_is_put_clocks(is);
 	return ret;
-- 
1.7.9.5


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

* Re: [PATCH 00/12] exynos4-is driver fixes
  2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
                   ` (11 preceding siblings ...)
  2013-04-22 14:03 ` [PATCH 12/12] exynos4-is: Fix runtime PM handling on fimc-is probe error path Sylwester Nawrocki
@ 2013-04-22 14:13 ` Sylwester Nawrocki
  12 siblings, 0 replies; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-22 14:13 UTC (permalink / raw)
  To: linux-media; +Cc: kyungmin.park, sw0312.kim, a.hajda

On 04/22/2013 04:03 PM, Sylwester Nawrocki wrote:
> This patch series includes fixes for several issues found during
> testing all exynos4-is device drivers build as modules. The exynos4-is
> build with all sub-drivers as 'M' is hopefully now free of all serious
> issues, but one. I.e. the requirement now is to have all sub-device
> drivers, including the sensor subdev drivers, built as modules.

Hmm, to avoid issues all drivers must now be either statically linked or
build as modules and all need to be inserted, the all removed. Leaving
any one loaded all time may lead to a disaster... This is not a new
issue and and is related to all drivers using MC framework, thus I plan
to address it for 3.11.

> The problem when some of the sub-device drivers is statically linked
> is that the media links of a media entity just unregistered from
> the media device are not fully cleaned up in the media controller
> API. This means other entities can have dangling pointers to the links
> array owned by en entity just removed and freed. The problem is not
> existent when all media entites are registered/unregistred together.
> In such a case it doesn't hurt that media_entity_cleanup() function
> just frees the links array.
> 
> I will post a separate RFC patch to address this issue, since it is
> not trivial where the link references should be removed from all
> involved media entities.
> 
> I verified that adding a call to media_entity_remove_links() as in
> patch [1] to the v4l2_sdubdev_unregister_function() eliminates all
> weird crashes present before, when inserting/removing all the host
> driver modules while the sensor driver stays loaded.
> 
> [1] http://git.linuxtv.org/snawrocki/samsung.git/commitdiff/f7007880a37c28beef845aa0787696aa8cead1cd

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

end of thread, other threads:[~2013-04-22 14:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-22 14:03 [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 02/12] s5c73m3: Add missing subdev .unregistered callback Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 03/12] exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 04/12] exynos4-is: Fix initialization of subdev 'flags' field Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 05/12] exynos4-is: Fix regulator/gpio resource releasing on the driver removal Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 06/12] exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 07/12] exynos4-is: Unregister fimc-is subdevs from the media device properly Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 08/12] exynos4-is: Set fimc-lite subdev subdev owner module Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 09/12] exynos4-is: Remove redundant module_put() for MIPI-CSIS module Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 10/12] exynos4-is: Remove debugfs entries properly Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 11/12] exynos4-is: Change function call order in fimc_is_module_exit() Sylwester Nawrocki
2013-04-22 14:03 ` [PATCH 12/12] exynos4-is: Fix runtime PM handling on fimc-is probe error path Sylwester Nawrocki
2013-04-22 14:13 ` [PATCH 00/12] exynos4-is driver fixes Sylwester Nawrocki

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