Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
On 1/12/2015 6:38 PM, Laurent Pinchart wrote: Hi Josh, On Monday 12 January 2015 17:14:33 Josh Wu wrote: On 1/9/2015 6:47 AM, Laurent Pinchart wrote: On Thursday 08 January 2015 23:37:58 Guennadi Liakhovetski wrote: On Wed, 7 Jan 2015, Josh Wu wrote: On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote: On Tue, 6 Jan 2015, Josh Wu wrote: Hi, Guennadi After look deep into this patch, I found you miss one line that should be changed as well. It's In function v4l2_clk_get(), there still has one line code called v4l2_clk_find(dev_id, id). You need to change it to v4l2_clk_find(dev_id, NULL) as well. Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk) cannot acquired the mclk clock. After above changes, this patch works for me. I think you're right, in fact, since we now don't store CCF-based v4l2_clk wrappers on the list, this can be simplified even further, I'll update the patch. Did you only test this patch or both? I tested both patches with Atmel-isi driver. For the 2/2 patch I applied the modification Laurent suggested. Those patches works for me. The only concern is in ov2640 I still need to acquired two v4l2 clocks: xvclk that will get the xvclk CCF clock directly. mclk that make ISI driver call his clock_start()/stop() to enable/disable ISI's peripheral clock. If I only get xvclk clock, then the camera capture will be failed with a ISI timeout error. No, this doesn't look right to me. The camera sensor has only one clock input, so, it should only request one clock. Where does the clock signal to the camera come from on your system? That's correct, the sensor driver only has one clock input, so it should just request the xvclk clock. If it comes from the ISI itself, you don't need to specify the clock in the DT, since the ISI doesn't produce a clock from DT. If you do want to have your clock consumer (ov2640) and the supplier (ISI) properly described in DT, you'll have to teach the ISI to register a CCF clock source, which then will be connected to from the ov2640. If you choose not to show your clock in the DT, you can just use v4l2_clk_get(dev, xvclk) and it will be handled by v4l2_clk / soc-camera / isi-atmel. If the closk to ov2640 is supplied by a separate clock source, then you v4l2_clk_get() will connect ov2640 to it directly and soc-camera will enable and disable it on power-on / -off as required. The ISI has no way to supply a sensor clock, the clock is supplied by a separate clock source. From your above description it looks like the clock to ov2640 is supplied by a separate source, but atmel-isi's .clock_start() / .clock_stop() functions still need to be called? By looking at those functions it looks like they turn on and off clocks, supplying the ISI itself... Instead of only turning on and off clocks, provided by the ISI to a camera sensor. If my understanding is right, then this is a bug in atmel-isi and it has to be fixed. That's correct as well, the ISI driver needs to be fixed. Thanks both of you for the details. Now I got it. Indeed, I need fix this in atmel-isi driver not in ov2640 driver. So I will send a new patch for this, which should move the ISI peripheral clock enable/disable() from clock_start/stop() to isi_camera_add_device/remove_device(). Shouldn't you move it to the start_streaming() and stop_streaming() functions instead ? An even better solution would be to use runtime PM to enable/disable the ISI clock in the runtime PM resume and suspend handlers, and call pm_runtime_get_sync() and pm_runtime_put() when you need the ISI to be operational. Okay, I'll try to add the PM functions for atmel-isi in the meantime. Best Regards, Josh Wu -- 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 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi and Laurent On 1/9/2015 6:47 AM, Laurent Pinchart wrote: Hi Guennadi and Josh, On Thursday 08 January 2015 23:37:58 Guennadi Liakhovetski wrote: On Wed, 7 Jan 2015, Josh Wu wrote: On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote: On Tue, 6 Jan 2015, Josh Wu wrote: Hi, Guennadi After look deep into this patch, I found you miss one line that should be changed as well. It's In function v4l2_clk_get(), there still has one line code called v4l2_clk_find(dev_id, id). You need to change it to v4l2_clk_find(dev_id, NULL) as well. Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk) cannot acquired the mclk clock. After above changes, this patch works for me. I think you're right, in fact, since we now don't store CCF-based v4l2_clk wrappers on the list, this can be simplified even further, I'll update the patch. Did you only test this patch or both? I tested both patches with Atmel-isi driver. For the 2/2 patch I applied the modification Laurent suggested. Those patches works for me. The only concern is in ov2640 I still need to acquired two v4l2 clocks: xvclk that will get the xvclk CCF clock directly. mclk that make ISI driver call his clock_start()/stop() to enable/disable ISI's peripheral clock. If I only get xvclk clock, then the camera capture will be failed with a ISI timeout error. No, this doesn't look right to me. The camera sensor has only one clock input, so, it should only request one clock. Where does the clock signal to the camera come from on your system? That's correct, the sensor driver only has one clock input, so it should just request the xvclk clock. If it comes from the ISI itself, you don't need to specify the clock in the DT, since the ISI doesn't produce a clock from DT. If you do want to have your clock consumer (ov2640) and the supplier (ISI) properly described in DT, you'll have to teach the ISI to register a CCF clock source, which then will be connected to from the ov2640. If you choose not to show your clock in the DT, you can just use v4l2_clk_get(dev, xvclk) and it will be handled by v4l2_clk / soc-camera / isi-atmel. If the closk to ov2640 is supplied by a separate clock source, then you v4l2_clk_get() will connect ov2640 to it directly and soc-camera will enable and disable it on power-on / -off as required. The ISI has no way to supply a sensor clock, the clock is supplied by a separate clock source. From your above description it looks like the clock to ov2640 is supplied by a separate source, but atmel-isi's .clock_start() / .clock_stop() functions still need to be called? By looking at those functions it looks like they turn on and off clocks, supplying the ISI itself... Instead of only turning on and off clocks, provided by the ISI to a camera sensor. If my understanding is right, then this is a bug in atmel-isi and it has to be fixed. That's correct as well, the ISI driver needs to be fixed. Thanks both of you for the details. Now I got it. Indeed, I need fix this in atmel-isi driver not in ov2640 driver. So I will send a new patch for this, which should move the ISI peripheral clock enable/disable() from clock_start/stop() to isi_camera_add_device/remove_device(). Best Regards, Josh Wu -- 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 1/2] V4L: remove clock name from v4l2_clk API
Hi Josh, On Monday 12 January 2015 17:14:33 Josh Wu wrote: On 1/9/2015 6:47 AM, Laurent Pinchart wrote: On Thursday 08 January 2015 23:37:58 Guennadi Liakhovetski wrote: On Wed, 7 Jan 2015, Josh Wu wrote: On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote: On Tue, 6 Jan 2015, Josh Wu wrote: Hi, Guennadi After look deep into this patch, I found you miss one line that should be changed as well. It's In function v4l2_clk_get(), there still has one line code called v4l2_clk_find(dev_id, id). You need to change it to v4l2_clk_find(dev_id, NULL) as well. Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk) cannot acquired the mclk clock. After above changes, this patch works for me. I think you're right, in fact, since we now don't store CCF-based v4l2_clk wrappers on the list, this can be simplified even further, I'll update the patch. Did you only test this patch or both? I tested both patches with Atmel-isi driver. For the 2/2 patch I applied the modification Laurent suggested. Those patches works for me. The only concern is in ov2640 I still need to acquired two v4l2 clocks: xvclk that will get the xvclk CCF clock directly. mclk that make ISI driver call his clock_start()/stop() to enable/disable ISI's peripheral clock. If I only get xvclk clock, then the camera capture will be failed with a ISI timeout error. No, this doesn't look right to me. The camera sensor has only one clock input, so, it should only request one clock. Where does the clock signal to the camera come from on your system? That's correct, the sensor driver only has one clock input, so it should just request the xvclk clock. If it comes from the ISI itself, you don't need to specify the clock in the DT, since the ISI doesn't produce a clock from DT. If you do want to have your clock consumer (ov2640) and the supplier (ISI) properly described in DT, you'll have to teach the ISI to register a CCF clock source, which then will be connected to from the ov2640. If you choose not to show your clock in the DT, you can just use v4l2_clk_get(dev, xvclk) and it will be handled by v4l2_clk / soc-camera / isi-atmel. If the closk to ov2640 is supplied by a separate clock source, then you v4l2_clk_get() will connect ov2640 to it directly and soc-camera will enable and disable it on power-on / -off as required. The ISI has no way to supply a sensor clock, the clock is supplied by a separate clock source. From your above description it looks like the clock to ov2640 is supplied by a separate source, but atmel-isi's .clock_start() / .clock_stop() functions still need to be called? By looking at those functions it looks like they turn on and off clocks, supplying the ISI itself... Instead of only turning on and off clocks, provided by the ISI to a camera sensor. If my understanding is right, then this is a bug in atmel-isi and it has to be fixed. That's correct as well, the ISI driver needs to be fixed. Thanks both of you for the details. Now I got it. Indeed, I need fix this in atmel-isi driver not in ov2640 driver. So I will send a new patch for this, which should move the ISI peripheral clock enable/disable() from clock_start/stop() to isi_camera_add_device/remove_device(). Shouldn't you move it to the start_streaming() and stop_streaming() functions instead ? An even better solution would be to use runtime PM to enable/disable the ISI clock in the runtime PM resume and suspend handlers, and call pm_runtime_get_sync() and pm_runtime_put() when you need the ISI to be operational. -- Regards, Laurent Pinchart -- 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 1/2] V4L: remove clock name from v4l2_clk API
Hi Josh,
On Wed, 7 Jan 2015, Josh Wu wrote:
Hi, Guennadi
On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote:
Hi Josh,
On Tue, 6 Jan 2015, Josh Wu wrote:
Hi, Guennadi
After look deep into this patch, I found you miss one line that should be
changed as well.
It's In function v4l2_clk_get(), there still has one line code called
v4l2_clk_find(dev_id, id).
You need to change it to v4l2_clk_find(dev_id, NULL) as well.
Otherwise the code that many sensor used: v4l2_clk_get(client-dev,
mclk)
cannot acquired the mclk clock.
After above changes, this patch works for me.
I think you're right, in fact, since we now don't store CCF-based v4l2_clk
wrappers on the list, this can be simplified even further, I'll update the
patch. Did you only test this patch or both?
I tested both patches with Atmel-isi driver. For the 2/2 patch I applied the
modification Laurent suggested.
Those patches works for me.
The only concern is in ov2640 I still need to acquired two v4l2 clocks:
xvclk that will get the xvclk CCF clock directly.
mclk that make ISI driver call his clock_start()/stop() to
enable/disable ISI's peripheral clock.
If I only get xvclk clock, then the camera capture will be failed with a ISI
timeout error.
No, this doesn't look right to me. The camera sensor has only one clock
input, so, it should only request one clock. Where does the clock signal
to the camera come from on your system?
If it comes from the ISI itself, you don't need to specify the clock in
the DT, since the ISI doesn't produce a clock from DT. If you do want to
have your clock consumer (ov2640) and the supplier (ISI) properly
described in DT, you'll have to teach the ISI to register a CCF clock
source, which then will be connected to from the ov2640. If you choose not
to show your clock in the DT, you can just use v4l2_clk_get(dev, xvclk)
and it will be handled by v4l2_clk / soc-camera / isi-atmel.
If the closk to ov2640 is supplied by a separate clock source, then you
v4l2_clk_get() will connect ov2640 to it directly and soc-camera will
enable and disable it on power-on / -off as required.
From your above description it looks like the clock to ov2640 is supplied
by a separate source, but atmel-isi's .clock_start() / .clock_stop()
functions still need to be called? By looking at those functions it looks
like they turn on and off clocks, supplying the ISI itself... Instead of
only turning on and off clocks, provided by the ISI to a camera sensor. If
my understanding is right, then this is a bug in atmel-isi and it has to
be fixed.
Thanks
Guennadi
But I think this is acceptable as we will keep go forward. Finally we'll
switch to CCF and removed the v4l2_clock then we will move the
clock_start()/stop() caller code to soc_camera.c.
Best Regards,
Josh Wu
Thanks
Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name,
mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name,
icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id,
sasd-asd.match.i2c.address);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name,
mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name,
icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi Guennadi and Josh, On Thursday 08 January 2015 23:37:58 Guennadi Liakhovetski wrote: On Wed, 7 Jan 2015, Josh Wu wrote: On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote: On Tue, 6 Jan 2015, Josh Wu wrote: Hi, Guennadi After look deep into this patch, I found you miss one line that should be changed as well. It's In function v4l2_clk_get(), there still has one line code called v4l2_clk_find(dev_id, id). You need to change it to v4l2_clk_find(dev_id, NULL) as well. Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk) cannot acquired the mclk clock. After above changes, this patch works for me. I think you're right, in fact, since we now don't store CCF-based v4l2_clk wrappers on the list, this can be simplified even further, I'll update the patch. Did you only test this patch or both? I tested both patches with Atmel-isi driver. For the 2/2 patch I applied the modification Laurent suggested. Those patches works for me. The only concern is in ov2640 I still need to acquired two v4l2 clocks: xvclk that will get the xvclk CCF clock directly. mclk that make ISI driver call his clock_start()/stop() to enable/disable ISI's peripheral clock. If I only get xvclk clock, then the camera capture will be failed with a ISI timeout error. No, this doesn't look right to me. The camera sensor has only one clock input, so, it should only request one clock. Where does the clock signal to the camera come from on your system? That's correct, the sensor driver only has one clock input, so it should just request the xvclk clock. If it comes from the ISI itself, you don't need to specify the clock in the DT, since the ISI doesn't produce a clock from DT. If you do want to have your clock consumer (ov2640) and the supplier (ISI) properly described in DT, you'll have to teach the ISI to register a CCF clock source, which then will be connected to from the ov2640. If you choose not to show your clock in the DT, you can just use v4l2_clk_get(dev, xvclk) and it will be handled by v4l2_clk / soc-camera / isi-atmel. If the closk to ov2640 is supplied by a separate clock source, then you v4l2_clk_get() will connect ov2640 to it directly and soc-camera will enable and disable it on power-on / -off as required. The ISI has no way to supply a sensor clock, the clock is supplied by a separate clock source. From your above description it looks like the clock to ov2640 is supplied by a separate source, but atmel-isi's .clock_start() / .clock_stop() functions still need to be called? By looking at those functions it looks like they turn on and off clocks, supplying the ISI itself... Instead of only turning on and off clocks, provided by the ISI to a camera sensor. If my understanding is right, then this is a bug in atmel-isi and it has to be fixed. That's correct as well, the ISI driver needs to be fixed. -- Regards, Laurent Pinchart -- 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 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi
After look deep into this patch, I found you miss one line that should
be changed as well.
It's In function v4l2_clk_get(), there still has one line code called
v4l2_clk_find(dev_id, id).
You need to change it to v4l2_clk_find(dev_id, NULL) as well.
Otherwise the code that many sensor used: v4l2_clk_get(client-dev,
mclk) cannot acquired the mclk clock.
After above changes, this patch works for me.
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24 +++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
- if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable clk-ops-disable)
clk-ops-disable(clk);
@@ -181,7 +182,7 @@ EXPORT_SYMBOL(v4l2_clk_set_rate);
struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
const char *dev_id,
- const char *id, void *priv)
+ void *priv)
{
struct v4l2_clk *clk;
int ret;
@@ -193,9 +194,8 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
if (!clk)
return ERR_PTR(-ENOMEM);
- clk-id = kstrdup(id, GFP_KERNEL);
clk-dev_id = kstrdup(dev_id, GFP_KERNEL);
- if ((id !clk-id) || !clk-dev_id) {
+ if (!clk-dev_id) {
ret = -ENOMEM;
goto ealloc;
}
@@ -205,7 +205,7 @@ struct v4l2_clk *v4l2_clk_register(const struct
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi Josh,
On Tue, 6 Jan 2015, Josh Wu wrote:
Hi, Guennadi
After look deep into this patch, I found you miss one line that should be
changed as well.
It's In function v4l2_clk_get(), there still has one line code called
v4l2_clk_find(dev_id, id).
You need to change it to v4l2_clk_find(dev_id, NULL) as well.
Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk)
cannot acquired the mclk clock.
After above changes, this patch works for me.
I think you're right, in fact, since we now don't store CCF-based v4l2_clk
wrappers on the list, this can be simplified even further, I'll update the
patch. Did you only test this patch or both?
Thanks
Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
- if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable clk-ops-disable)
clk-ops-disable(clk);
@@ -181,7 +182,7 @@ EXPORT_SYMBOL(v4l2_clk_set_rate);
struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
const char *dev_id,
- const char *id, void *priv)
+ void *priv)
{
struct v4l2_clk *clk;
int ret;
@@ -193,9 +194,8 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
if (!clk)
return ERR_PTR(-ENOMEM);
- clk-id =
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi
On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote:
Hi Josh,
On Tue, 6 Jan 2015, Josh Wu wrote:
Hi, Guennadi
After look deep into this patch, I found you miss one line that should be
changed as well.
It's In function v4l2_clk_get(), there still has one line code called
v4l2_clk_find(dev_id, id).
You need to change it to v4l2_clk_find(dev_id, NULL) as well.
Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk)
cannot acquired the mclk clock.
After above changes, this patch works for me.
I think you're right, in fact, since we now don't store CCF-based v4l2_clk
wrappers on the list, this can be simplified even further, I'll update the
patch. Did you only test this patch or both?
I tested both patches with Atmel-isi driver. For the 2/2 patch I applied
the modification Laurent suggested.
Those patches works for me.
The only concern is in ov2640 I still need to acquired two v4l2 clocks:
xvclk that will get the xvclk CCF clock directly.
mclk that make ISI driver call his clock_start()/stop() to
enable/disable ISI's peripheral clock.
If I only get xvclk clock, then the camera capture will be failed with a
ISI timeout error.
But I think this is acceptable as we will keep go forward. Finally we'll
switch to CCF and removed the v4l2_clock then we will move the
clock_start()/stop() caller code to soc_camera.c.
Best Regards,
Josh Wu
Thanks
Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
-if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi
On 1/7/2015 6:17 AM, Guennadi Liakhovetski wrote:
Hi Josh,
On Tue, 6 Jan 2015, Josh Wu wrote:
Hi, Guennadi
After look deep into this patch, I found you miss one line that should be
changed as well.
It's In function v4l2_clk_get(), there still has one line code called
v4l2_clk_find(dev_id, id).
You need to change it to v4l2_clk_find(dev_id, NULL) as well.
Otherwise the code that many sensor used: v4l2_clk_get(client-dev, mclk)
cannot acquired the mclk clock.
After above changes, this patch works for me.
I think you're right, in fact, since we now don't store CCF-based v4l2_clk
wrappers on the list, this can be simplified even further, I'll update the
patch. Did you only test this patch or both?
I tested both patches with Atmel-isi driver. For the 2/2 patch I applied
the modification Laurent suggested.
Those patches works for me.
The only concern is in ov2640 I still need to acquired two v4l2 clocks:
xvclk that will get the xvclk CCF clock directly.
mclk that make ISI driver call his clock_start()/stop() to
enable/disable ISI's peripheral clock.
If I only get xvclk clock, then the camera capture will be failed with a
ISI timeout error.
But I think this is acceptable as we will keep go forward. Finally we'll
switch to CCF and removed the v4l2_clock then we will move the
clock_start()/stop() caller code to soc_camera.c.
Best Regards,
Josh Wu
Thanks
Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
-if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi Josh,
On Mon, 5 Jan 2015, Josh Wu wrote:
Hi, Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
This patch not register a v4l2_clk with name: mclk. So this will break all
the i2c sensors that used the v4l2 clock mclk.
To make those drivers work, we need change all lines from:
v4l2_clk_get(client-dev, mclk);
to:
v4l2_clk_get(client-dev, NULL);
Am I understanding correctly?
No, it's the opposite. Clock consumers should request clock names,
according to their datasheets. In ov2640 case it's xvclk. For v4l2_clk the
name will simply be ignored. But it will be used to try to get a CCF clock
before falling back to V4L2 clocks.
Thanks
Guennadi
if yes, I'd like to define a SOC_CAMERA_HOST_CLK_ID in soc_camera.h, and all
code will use:
v4l2_clk_get(client-dev, SOC_CAMERA_HOST_CLK_ID);
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
- if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable clk-ops-disable)
clk-ops-disable(clk);
@@ -181,7 +182,7 @@ EXPORT_SYMBOL(v4l2_clk_set_rate);
struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
const char *dev_id,
- const char *id, void *priv)
+ void *priv)
{
struct v4l2_clk *clk;
int ret;
@@ -193,9 +194,8 @@ struct v4l2_clk *v4l2_clk_register(const struct
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi
On 1/5/2015 5:28 PM, Guennadi Liakhovetski wrote:
Hi Josh,
On Mon, 5 Jan 2015, Josh Wu wrote:
Hi, Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
This patch not register a v4l2_clk with name: mclk. So this will break all
the i2c sensors that used the v4l2 clock mclk.
To make those drivers work, we need change all lines from:
v4l2_clk_get(client-dev, mclk);
to:
v4l2_clk_get(client-dev, NULL);
Am I understanding correctly?
No, it's the opposite. Clock consumers should request clock names,
according to their datasheets. In ov2640 case it's xvclk. For v4l2_clk the
name will simply be ignored. But it will be used to try to get a CCF clock
before falling back to V4L2 clocks.
Okay, you mean I only need request xvclk clock via call v4l2_clk_get()
in ov2640 driver. right?
So the question is how do we operation the camera host's
.clock_start/stop()? Which is by v4l2_clk_get() old mclk.
hmm, wait. Since this part is not belong to ov2640, I think we should
move this part code to soc_camera.c 's function: soc_camera_power_on().
or do I miss anything?
Thanks
Guennadi
Best Regards,
Josh Wu
if yes, I'd like to define a SOC_CAMERA_HOST_CLK_ID in soc_camera.h, and all
code will use:
v4l2_clk_get(client-dev, SOC_CAMERA_HOST_CLK_ID);
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24
+++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct
soc_camera_device *icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host
*ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
-icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
-if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable
Re: [PATCH 1/2] V4L: remove clock name from v4l2_clk API
Hi, Guennadi
On 1/2/2015 7:48 PM, Guennadi Liakhovetski wrote:
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
This patch not register a v4l2_clk with name: mclk. So this will break
all the i2c sensors that used the v4l2 clock mclk.
To make those drivers work, we need change all lines from:
v4l2_clk_get(client-dev, mclk);
to:
v4l2_clk_get(client-dev, NULL);
Am I understanding correctly?
if yes, I'd like to define a SOC_CAMERA_HOST_CLK_ID in soc_camera.h, and
all code will use:
v4l2_clk_get(client-dev, SOC_CAMERA_HOST_CLK_ID);
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24 +++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk, icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
- if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable clk-ops-disable)
clk-ops-disable(clk);
@@ -181,7 +182,7 @@ EXPORT_SYMBOL(v4l2_clk_set_rate);
struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
const char *dev_id,
- const char *id, void *priv)
+ void *priv)
{
struct v4l2_clk *clk;
int ret;
@@ -193,9 +194,8 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
if (!clk)
return ERR_PTR(-ENOMEM);
- clk-id = kstrdup(id, GFP_KERNEL);
clk-dev_id = kstrdup(dev_id, GFP_KERNEL);
- if ((id !clk-id) || !clk-dev_id) {
+ if (!clk-dev_id) {
ret = -ENOMEM;
goto ealloc;
}
@@ -205,7 +205,7 @@ struct v4l2_clk
[PATCH 1/2] V4L: remove clock name from v4l2_clk API
All uses of the v4l2_clk API so far only register one clock with a fixed
name. This allows us to get rid of it, which also will make CCF and DT
integration easier.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
drivers/media/v4l2-core/v4l2-clk.c | 24 +++-
include/media/v4l2-clk.h | 7 +++
4 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index f4be2a1..ce192b6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1380,7 +1380,7 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
shd-i2c_adapter_id, shd-board_info-addr);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1561,7 +1561,7 @@ static int scan_async_group(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), %d-%04x,
sasd-asd.match.i2c.adapter_id, sasd-asd.match.i2c.address);
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
@@ -1666,7 +1666,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
snprintf(clk_name, sizeof(clk_name), of-%s,
of_node_full_name(remote));
- icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, mclk,
icd);
+ icd-clk = v4l2_clk_register(soc_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd-clk)) {
ret = PTR_ERR(icd-clk);
goto eclkreg;
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index 7be661f..a4b22c2 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,7 +330,7 @@ int em28xx_init_camera(struct em28xx *dev)
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
i2c_adapter_id(adap), client-addr);
- v4l2-clk = v4l2_clk_register_fixed(clk_name, mclk, -EINVAL);
+ v4l2-clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
if (IS_ERR(v4l2-clk))
return PTR_ERR(v4l2-clk);
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index e18cc04..c210906 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -31,7 +31,8 @@ static struct v4l2_clk *v4l2_clk_find(const char *dev_id,
const char *id)
if (strcmp(dev_id, clk-dev_id))
continue;
- if (!id || !clk-id || !strcmp(clk-id, id))
+ if ((!id !clk-id) ||
+ (id clk-id !strcmp(clk-id, id)))
return clk;
}
@@ -127,8 +128,8 @@ void v4l2_clk_disable(struct v4l2_clk *clk)
mutex_lock(clk-lock);
enable = --clk-enable;
- if (WARN(enable 0, Unbalanced %s() on %s:%s!\n, __func__,
-clk-dev_id, clk-id))
+ if (WARN(enable 0, Unbalanced %s() on %s!\n, __func__,
+clk-dev_id))
clk-enable++;
else if (!enable clk-ops-disable)
clk-ops-disable(clk);
@@ -181,7 +182,7 @@ EXPORT_SYMBOL(v4l2_clk_set_rate);
struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
const char *dev_id,
- const char *id, void *priv)
+ void *priv)
{
struct v4l2_clk *clk;
int ret;
@@ -193,9 +194,8 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
if (!clk)
return ERR_PTR(-ENOMEM);
- clk-id = kstrdup(id, GFP_KERNEL);
clk-dev_id = kstrdup(dev_id, GFP_KERNEL);
- if ((id !clk-id) || !clk-dev_id) {
+ if (!clk-dev_id) {
ret = -ENOMEM;
goto ealloc;
}
@@ -205,7 +205,7 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
mutex_init(clk-lock);
mutex_lock(clk_lock);
- if (!IS_ERR(v4l2_clk_find(dev_id, id))) {
+ if (!IS_ERR(v4l2_clk_find(dev_id, NULL))) {
mutex_unlock(clk_lock);
ret = -EEXIST;
goto eexist;
@@ -217,7 +217,6 @@ struct v4l2_clk *v4l2_clk_register(const struct
v4l2_clk_ops *ops,
eexist:
ealloc:
- kfree(clk-id);
kfree(clk-dev_id);
