diff mbox

[v3,4/6] exynos4-is: Add clock provider for the external clocks

Message ID 1382033211-32329-5-git-send-email-s.nawrocki@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

This patch adds clock provider to expose the sclk_cam0/1 clocks
for external image sensor devices.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v2:
 - use 'camera' DT node drirectly as clock provider node, rather than
  creating additional clock-controller child node.
---
 .../devicetree/bindings/media/samsung-fimc.txt     |   15 ++-
 drivers/media/platform/exynos4-is/media-dev.c      |  108 ++++++++++++++++++++
 drivers/media/platform/exynos4-is/media-dev.h      |   18 +++-
 3 files changed, 137 insertions(+), 4 deletions(-)

Comments

Mauro Carvalho Chehab Jan. 2, 2014, 7:58 p.m. UTC | #1
Em Thu, 17 Oct 2013 20:06:49 +0200
Sylwester Nawrocki <s.nawrocki@samsung.com> escreveu:

> This patch adds clock provider to expose the sclk_cam0/1 clocks
> for external image sensor devices.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Changes since v2:
>  - use 'camera' DT node drirectly as clock provider node, rather than
>   creating additional clock-controller child node.
> ---
>  .../devicetree/bindings/media/samsung-fimc.txt     |   15 ++-
>  drivers/media/platform/exynos4-is/media-dev.c      |  108 ++++++++++++++++++++
>  drivers/media/platform/exynos4-is/media-dev.h      |   18 +++-
>  3 files changed, 137 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> index 96312f6..968e065 100644
> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> @@ -32,6 +32,15 @@ way around.
>  
>  The 'camera' node must include at least one 'fimc' child node.
>  
> +Optional properties:
> +
> +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt),
> +  must be 1. A clock provider is associated with the camera node and it should
> +  be referenced by external sensors that use clocks provided by the SoC on
> +  CAM_*_CLKOUT pins. The second cell of the clock specifier is a clock's index.
> +  The indexes are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively.
> +
> +
>  'fimc' device nodes
>  -------------------
>  
> @@ -114,7 +123,7 @@ Example:
>  			vddio-supply = <...>;
>  
>  			clock-frequency = <24000000>;
> -			clocks = <...>;
> +			clocks = <&camera 1>;
>  			clock-names = "mclk";
>  
>  			port {
> @@ -135,7 +144,7 @@ Example:
>  			vddio-supply = <...>;
>  
>  			clock-frequency = <24000000>;
> -			clocks = <...>;
> +			clocks = <&camera 0>;
>  			clock-names = "mclk";
>  
>  			port {
> @@ -151,8 +160,8 @@ Example:
>  		compatible = "samsung,fimc", "simple-bus";
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> +		#clock-cells = <1>;
>  		status = "okay";
> -
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&cam_port_a_clk_active>;

I didn't see the above on the patch series you sent me on your git pull
request. Where is it?

>  
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index a835112..d78e3da 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -11,6 +11,8 @@
>   */
>  
>  #include <linux/bug.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
>  #include <linux/i2c.h>
> @@ -1437,6 +1439,101 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static int cam_clk_prepare(struct clk_hw *hw)
> +{
> +	struct cam_clk *camclk = to_cam_clk(hw);
> +	int ret;
> +
> +	if (camclk->fmd->pmf == NULL)
> +		return -ENODEV;
> +
> +	ret = pm_runtime_get_sync(camclk->fmd->pmf);
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static void cam_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct cam_clk *camclk = to_cam_clk(hw);
> +
> +	if (camclk->fmd->pmf == NULL)
> +		return;
> +
> +	pm_runtime_put_sync(camclk->fmd->pmf);
> +}
> +
> +static const struct clk_ops cam_clk_ops = {
> +	.prepare = cam_clk_prepare,
> +	.unprepare = cam_clk_unprepare,
> +};
> +
> +static const char *cam_clk_p_names[] = { "sclk_cam0", "sclk_cam1" };
> +
> +static void fimc_md_unregister_clk_provider(struct fimc_md *fmd)
> +{
> +	struct cam_clk_provider *cp = &fmd->clk_provider;
> +	unsigned int i;
> +
> +	if (cp->of_node)
> +		of_clk_del_provider(cp->of_node);
> +
> +	for (i = 0; i < ARRAY_SIZE(cp->clks); i++)
> +		if (!IS_ERR(cp->clks[i]))
> +			clk_unregister(cp->clks[i]);

Huh? Why to initialize an array with an error code??? Does it make sense to
have one of the clocks with an error and the others ok, and to store the
error code? The code below doesn't seem to allow that.

Just initialize cp->clks with zero and test it with:

	if (cp->clks[i])
		clk_unregister(cp->clks[i]);

That makes it easier to understand and review.

> +}
> +
> +static int fimc_md_register_clk_provider(struct fimc_md *fmd)
> +{
> +	struct cam_clk_provider *cp = &fmd->clk_provider;
> +	struct device *dev = &fmd->pdev->dev;
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(cp->clks); i++)
> +		cp->clks[i] = ERR_PTR(-EINVAL);

That looks weird for me, due to several reasons:

1) ARRAY_SIZE(cp->clks) is equal to FIMC_MAX_CAMCLKS. Why are you using
different syntaxes on the first loop and on the next one? Just to loose
more time from reviewers to double check what number is bigger?

2) Why don't you just do:

	memset(cp->clks, ARRAY_SIZE(cp->clks), 0).

