Skip to contentSkip to conversation

clk: max77686: Clock provider implementation fixes

PATCH Superseded Tomasz Figa 4 participants 21 messages Dec 12, 2013 – Jan 8, 2014

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Tested on Exynos4412-based Trats2 board, with additional patches adding driver being a consumer of one of max77686 clocks.

Changes since v1:
- rebased on top of current clk-next (no functional changes)

Tomasz Figa (8):
clk: max77686: Correct callback used for checking clock status
clk: max77686: Provide .recalc_rate() operation
clk: max77686: Refactor successful exit of probe function
clk: max77686: Make max77686_clk_register() return struct clk *
clk: max77686: Fix clean-up in error and remove paths
clk: max77686: Refactor driver data handling
clk: max77686: Register OF clock provider
ARM: dts: Add clock provider specific properties to max77686 node

 .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++
 Documentation/devicetree/bindings/mfd/max77686.txt |  3 +
 arch/arm/boot/dts/cros5250-common.dtsi             |  1 +
 arch/arm/boot/dts/exynos4412-odroidx.dts           |  1 +
 arch/arm/boot/dts/exynos4412-trats2.dts            |  1 +
 drivers/clk/clk-max77686.c                         | 97 +++++++++++++---------
 6 files changed, 104 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/maxim,max77686.txt
Signature · 1 line
1.8.4.3
#1 lore raw

PATCH v2 1/8 clk: max77686: Correct callback used for checking clock status

Changing status of clock gates in max77686 requires i2c transfers, which can sleep, so this is done in prepare and unprepare callbacks. Due to this, checking whether whether the clock is ungated must be done in is_prepared() callback as well, for consistency.

---
 drivers/clk/clk-max77686.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/clk/clk-max77686.c +2 -2
index 9f57bc3..e39d463 100644
@@ -66,7 +66,7 @@ static void max77686_clk_unprepare(struct clk_hw *hw)
 		MAX77686_REG_32KHZ, max77686->mask, ~max77686->mask);
 }
 
-static int max77686_clk_is_enabled(struct clk_hw *hw)
+static int max77686_clk_is_prepared(struct clk_hw *hw)
 {
 	struct max77686_clk *max77686 = to_max77686_clk(hw);
 	int ret;
@@ -84,7 +84,7 @@ static int max77686_clk_is_enabled(struct clk_hw *hw)
 static struct clk_ops max77686_clk_ops = {
 	.prepare	= max77686_clk_prepare,
 	.unprepare	= max77686_clk_unprepare,
-	.is_enabled	= max77686_clk_is_enabled,
+	.is_prepared	= max77686_clk_is_prepared,
 };
 
 static struct clk_init_data max77686_clks_init[MAX77686_CLKS_NUM] = {
Signature · 1 line
1.8.4.3
#2 lore raw

PATCH v2 2/8 clk: max77686: Provide .recalc_rate() operation

In reply to Tomasz Figa

It is usually nice to know frequency of a clock, so this patch adds a .recalc_rate() callback returning rates of provided clocks.

---
 drivers/clk/clk-max77686.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/clk/clk-max77686.c +7 -0
index e39d463..e40ecdb 100644
@@ -81,10 +81,17 @@ static int max77686_clk_is_prepared(struct clk_hw *hw)
 	return val & max77686->mask;
 }
 
+static unsigned long max77686_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	return 32768;
+}
+
 static struct clk_ops max77686_clk_ops = {
 	.prepare	= max77686_clk_prepare,
 	.unprepare	= max77686_clk_unprepare,
 	.is_prepared	= max77686_clk_is_prepared,
+	.recalc_rate	= max77686_recalc_rate,
 };
 
 static struct clk_init_data max77686_clks_init[MAX77686_CLKS_NUM] = {
Signature · 1 line
1.8.4.3
#3 lore raw

PATCH v2 3/8 clk: max77686: Refactor successful exit of probe function

In reply to Tomasz Figa

The function can simply return 0, without jumping to a separate label, which does exactly the same. This patch does not introduce any functional change, just a clean-up.

---
 drivers/clk/clk-max77686.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/clk/clk-max77686.c +1 -2
index e40ecdb..5d78460 100644
@@ -178,7 +178,7 @@ static int max77686_clk_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, max77686_clks);
 
