Re: [PATCH v2 4/7] V4L: s5k6a3: Add support for asynchronous subdev registration
On 08/28/2013 06:21 PM, Guennadi Liakhovetski wrote: > Hi Sylwester, > > Just one doubt below > > On Wed, 28 Aug 2013, Sylwester Nawrocki wrote: > >> > This patch converts the driver to use v4l2 asynchronous subdev >> > registration API an the clock API to control the external master >> > clock directly. >> > >> > Signed-off-by: Sylwester Nawrocki >> > Signed-off-by: Kyungmin Park >> > --- >> > drivers/media/i2c/s5k6a3.c | 36 ++-- >> > 1 file changed, 26 insertions(+), 10 deletions(-) >> > >> > diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c >> > index ba86e24..f65a4f8 100644 >> > --- a/drivers/media/i2c/s5k6a3.c >> > +++ b/drivers/media/i2c/s5k6a3.c > [snip] > >> > @@ -282,7 +297,7 @@ static int s5k6a3_probe(struct i2c_client *client, >> >pm_runtime_no_callbacks(dev); >> >pm_runtime_enable(dev); >> > >> > - return 0; >> > + return v4l2_async_register_subdev(sd); > > If the above fails - don't you have to do any clean up? E.g. below you do > disable runtime PM and clean up the media entity. You're right, the clean up steps are missing. Thanks for pointing out, I've corrected this for the next iteration. Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH v2 4/7] V4L: s5k6a3: Add support for asynchronous subdev registration
This patch converts the driver to use v4l2 asynchronous subdev
registration API an the clock API to control the external master
clock directly.
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/i2c/s5k6a3.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index ba86e24..f65a4f8 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -34,6 +34,7 @@
#define S5K6A3_DEFAULT_HEIGHT 732
#define S5K6A3_DRV_NAME"S5K6A3"
+#define S5K6A3_CLK_NAME"extclk"
#define S5K6A3_DEFAULT_CLK_FREQ2400U
#define S5K6A3_NUM_SUPPLIES2
@@ -56,6 +57,7 @@ struct s5k6a3 {
int gpio_reset;
struct mutex lock;
struct v4l2_mbus_framefmt format;
+ struct clk *clock;
u32 clock_frequency;
};
@@ -181,19 +183,25 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
{
struct s5k6a3 *sensor = sd_to_s5k6a3(sd);
int gpio = sensor->gpio_reset;
- int ret;
+ int ret = 0;
if (on) {
+ ret = clk_set_rate(sensor->clock, sensor->clock_frequency);
+ if (ret < 0)
+ return ret;
+
ret = pm_runtime_get(sensor->dev);
if (ret < 0)
return ret;
ret = regulator_bulk_enable(S5K6A3_NUM_SUPPLIES,
sensor->supplies);
- if (ret < 0) {
- pm_runtime_put(sensor->dev);
- return ret;
- }
+ if (ret < 0)
+ goto rpm_put;
+
+ ret = clk_prepare_enable(sensor->clock);
+ if (ret < 0)
+ goto reg_dis;
if (gpio_is_valid(gpio)) {
gpio_set_value(gpio, 1);
@@ -209,10 +217,12 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
if (gpio_is_valid(gpio))
gpio_set_value(gpio, 0);
- ret = regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
-sensor->supplies);
- if (!ret)
- pm_runtime_put(sensor->dev);
+ clk_disable_unprepare(sensor->clock);
+reg_dis:
+ regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
+ sensor->supplies);
+rpm_put:
+ pm_runtime_put(sensor->dev);
}
return ret;
}
@@ -240,6 +250,7 @@ static int s5k6a3_probe(struct i2c_client *client,
mutex_init(&sensor->lock);
sensor->gpio_reset = -EINVAL;
+ sensor->clock = ERR_PTR(-EINVAL);
sensor->dev = dev;
gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
@@ -266,6 +277,10 @@ static int s5k6a3_probe(struct i2c_client *client,
if (ret < 0)
return ret;
+ sensor->clock = devm_clk_get(dev, S5K6A3_CLK_NAME);
+ if (IS_ERR(sensor->clock))
+ return -EPROBE_DEFER;
+
sd = &sensor->subdev;
v4l2_i2c_subdev_init(sd, client, &s5k6a3_subdev_ops);
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -282,7 +297,7 @@ static int s5k6a3_probe(struct i2c_client *client,
pm_runtime_no_callbacks(dev);
pm_runtime_enable(dev);
- return 0;
+ return v4l2_async_register_subdev(sd);
}
static int s5k6a3_remove(struct i2c_client *client)
@@ -290,6 +305,7 @@ static int s5k6a3_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
pm_runtime_disable(&client->dev);
+ v4l2_async_unregister_subdev(sd);
media_entity_cleanup(&sd->entity);
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/7] V4L: s5k6a3: Add support for asynchronous subdev registration
Hi Sylwester, Just one doubt below On Wed, 28 Aug 2013, Sylwester Nawrocki wrote: > This patch converts the driver to use v4l2 asynchronous subdev > registration API an the clock API to control the external master > clock directly. > > Signed-off-by: Sylwester Nawrocki > Signed-off-by: Kyungmin Park > --- > drivers/media/i2c/s5k6a3.c | 36 ++-- > 1 file changed, 26 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c > index ba86e24..f65a4f8 100644 > --- a/drivers/media/i2c/s5k6a3.c > +++ b/drivers/media/i2c/s5k6a3.c [snip] > @@ -282,7 +297,7 @@ static int s5k6a3_probe(struct i2c_client *client, > pm_runtime_no_callbacks(dev); > pm_runtime_enable(dev); > > - return 0; > + return v4l2_async_register_subdev(sd); If the above fails - don't you have to do any clean up? E.g. below you do disable runtime PM and clean up the media entity. Thanks Guennadi > } > > static int s5k6a3_remove(struct i2c_client *client) > @@ -290,6 +305,7 @@ static int s5k6a3_remove(struct i2c_client *client) > struct v4l2_subdev *sd = i2c_get_clientdata(client); > > pm_runtime_disable(&client->dev); > + v4l2_async_unregister_subdev(sd); > media_entity_cleanup(&sd->entity); > return 0; > } > -- > 1.7.9.5 > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/7] V4L: s5k6a3: Add support for asynchronous subdev registration
This patch converts the driver to use v4l2 asynchronous subdev
registration API an the clock API to control the external master
clock directly.
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/i2c/s5k6a3.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index ba86e24..f65a4f8 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -34,6 +34,7 @@
#define S5K6A3_DEFAULT_HEIGHT 732
#define S5K6A3_DRV_NAME"S5K6A3"
+#define S5K6A3_CLK_NAME"extclk"
#define S5K6A3_DEFAULT_CLK_FREQ2400U
#define S5K6A3_NUM_SUPPLIES2
@@ -56,6 +57,7 @@ struct s5k6a3 {
int gpio_reset;
struct mutex lock;
struct v4l2_mbus_framefmt format;
+ struct clk *clock;
u32 clock_frequency;
};
@@ -181,19 +183,25 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
{
struct s5k6a3 *sensor = sd_to_s5k6a3(sd);
int gpio = sensor->gpio_reset;
- int ret;
+ int ret = 0;
if (on) {
+ ret = clk_set_rate(sensor->clock, sensor->clock_frequency);
+ if (ret < 0)
+ return ret;
+
ret = pm_runtime_get(sensor->dev);
if (ret < 0)
return ret;
ret = regulator_bulk_enable(S5K6A3_NUM_SUPPLIES,
sensor->supplies);
- if (ret < 0) {
- pm_runtime_put(sensor->dev);
- return ret;
- }
+ if (ret < 0)
+ goto rpm_put;
+
+ ret = clk_prepare_enable(sensor->clock);
+ if (ret < 0)
+ goto reg_dis;
if (gpio_is_valid(gpio)) {
gpio_set_value(gpio, 1);
@@ -209,10 +217,12 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
if (gpio_is_valid(gpio))
gpio_set_value(gpio, 0);
- ret = regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
-sensor->supplies);
- if (!ret)
- pm_runtime_put(sensor->dev);
+ clk_disable_unprepare(sensor->clock);
+reg_dis:
+ regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
+ sensor->supplies);
+rpm_put:
+ pm_runtime_put(sensor->dev);
}
return ret;
}
@@ -240,6 +250,7 @@ static int s5k6a3_probe(struct i2c_client *client,
mutex_init(&sensor->lock);
sensor->gpio_reset = -EINVAL;
+ sensor->clock = ERR_PTR(-EINVAL);
sensor->dev = dev;
gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
@@ -266,6 +277,10 @@ static int s5k6a3_probe(struct i2c_client *client,
if (ret < 0)
return ret;
+ sensor->clock = devm_clk_get(dev, S5K6A3_CLK_NAME);
+ if (IS_ERR(sensor->clock))
+ return -EPROBE_DEFER;
+
sd = &sensor->subdev;
v4l2_i2c_subdev_init(sd, client, &s5k6a3_subdev_ops);
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -282,7 +297,7 @@ static int s5k6a3_probe(struct i2c_client *client,
pm_runtime_no_callbacks(dev);
pm_runtime_enable(dev);
- return 0;
+ return v4l2_async_register_subdev(sd);
}
static int s5k6a3_remove(struct i2c_client *client)
@@ -290,6 +305,7 @@ static int s5k6a3_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
pm_runtime_disable(&client->dev);
+ v4l2_async_unregister_subdev(sd);
media_entity_cleanup(&sd->entity);
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