or initialize struct fimc_md with kzalloc()?

> +
> +	for (i = 0; i < FIMC_MAX_CAMCLKS; i++) {
> +		struct cam_clk *camclk = &cp->camclk[i];
> +		struct clk_init_data init;
> +		char clk_name[16];
> +		struct clk *clk;
> +
> +		snprintf(clk_name, sizeof(clk_name), "cam_clkout%d", i);
> +
> +		init.name = clk_name;
> +		init.ops = &cam_clk_ops;
> +		init.flags = CLK_SET_RATE_PARENT;
> +		init.parent_names = &cam_clk_p_names[i];
> +		init.num_parents = 1;
> +		camclk->hw.init = &init;
> +		camclk->fmd = fmd;
> +
> +		clk = clk_register(dev, &camclk->hw);
> +		if (IS_ERR(clk)) {
> +			dev_err(dev, "failed to register clock: %s (%ld)\n",
> +						clk_name, PTR_ERR(clk));
> +			ret = PTR_ERR(clk);
> +			goto err;
> +		}
> +		cp->clks[i] = clk;
> +	}
> +
> +	cp->clk_data.clks = cp->clks;
> +	cp->clk_data.clk_num = i;
> +	cp->of_node = dev->of_node;
> +
> +	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> +				  &cp->clk_data);
> +	if (!ret)
> +		return 0;
> +err:
> +	fimc_md_unregister_clk_provider(fmd);
> +	return ret;
> +}
> +#else
> +#define fimc_md_register_clk_provider(fmd) (0)
> +#define fimc_md_unregister_clk_provider(fmd) (0)
> +#endif
> +
>  static int fimc_md_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1464,16 +1561,24 @@ static int fimc_md_probe(struct platform_device *pdev)
>  
>  	fmd->use_isp = fimc_md_is_isp_available(dev->of_node);
>  
> +	ret = fimc_md_register_clk_provider(fmd);
> +	if (ret < 0) {
> +		v4l2_err(v4l2_dev, "clock provider registration failed\n");
> +		return ret;
> +	}
> +
>  	ret = v4l2_device_register(dev, &fmd->v4l2_dev);
>  	if (ret < 0) {
>  		v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
>  		return ret;
>  	}
> +
>  	ret = media_device_register(&fmd->media_dev);
>  	if (ret < 0) {
>  		v4l2_err(v4l2_dev, "Failed to register media device: %d\n", ret);
>  		goto err_md;
>  	}
> +
>  	ret = fimc_md_get_clocks(fmd);
>  	if (ret)
>  		goto err_clk;
> @@ -1507,6 +1612,7 @@ static int fimc_md_probe(struct platform_device *pdev)
>  	ret = fimc_md_create_links(fmd);
>  	if (ret)
>  		goto err_unlock;
> +
>  	ret = v4l2_device_register_subdev_nodes(&fmd->v4l2_dev);
>  	if (ret)
>  		goto err_unlock;
> @@ -1527,6 +1633,7 @@ err_clk:
>  	media_device_unregister(&fmd->media_dev);
>  err_md:
>  	v4l2_device_unregister(&fmd->v4l2_dev);
> +	fimc_md_unregister_clk_provider(fmd);
>  	return ret;
>  }
>  
> @@ -1537,6 +1644,7 @@ static int fimc_md_remove(struct platform_device *pdev)
>  	if (!fmd)
>  		return 0;
>  
> +	fimc_md_unregister_clk_provider(fmd);
>  	v4l2_device_unregister(&fmd->v4l2_dev);
>  	device_remove_file(&pdev->dev, &dev_attr_subdev_conf_mode);
>  	fimc_md_unregister_entities(fmd);
> diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
> index 62599fd..240ca71 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.h
> +++ b/drivers/media/platform/exynos4-is/media-dev.h
> @@ -10,6 +10,7 @@
>  #define FIMC_MDEVICE_H_
>  
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/platform_device.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
> @@ -89,6 +90,12 @@ struct fimc_sensor_info {
>  	struct fimc_dev *host;
>  };
>  
> +struct cam_clk {
> +	struct clk_hw hw;
> +	struct fimc_md *fmd;
> +};
> +#define to_cam_clk(_hw) container_of(_hw, struct cam_clk, hw)
> +
>  /**
>   * struct fimc_md - fimc media device information
>   * @csis: MIPI CSIS subdevs data
> @@ -105,6 +112,7 @@ struct fimc_sensor_info {
>   * @pinctrl: camera port pinctrl handle
>   * @state_default: pinctrl default state handle
>   * @state_idle: pinctrl idle state handle
> + * @cam_clk_provider: CAMCLK clock provider structure
>   * @user_subdev_api: true if subdevs are not configured by the host driver
>   * @slock: spinlock protecting @sensor array
>   */
> @@ -122,13 +130,21 @@ struct fimc_md {
>  	struct media_device media_dev;
>  	struct v4l2_device v4l2_dev;
>  	struct platform_device *pdev;
> +
>  	struct fimc_pinctrl {
>  		struct pinctrl *pinctrl;
>  		struct pinctrl_state *state_default;
>  		struct pinctrl_state *state_idle;
>  	} pinctl;
> -	bool user_subdev_api;
>  
> +	struct cam_clk_provider {
> +		struct clk *clks[FIMC_MAX_CAMCLKS];
> +		struct clk_onecell_data clk_data;
> +		struct device_node *of_node;
> +		struct cam_clk camclk[FIMC_MAX_CAMCLKS];
> +	} clk_provider;
> +
> +	bool user_subdev_api;
>  	spinlock_t slock;
>  	struct list_head pipelines;
>  };
Sylwester Nawrocki Jan. 7, 2014, 9:23 p.m. UTC | #2
On 01/02/2014 08:58 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 17 Oct 2013 20:06:49 +0200
> Sylwester Nawrocki<s.nawrocki@samsung.com>  escreveu:
>
>> This patch adds clock provider to expose the sclk_cam0/1 clocks
>> for external image sensor devices.
>>
>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>> ---
>> Changes since v2:
>>   - use 'camera' DT node drirectly as clock provider node, rather than
>>    creating additional clock-controller child node.
>> ---
>>   .../devicetree/bindings/media/samsung-fimc.txt     |   15 ++-
>>   drivers/media/platform/exynos4-is/media-dev.c      |  108 ++++++++++++++++++++
>>   drivers/media/platform/exynos4-is/media-dev.h      |   18 +++-
>>   3 files changed, 137 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> index 96312f6..968e065 100644
>> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> @@ -32,6 +32,15 @@ way around.
>>
>>   The 'camera' node must include at least one 'fimc' child node.
>>
>> +Optional properties:
>> +
>> +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt),
>> +  must be 1. A clock provider is associated with the camera node and it should
>> +  be referenced by external sensors that use clocks provided by the SoC on
>> +  CAM_*_CLKOUT pins. The second cell of the clock specifier is a clock's index.
>> +  The indexes are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively.
>> +
>> +
>>   'fimc' device nodes
>>   -------------------
>>
>> @@ -114,7 +123,7 @@ Example:
>>   			vddio-supply =<...>;
>>
>>   			clock-frequency =<24000000>;
>> -			clocks =<...>;
>> +			clocks =<&camera 1>;
>>   			clock-names = "mclk";
>>
>>   			port {
>> @@ -135,7 +144,7 @@ Example:
>>   			vddio-supply =<...>;
>>
>>   			clock-frequency =<24000000>;
>> -			clocks =<...>;
>> +			clocks =<&camera 0>;
>>   			clock-names = "mclk";
>>
>>   			port {
>> @@ -151,8 +160,8 @@ Example:
>>   		compatible = "samsung,fimc", "simple-bus";
>>   		#address-cells =<1>;
>>   		#size-cells =<1>;
>> +		#clock-cells =<1>;
>>   		status = "okay";
>> -
>>   		pinctrl-names = "default";
>>   		pinctrl-0 =<&cam_port_a_clk_active>;
>
> I didn't see the above on the patch series you sent me on your git pull
> request. Where is it?

Mauro, I've moved it into a separate patch, since it is more appropriate
as that's a change in the DT binding [1].

My plan was to send it through the devicetree tree with your Ack, but
I forgot to resend the whole series. Sorry about that.

Moreover I did this split because you are requiring an explicit ACK
from a DT maintainer for anything that is related to devicetree.
I believe it is physically impossible to have every single patch
acked by a DT maintainer. Subsystem maintainers should handle minor
stuff, otherwise we won't be able to have _anything_ merged upstream.

Pinging the DT maintainers for every single patch seems a bit silly and
I'm already fed up with it. I find this whole process frustrating and very
long times to get things merged really discourage to develop anything in
mainline.

[...]
>> +static const char *cam_clk_p_names[] = { "sclk_cam0", "sclk_cam1" };
>> +
>> +static void fimc_md_unregister_clk_provider(struct fimc_md *fmd)
>> +{
>> +	struct cam_clk_provider *cp =&fmd->clk_provider;
>> +	unsigned int i;
>> +
>> +	if (cp->of_node)
>> +		of_clk_del_provider(cp->of_node);
>> +
>> +	for (i = 0; i<  ARRAY_SIZE(cp->clks); i++)
>> +		if (!IS_ERR(cp->clks[i]))
>> +			clk_unregister(cp->clks[i]);
>
> Huh? Why to initialize an array with an error code??? Does it make sense to
> have one of the clocks with an error and the others ok, and to store the
> error code? The code below doesn't seem to allow that.

If fimc_md_unregister_clk_provider() fails at any point
fimc_md_register_clk_provider() can be called to undo what has been
registered successfully. That's the point of storing the error codes.
The error paths in probe() are already complex enough.

> Just initialize cp->clks with zero and test it with:
>
> 	if (cp->clks[i])
> 		clk_unregister(cp->clks[i]);

I'm afraid this is incorrect, clock pointers should be treated as
opaque cookies and tested only with IS_ERR(). Clk users should not
assume NULL clk pointer indicate an invalid clock.

> That makes it easier to understand and review.

Maybe it is easier to understand but it is a wrong API usage. And
I believe it should never be suggested in reviews.

>> +}
>> +
>> +static int fimc_md_register_clk_provider(struct fimc_md *fmd)
>> +{
>> +	struct cam_clk_provider *cp =&fmd->clk_provider;
>> +	struct device *dev =&fmd->pdev->dev;
>> +	int i, ret;
>> +
>> +	for (i = 0; i<  ARRAY_SIZE(cp->clks); i++)
>> +		cp->clks[i] = ERR_PTR(-EINVAL);
>
> That looks weird for me, due to several reasons:
>
> 1) ARRAY_SIZE(cp->clks) is equal to FIMC_MAX_CAMCLKS. Why are you using
> different syntaxes on the first loop and on the next one? Just to loose
> more time from reviewers to double check what number is bigger?

The loop limit argument is valid, I can correct that so the code is
easier to follow.

> 2) Why don't you just do:
>
> 	memset(cp->clks, ARRAY_SIZE(cp->clks), 0).
>
> or initialize struct fimc_md with kzalloc()?

Because it isn't correct. struct fimc_md is already allocated with
kzalloc(), I would certainly consider the above simpler code if it
were valid.

>> +
>> +	for (i = 0; i<  FIMC_MAX_CAMCLKS; i++) {
>> +		struct cam_clk *camclk =&cp->camclk[i];
>> +		struct clk_init_data init;
>> +		char clk_name[16];
>> +		struct clk *clk;
>> +
>> +		snprintf(clk_name, sizeof(clk_name), "cam_clkout%d", i);
>> +
>> +		init.name = clk_name;
>> +		init.ops =&cam_clk_ops;
>> +		init.flags = CLK_SET_RATE_PARENT;
>> +		init.parent_names =&cam_clk_p_names[i];
>> +		init.num_parents = 1;
>> +		camclk->hw.init =&init;
>> +		camclk->fmd = fmd;
>> +
>> +		clk = clk_register(dev,&camclk->hw);
>> +		if (IS_ERR(clk)) {
>> +			dev_err(dev, "failed to register clock: %s (%ld)\n",
>> +						clk_name, PTR_ERR(clk));
>> +			ret = PTR_ERR(clk);
>> +			goto err;
>> +		}
>> +		cp->clks[i] = clk;
>> +	}
>> +
>> +	cp->clk_data.clks = cp->clks;
>> +	cp->clk_data.clk_num = i;
>> +	cp->of_node = dev->of_node;
>> +
>> +	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
>> +				&cp->clk_data);
>> +	if (!ret)
>> +		return 0;
>> +err:
>> +	fimc_md_unregister_clk_provider(fmd);
>> +	return ret;
>> +}
>> +#else
>> +#define fimc_md_register_clk_provider(fmd) (0)
>> +#define fimc_md_unregister_clk_provider(fmd) (0)
>> +#endif