-	goto out;
+	return 0;
 
 err_clk_pmic:
 	clkdev_drop(max77686_clks[MAX77686_CLK_CP]->lookup);
@@ -187,7 +187,6 @@ err_clk_cp:
 	clkdev_drop(max77686_clks[MAX77686_CLK_AP]->lookup);
 	kfree(max77686_clks[MAX77686_CLK_AP]->hw.clk);
 err_clk_ap:
-out:
 	return ret;
 }
 
Signature · 1 line
1.8.4.3
#4 lore raw

PATCH v2 4/8 clk: max77686: Make max77686_clk_register() return struct clk *

In reply to Tomasz Figa

As a preparation for further patches, this patch modifies the clock registration helper function to return a pointer to the newly registered clock. No functional change is done to the driver.

---
 drivers/clk/clk-max77686.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/clk/clk-max77686.c +10 -7
index 5d78460..2e6ba0e 100644
@@ -112,27 +112,26 @@ static struct clk_init_data max77686_clks_init[MAX77686_CLKS_NUM] = {
 	},
 };
 
-static int max77686_clk_register(struct device *dev,
+static struct clk *max77686_clk_register(struct device *dev,
 				struct max77686_clk *max77686)
 {
 	struct clk *clk;
 	struct clk_hw *hw = &max77686->hw;
 
 	clk = clk_register(dev, hw);
-
 	if (IS_ERR(clk))
-		return -ENOMEM;
+		return clk;
 
 	max77686->lookup = kzalloc(sizeof(struct clk_lookup), GFP_KERNEL);
 	if (!max77686->lookup)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	max77686->lookup->con_id = hw->init->name;
 	max77686->lookup->clk = clk;
 
 	clkdev_add(max77686->lookup);
 
-	return 0;
+	return clk;
 }
 
 static int max77686_clk_probe(struct platform_device *pdev)
@@ -154,12 +153,16 @@ static int max77686_clk_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
+		struct clk *clk;
+
 		max77686_clks[i]->iodev = iodev;
 		max77686_clks[i]->mask = 1 << i;
 		max77686_clks[i]->hw.init = &max77686_clks_init[i];
 
-		ret = max77686_clk_register(&pdev->dev, max77686_clks[i]);
-		if (ret) {
+		clk = max77686_clk_register(&pdev->dev, max77686_clks[i]);
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+
 			switch (i) {
 			case MAX77686_CLK_AP:
 				dev_err(&pdev->dev, "Fail to register CLK_AP\n");
Signature · 1 line
1.8.4.3
#5 lore raw

PATCH v2 5/8 clk: max77686: Fix clean-up in error and remove paths

In reply to Tomasz Figa

This patch fixes invalid kfree() and adds missing call to clk_unregister() in error and remove paths in max77686_clk_probe(). While at it, error handling is also cleaned up.

---
 drivers/clk/clk-max77686.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)
drivers/clk/clk-max77686.c +10 -22
index 2e6ba0e..81b3680 100644
@@ -162,20 +162,9 @@ static int max77686_clk_probe(struct platform_device *pdev)
 		clk = max77686_clk_register(&pdev->dev, max77686_clks[i]);
 		if (IS_ERR(clk)) {
 			ret = PTR_ERR(clk);
-
-			switch (i) {
-			case MAX77686_CLK_AP:
-				dev_err(&pdev->dev, "Fail to register CLK_AP\n");
-				goto err_clk_ap;
-				break;
-			case MAX77686_CLK_CP:
-				dev_err(&pdev->dev, "Fail to register CLK_CP\n");
-				goto err_clk_cp;
-				break;
-			case MAX77686_CLK_PMIC:
-				dev_err(&pdev->dev, "Fail to register CLK_PMIC\n");
-				goto err_clk_pmic;
-			}
+			dev_err(&pdev->dev, "failed to register %s\n",
+				max77686_clks[i]->hw.init->name);
+			goto err_clocks;
 		}
 	}
 
@@ -183,13 +172,12 @@ static int max77686_clk_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_clk_pmic:
-	clkdev_drop(max77686_clks[MAX77686_CLK_CP]->lookup);
-	kfree(max77686_clks[MAX77686_CLK_CP]->hw.clk);
-err_clk_cp:
-	clkdev_drop(max77686_clks[MAX77686_CLK_AP]->lookup);
-	kfree(max77686_clks[MAX77686_CLK_AP]->hw.clk);
-err_clk_ap:
+err_clocks:
+	for (--i; i >= 0; --i) {
+		clkdev_drop(max77686_clks[i]->lookup);
+		clk_unregister(max77686_clks[i]->hw.clk);
+	}
+
 	return ret;
 }
 