--
Regards,
Sylwester

[1] 
http://git.linuxtv.org/snawrocki/samsung.git/shortlog/refs/heads/v3.14-exynos4-is-dt
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 96312f6..968e065 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -32,6 +32,15 @@  way around.
 
 The 'camera' node must include at least one 'fimc' child node.
 
+Optional properties:
+
+- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt),
+  must be 1. A clock provider is associated with the camera node and it should
+  be referenced by external sensors that use clocks provided by the SoC on
+  CAM_*_CLKOUT pins. The second cell of the clock specifier is a clock's index.
+  The indexes are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively.
+
+
 'fimc' device nodes
 -------------------
 
@@ -114,7 +123,7 @@  Example:
 			vddio-supply = <...>;
 
 			clock-frequency = <24000000>;
-			clocks = <...>;
+			clocks = <&camera 1>;
 			clock-names = "mclk";
 
 			port {
@@ -135,7 +144,7 @@  Example:
 			vddio-supply = <...>;
 
 			clock-frequency = <24000000>;
-			clocks = <...>;
+			clocks = <&camera 0>;
 			clock-names = "mclk";
 
 			port {
@@ -151,8 +160,8 @@  Example:
 		compatible = "samsung,fimc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
+		#clock-cells = <1>;
 		status = "okay";
-
 		pinctrl-names = "default";
 		pinctrl-0 = <&cam_port_a_clk_active>;
 
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index a835112..d78e3da 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -11,6 +11,8 @@ 
  */
 
 #include <linux/bug.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
@@ -1437,6 +1439,101 @@  static int fimc_md_get_pinctrl(struct fimc_md *fmd)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static int cam_clk_prepare(struct clk_hw *hw)
+{
+	struct cam_clk *camclk = to_cam_clk(hw);
+	int ret;
+
+	if (camclk->fmd->pmf == NULL)
+		return -ENODEV;
+
+	ret = pm_runtime_get_sync(camclk->fmd->pmf);
+	return ret < 0 ? ret : 0;
+}
+
+static void cam_clk_unprepare(struct clk_hw *hw)
+{
+	struct cam_clk *camclk = to_cam_clk(hw);
+
+	if (camclk->fmd->pmf == NULL)
+		return;
+
+	pm_runtime_put_sync(camclk->fmd->pmf);
+}
+
+static const struct clk_ops cam_clk_ops = {
+	.prepare = cam_clk_prepare,
+	.unprepare = cam_clk_unprepare,
+};
+
+static const char *cam_clk_p_names[] = { "sclk_cam0", "sclk_cam1" };
+
+static void fimc_md_unregister_clk_provider(struct fimc_md *fmd)
+{
+	struct cam_clk_provider *cp = &fmd->clk_provider;
+	unsigned int i;
+
+	if (cp->of_node)
+		of_clk_del_provider(cp->of_node);
+
+	for (i = 0; i < ARRAY_SIZE(cp->clks); i++)
+		if (!IS_ERR(cp->clks[i]))
+			clk_unregister(cp->clks[i]);
+}
+
+static int fimc_md_register_clk_provider(struct fimc_md *fmd)
+{
+	struct cam_clk_provider *cp = &fmd->clk_provider;
+	struct device *dev = &fmd->pdev->dev;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(cp->clks); i++)
+		cp->clks[i] = ERR_PTR(-EINVAL);
+
+	for (i = 0; i < FIMC_MAX_CAMCLKS; i++) {
+		struct cam_clk *camclk = &cp->camclk[i];
+		struct clk_init_data init;
+		char clk_name[16];
+		struct clk *clk;
+
+		snprintf(clk_name, sizeof(clk_name), "cam_clkout%d", i);
+
+		init.name = clk_name;
+		init.ops = &cam_clk_ops;
+		init.flags = CLK_SET_RATE_PARENT;
+		init.parent_names = &cam_clk_p_names[i];
+		init.num_parents = 1;
+		camclk->hw.init = &init;
+		camclk->fmd = fmd;
+
+		clk = clk_register(dev, &camclk->hw);
+		if (IS_ERR(clk)) {
+			dev_err(dev, "failed to register clock: %s (%ld)\n",
+						clk_name, PTR_ERR(clk));
+			ret = PTR_ERR(clk);
+			goto err;
+		}
+		cp->clks[i] = clk;
+	}
+
+	cp->clk_data.clks = cp->clks;
+	cp->clk_data.clk_num = i;
+	cp->of_node = dev->of_node;
+
+	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
+				  &cp->clk_data);
+	if (!ret)
+		return 0;
+err:
+	fimc_md_unregister_clk_provider(fmd);
+	return ret;
+}
+#else
+#define fimc_md_register_clk_provider(fmd) (0)
+#define fimc_md_unregister_clk_provider(fmd) (0)
+#endif
+
 static int fimc_md_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1464,16 +1561,24 @@  static int fimc_md_probe(struct platform_device *pdev)
 
 	fmd->use_isp = fimc_md_is_isp_available(dev->of_node);
 
+	ret = fimc_md_register_clk_provider(fmd);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "clock provider registration failed\n");
+		return ret;
+	}
+
 	ret = v4l2_device_register(dev, &fmd->v4l2_dev);
 	if (ret < 0) {
 		v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
 		return ret;
 	}
+
 	ret = media_device_register(&fmd->media_dev);
 	if (ret < 0) {
 		v4l2_err(v4l2_dev, "Failed to register media device: %d\n", ret);
 		goto err_md;
 	}
+
 	ret = fimc_md_get_clocks(fmd);
 	if (ret)
 		goto err_clk;
@@ -1507,6 +1612,7 @@  static int fimc_md_probe(struct platform_device *pdev)
 	ret = fimc_md_create_links(fmd);
 	if (ret)
 		goto err_unlock;
+
 	ret = v4l2_device_register_subdev_nodes(&fmd->v4l2_dev);
 	if (ret)
 		goto err_unlock;
@@ -1527,6 +1633,7 @@  err_clk:
 	media_device_unregister(&fmd->media_dev);
 err_md:
 	v4l2_device_unregister(&fmd->v4l2_dev);
+	fimc_md_unregister_clk_provider(fmd);
 	return ret;
 }
 
@@ -1537,6 +1644,7 @@  static int fimc_md_remove(struct platform_device *pdev)
 	if (!fmd)
 		return 0;
 