@@ -200,7 +188,7 @@ static int max77686_clk_remove(struct platform_device *pdev)
 
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
 		clkdev_drop(max77686_clks[i]->lookup);
-		kfree(max77686_clks[i]->hw.clk);
+		clk_unregister(max77686_clks[i]->hw.clk);
 	}
 	return 0;
 }
Signature · 1 line
1.8.4.3
#6 lore raw

PATCH v2 6/8 clk: max77686: Refactor driver data handling

In reply to Tomasz Figa

As a prerequisite for further patch adding OF clock provider support to the driver, this patch changes the driver to store an array of struct clk * as driver data.

---
 drivers/clk/clk-max77686.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)
drivers/clk/clk-max77686.c +14 -12
index 81b3680..3cf38dc 100644
@@ -137,12 +137,13 @@ static struct clk *max77686_clk_register(struct device *dev,
 static int max77686_clk_probe(struct platform_device *pdev)
 {
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max77686_clk **max77686_clks;
+	struct max77686_clk *max77686_clks[MAX77686_CLKS_NUM];
+	struct clk **clocks;
 	int i, ret;
 
-	max77686_clks = devm_kzalloc(&pdev->dev, sizeof(struct max77686_clk *)
+	clocks = devm_kzalloc(&pdev->dev, sizeof(struct clk *)
 					* MAX77686_CLKS_NUM, GFP_KERNEL);
-	if (!max77686_clks)
+	if (!clocks)
 		return -ENOMEM;
 
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
@@ -153,22 +154,20 @@ static int max77686_clk_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
-		struct clk *clk;
-
 		max77686_clks[i]->iodev = iodev;
 		max77686_clks[i]->mask = 1 << i;
 		max77686_clks[i]->hw.init = &max77686_clks_init[i];
 
-		clk = max77686_clk_register(&pdev->dev, max77686_clks[i]);
-		if (IS_ERR(clk)) {
-			ret = PTR_ERR(clk);
+		clocks[i] = max77686_clk_register(&pdev->dev, max77686_clks[i]);
+		if (IS_ERR(clocks[i])) {
+			ret = PTR_ERR(clocks[i]);
 			dev_err(&pdev->dev, "failed to register %s\n",
 				max77686_clks[i]->hw.init->name);
 			goto err_clocks;
 		}
 	}
 
-	platform_set_drvdata(pdev, max77686_clks);
+	platform_set_drvdata(pdev, clocks);
 
 	return 0;
 
@@ -183,12 +182,15 @@ err_clocks:
 
 static int max77686_clk_remove(struct platform_device *pdev)
 {
-	struct max77686_clk **max77686_clks = platform_get_drvdata(pdev);
+	struct clk **clocks = platform_get_drvdata(pdev);
 	int i;
 
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
-		clkdev_drop(max77686_clks[i]->lookup);
-		clk_unregister(max77686_clks[i]->hw.clk);
+		struct clk_hw *hw = __clk_get_hw(clocks[i]);
+		struct max77686_clk *max77686 = to_max77686_clk(hw);
+
+		clkdev_drop(max77686->lookup);
+		clk_unregister(clocks[i]);
 	}
 	return 0;
 }
Signature · 1 line
1.8.4.3
#7 lore raw

PATCH v2 7/8 clk: max77686: Register OF clock provider

In reply to Tomasz Figa

If max77686 chip is instantiated from device tree, it is desirable to have an OF clock provider to allow device tree based look-up of clocks. This patch adds OF clock provider registration to the clk-max77686 driver.

---
 .../devicetree/bindings/clock/maxim,max77686.txt   | 38 ++++++++++++++++++++++
 Documentation/devicetree/bindings/mfd/max77686.txt |  3 ++
 drivers/clk/clk-max77686.c                         | 24 ++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/maxim,max77686.txt
Documentation/devicetree/bindings/clock/maxim,max77686.txt new file +38 -0
new file mode 100644
index 0000000..96ce71b
@@ -0,0 +1,38 @@
+Binding for Maxim MAX77686 32k clock generator block
+
+This is a part of device tree bindings of MAX77686 multi-function device.
+More information can be found in bindings/mfd/max77686.txt file.
+
+The MAX77686 contains three 32.768khz clock outputs that can be controlled
+(gated/ungated) over I2C.
+
+Following properties should be presend in main device node of the MFD chip.
+
+Required properties:
+- #clock-cells: simple one-cell clock specifier format is used, where the
+  only cell is used as an index of the clock inside the provider. Following
+  indices are allowed:
+    - 0: 32khz_ap clock,
+    - 1: 32khz_cp clock,
+    - 2: 32khz_pmic clock.
+
+Example: Node of the MFD chip
+
+	max77686: max77686 at 09 {
+		compatible = "maxim,max77686";
+		interrupt-parent = <&wakeup_eint>;
+		interrupts = <26 0>;
+		reg = <0x09>;
+		#clock-cells = <1>;
+
+		/* ... */
+	};
+
+Example: Clock consumer node
+
+	foo at 0 {
+		compatible = "bar,foo";
+		/* ... */
+		clock-names = "my-clock";
+		clocks = <&max77686 2>;
+	};
Documentation/devicetree/bindings/mfd/max77686.txt +3 -0
index c6a3469..678f3cf 100644
@@ -7,6 +7,9 @@ different i2c slave address,presently for which we are statically creating i2c
 client while probing.This document describes the binding for mfd device and
 PMIC submodule.
 
+Binding for the built-in 32k clock generator block is defined separately
+in bindings/clk/maxim,max77686.txt file.
+
 Required properties:
 - compatible : Must be "maxim,max77686";
 - reg : Specifies the i2c slave address of PMIC block.
drivers/clk/clk-max77686.c +24 -0
index 3cf38dc..3d7e8dd 100644
@@ -169,6 +169,26 @@ static int max77686_clk_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, clocks);
 
+	if (iodev->dev->of_node) {
+		struct clk_onecell_data *of_data;
+
+		of_data = devm_kzalloc(&pdev->dev,
+					sizeof(*of_data), GFP_KERNEL);
+		if (!of_data) {
+			ret = -ENOMEM;
+			goto err_clocks;
+		}
+
+		of_data->clks = clocks;
+		of_data->clk_num = MAX77686_CLKS_NUM;
+		ret = of_clk_add_provider(iodev->dev->of_node,
+					of_clk_src_onecell_get, of_data);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to register OF clock provider\n");
+			goto err_clocks;
+		}
+	}
+
 	return 0;
 
 err_clocks:
@@ -182,9 +202,13 @@ err_clocks:
 
 static int max77686_clk_remove(struct platform_device *pdev)
 {
+	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct clk **clocks = platform_get_drvdata(pdev);
 	int i;
 
+	if (iodev->dev->of_node)
+		of_clk_del_provider(iodev->dev->of_node);
+
 	for (i = 0; i < MAX77686_CLKS_NUM; i++) {
 		struct clk_hw *hw = __clk_get_hw(clocks[i]);
 		struct max77686_clk *max77686 = to_max77686_clk(hw);
Signature · 1 line
1.8.4.3
#8 lore raw

PATCH v2 8/8 ARM: dts: Add clock provider specific properties to max77686 node

In reply to Tomasz Figa

This patch adds a label and #clock-cells property to device node of max77686 PMIC to allow using it as a clock provider.

---
 arch/arm/boot/dts/cros5250-common.dtsi   | 1 +
 arch/arm/boot/dts/exynos4412-odroidx.dts | 1 +
 arch/arm/boot/dts/exynos4412-trats2.dts  | 1 +
 3 files changed, 3 insertions(+)
arch/arm/boot/dts/cros5250-common.dtsi +1 -0
index 42cef0a..a9dc776 100644
@@ -48,6 +48,7 @@
 			pinctrl-0 = <&max77686_irq>;
 			wakeup-source;
 			reg = <0x09>;
+			#clock-cells = <1>;
 
 			voltage-regulators {
 				ldo1_reg: LDO1 {
arch/arm/boot/dts/exynos4412-odroidx.dts +1 -0
index 46c678e..8d337cc 100644
@@ -119,6 +119,7 @@
 		max77686: pmic at 09 {
 			compatible = "maxim,max77686";
 			reg = <0x09>;
+			#clock-cells = <1>;
 
 			voltage-regulators {
 				ldo1_reg: LDO1 {
arch/arm/boot/dts/exynos4412-trats2.dts +1 -0
index fb7b9ae..c21a8b9 100644
@@ -139,6 +139,7 @@
 			interrupt-parent = <&gpx0>;
 			interrupts = <7 0>;
 			reg = <0x09>;
+			#clock-cells = <1>;
 
 			voltage-regulators {
 				ldo1_reg: ldo1 {
Signature · 1 line
1.8.4.3
#9 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

In reply to Tomasz Figa
Quoted 24 lines · On Thursday 12 of December 2013 17:07:13 Tomasz Figa wrote:

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Tested on Exynos4412-based Trats2 board, with additional patches adding driver being a consumer of one of max77686 clocks.

Changes since v1:
- rebased on top of current clk-next (no functional changes)

Tomasz Figa (8):
clk: max77686: Correct callback used for checking clock status
clk: max77686: Provide .recalc_rate() operation
clk: max77686: Refactor successful exit of probe function
clk: max77686: Make max77686_clk_register() return struct clk *
clk: max77686: Fix clean-up in error and remove paths
clk: max77686: Refactor driver data handling
clk: max77686: Register OF clock provider
ARM: dts: Add clock provider specific properties to max77686 node

Quoted 8 lines
 .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++
 Documentation/devicetree/bindings/mfd/max77686.txt |  3 +
 arch/arm/boot/dts/cros5250-common.dtsi             |  1 +
 arch/arm/boot/dts/exynos4412-odroidx.dts           |  1 +
 arch/arm/boot/dts/exynos4412-trats2.dts            |  1 +
 drivers/clk/clk-max77686.c                         | 97 +++++++++++++---------
 6 files changed, 104 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/maxim,max77686.txt

Ping.

Best regards,
Tomasz

#10 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

In reply to Tomasz Figa

Quoting Tomasz Figa (2013-12-12 08:07:13)

Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Quoted 29 lines

Tested on Exynos4412-based Trats2 board, with additional patches adding driver being a consumer of one of max77686 clocks.

Changes since v1:
- rebased on top of current clk-next (no functional changes)

Tomasz Figa (8):
clk: max77686: Correct callback used for checking clock status
clk: max77686: Provide .recalc_rate() operation
clk: max77686: Refactor successful exit of probe function
clk: max77686: Make max77686_clk_register() return struct clk *
clk: max77686: Fix clean-up in error and remove paths
clk: max77686: Refactor driver data handling
clk: max77686: Register OF clock provider
ARM: dts: Add clock provider specific properties to max77686 node

 .../devicetree/bindings/clock/maxim,max77686.txt   | 38 +++++++++
 Documentation/devicetree/bindings/mfd/max77686.txt |  3 +
 arch/arm/boot/dts/cros5250-common.dtsi             |  1 +
 arch/arm/boot/dts/exynos4412-odroidx.dts           |  1 +
 arch/arm/boot/dts/exynos4412-trats2.dts            |  1 +
 drivers/clk/clk-max77686.c                         | 97 +++++++++++++---------
 6 files changed, 104 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/maxim,max77686.txt
Signature · 1 line
1.8.4.3
#11 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoted 10 lines · On Tuesday 31 of December 2013 11:42:15 Mike Turquette wrote:

Quoting Tomasz Figa (2013-12-12 08:07:13)

Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Hmm... Shouldn't this series go through the clk tree directly? If not, who should be taking it?

Best regards,
Tomasz

#12 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Quoted 4 lines
Quoted 1 line

Hmm... Shouldn't this series go through the clk tree directly? If not, who should be taking it?

I'm not quite sure why this patch-set was sent to me in the first place, but I guess that's by the by.

Mike is probably waiting for the ARM and DT patches to be Acked, applied.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#13 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

In reply to Tomasz Figa

Quoting Tomasz Figa (2014-01-02 07:22:12)

Quoted 10 lines · On Tuesday 31 of December 2013 11:42:15 Mike Turquette wrote:
Quoted 9 lines

Quoting Tomasz Figa (2013-12-12 08:07:13)

Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Quoted 4 lines
Quoted 1 line

Hmm... Shouldn't this series go through the clk tree directly? If not, who should be taking it?

Yes it should, but I guessed it would come in a pull request. Maybe Ack is the wrong thing? I can just say "looks good to me me" or something else.

Regards,
Mike

Quoted 4 lines

Best regards,
Tomasz

#14 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoted 11 lines · On Wednesday 08 of January 2014 08:31:38 Mike Turquette wrote:

Quoting Tomasz Figa (2014-01-02 07:22:12)

Quoted 10 lines · On Tuesday 31 of December 2013 11:42:15 Mike Turquette wrote:
Quoted 9 lines

Quoting Tomasz Figa (2013-12-12 08:07:13)

Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Hmm... Shouldn't this series go through the clk tree directly? If not, who should be taking it?

Yes it should, but I guessed it would come in a pull request. Maybe Ack is the wrong thing? I can just say "looks good to me me" or something else.

Nah, I think Ack is completely right, but still this series is quite standalone, e.g. it does not really belong to samsung-clk nor any other clock subtree, so I thought you would be applying them directly.

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Best regards,
Tomasz

#15 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoting Tomasz Figa (2014-01-08 08:44:29)

Quoted 17 lines · On Wednesday 08 of January 2014 08:31:38 Mike Turquette wrote:
Quoted 16 lines

Quoting Tomasz Figa (2014-01-02 07:22:12)

Quoted 14 lines · On Tuesday 31 of December 2013 11:42:15 Mike Turquette wrote:
Quoted 10 lines

Quoting Tomasz Figa (2013-12-12 08:07:13)

Quoted 7 lines

This series intends to improve clock provider impementation of max77686 PMIC driver. First two patches are simple fixes of current implementation to allow correct control of provided clocks. Further patches refactor the driver to make the code cleaner and prepare for further patches. Then last two patches add registration of OF clock provider and adjust device tree nodes of boards containing max77686 chip to contain properties required by clock provider.

Hmm... Shouldn't this series go through the clk tree directly? If not, who should be taking it?

Quoted 7 lines
Quoted 3 lines

Yes it should, but I guessed it would come in a pull request. Maybe Ack is the wrong thing? I can just say "looks good to me me" or something else.

Nah, I think Ack is completely right, but still this series is quite standalone, e.g. it does not really belong to samsung-clk nor any other clock subtree, so I thought you would be applying them directly.

True, this isn't really a samsung clock pull request. We can just attribute this one to ironing out a few bugs in our workflow ;-)

Quoted 4 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

That is fine, and it would also be fine to state in the coverletter that you do NOT plan to send a pull request. That removes all ambiguity from my side and lets me know to pick the patches manually. Verbosity is always good with these sorts of things.

Regards,
Mike

Quoted 4 lines

Best regards,
Tomasz

#16 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

In reply to Tomasz Figa
Quoted 3 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Me? I haven't applied any clk patches.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#17 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoting Lee Jones (2014-01-08 09:15:44)

Quoted 5 lines
Quoted 3 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Me? I haven't applied any clk patches.

I have taken patches 1-7 into clk-next just now. I can take patch #8 as well but it would probably be better to send it through arm-soc?

Regards,
Mike

Quoted 6 lines
Signature · 5 lines
-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#18 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoted 8 lines · On Wednesday 08 of January 2014 09:59:39 Mike Turquette wrote:

Quoting Lee Jones (2014-01-08 09:15:44)

Quoted 5 lines
Quoted 3 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Me? I haven't applied any clk patches.

I have taken patches 1-7 into clk-next just now.

Thanks.

Quoted 2 lines

I can take patch #8 as
well but it would probably be better to send it through arm-soc?

Hmm, patch 8 is trivial enough to not cause any merge conflicts, but since it's unlikely that any users depending on introduced changes will show up for 3.14 it shouldn't really matter which tree it goes through...

Best regards,
Tomasz

#19 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoting Tomasz Figa (2014-01-08 10:04:57)

Quoted 10 lines · On Wednesday 08 of January 2014 09:59:39 Mike Turquette wrote:
Quoted 8 lines

Quoting Lee Jones (2014-01-08 09:15:44)

Quoted 5 lines
Quoted 3 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Me? I haven't applied any clk patches.

I have taken patches 1-7 into clk-next just now.

Quoted 8 lines

Thanks.

Quoted 2 lines

I can take patch #8 as
well but it would probably be better to send it through arm-soc?

Hmm, patch 8 is trivial enough to not cause any merge conflicts, but since it's unlikely that any users depending on introduced changes will show up for 3.14 it shouldn't really matter which tree it goes through...

In the absence of any strong opinion on the topic I have taken patch #8 into clk-next for 3.14.

Regards,
Mike

Quoted 4 lines

Best regards,
Tomasz

#20 lore raw

v2 cover clk: max77686: Clock provider implementation fixes

Quoted 16 lines · On Wednesday 08 of January 2014 13:54:32 Mike Turquette wrote:

Quoting Tomasz Figa (2014-01-08 10:04:57)

Quoted 15 lines · On Wednesday 08 of January 2014 09:59:39 Mike Turquette wrote:
Quoted 8 lines

Quoting Lee Jones (2014-01-08 09:15:44)

Quoted 5 lines
Quoted 3 lines

I believe Lee has already applied this one, but in future, to avoid such confusion, if it's more convenient for you, I can send you any clock series as pull requests.

Me? I haven't applied any clk patches.

I have taken patches 1-7 into clk-next just now.

Thanks.

Quoted 2 lines

I can take patch #8 as
well but it would probably be better to send it through arm-soc?

Quoted 6 lines
Quoted 3 lines

Hmm, patch 8 is trivial enough to not cause any merge conflicts, but since it's unlikely that any users depending on introduced changes will show up for 3.14 it shouldn't really matter which tree it goes through...

In the absence of any strong opinion on the topic I have taken patch #8 into clk-next for 3.14.

Great, thanks.

Best regards,
Tomasz