+	fimc_md_unregister_clk_provider(fmd);
 	v4l2_device_unregister(&fmd->v4l2_dev);
 	device_remove_file(&pdev->dev, &dev_attr_subdev_conf_mode);
 	fimc_md_unregister_entities(fmd);
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 62599fd..240ca71 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -10,6 +10,7 @@ 
 #define FIMC_MDEVICE_H_
 
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/platform_device.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -89,6 +90,12 @@  struct fimc_sensor_info {
 	struct fimc_dev *host;
 };
 
+struct cam_clk {
+	struct clk_hw hw;
+	struct fimc_md *fmd;
+};
+#define to_cam_clk(_hw) container_of(_hw, struct cam_clk, hw)
+
 /**
  * struct fimc_md - fimc media device information
  * @csis: MIPI CSIS subdevs data
@@ -105,6 +112,7 @@  struct fimc_sensor_info {
  * @pinctrl: camera port pinctrl handle
  * @state_default: pinctrl default state handle
  * @state_idle: pinctrl idle state handle
+ * @cam_clk_provider: CAMCLK clock provider structure
  * @user_subdev_api: true if subdevs are not configured by the host driver
  * @slock: spinlock protecting @sensor array
  */
@@ -122,13 +130,21 @@  struct fimc_md {
 	struct media_device media_dev;
 	struct v4l2_device v4l2_dev;
 	struct platform_device *pdev;
+
 	struct fimc_pinctrl {
 		struct pinctrl *pinctrl;
 		struct pinctrl_state *state_default;
 		struct pinctrl_state *state_idle;
 	} pinctl;
-	bool user_subdev_api;
 
+	struct cam_clk_provider {
+		struct clk *clks[FIMC_MAX_CAMCLKS];
+		struct clk_onecell_data clk_data;
+		struct device_node *of_node;
+		struct cam_clk camclk[FIMC_MAX_CAMCLKS];
+	} clk_provider;
+
+	bool user_subdev_api;
 	spinlock_t slock;
 	struct list_head pipelines;
 };