Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1313072 > unrolled thread

[PATCH v3 0/4] s2mps11 clock driver refactoring

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/4] s2mps11 clock driver refactoring Andi Shyti <andi.shyti@samsung.com> - 2016-01-20 11:20 +0100
    [PATCH v3 3/4] clk: s2mps11: remove redundant static variables  declaration Andi Shyti <andi.shyti@samsung.com> - 2016-01-20 11:20 +0100
      Re: [PATCH v3 3/4] clk: s2mps11: remove redundant static variables  declaration Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-01-21 01:30 +0100
    Re: [PATCH v3 0/4] s2mps11 clock driver refactoring Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-01-21 01:20 +0100

#1313072 — [PATCH v3 0/4] s2mps11 clock driver refactoring

Hi,

This patchset contains some code refactoring on the s2mps11 clock
device driver. The main goal is to remove some dead code and
improve its readability.

At any iteration the patchset is getting a patch more, so that
this time there are four patches :)

Thanks to Krzysztof for his review, the changset is coming from
his suggestions:

V1 -> V2
 - the order of the patches has changed
 - the second patch has been added and it merges a for loop
   inside
   a previous one
 - the 3rd patch (which in the first version was the 1st)
   contains some more redundant variables removal.

V2 -> V3
 - The "merge two for loops in one" has been moved as first patch
 - removed the global variable clk_data

The patches have been applied on next-20160120 and tested on
Odroid Xu4.

Andi Shyti (4):
  clk: s2mps11: merge two for loops in one
  clk: s2mps11: allocate only one structure for clock init
  clk: s2mps11: remove redundant static variables declaration
  clk: s2mps11: remove redundant code

 drivers/clk/clk-s2mps11.c | 108 +++++++++++++---------------------------------
 1 file changed, 31 insertions(+), 77 deletions(-)

-- 
2.7.0.rc3

[toc] | [next] | [standalone]

#1313078 — [PATCH v3 3/4] clk: s2mps11: remove redundant static variables declaration

The clk_table and clk_data are declared static. The clk_table
contains the three clock data stractures belonging to the s2mps11
driver. In the probe function it gets stored into clk_data.

Remove clk_table and refer directly to clk_data.

clk_data, itself, is also declared static. Declare locally it
and allocate it inside the probe function, as it is not used
anywhere else.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/clk/clk-s2mps11.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index 3ff2162..fac0e20 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -30,9 +30,6 @@
 
 #define s2mps11_name(a) (a->hw.init->name)
 
-static struct clk **clk_table;
-static struct clk_onecell_data clk_data;
-
 enum {
 	S2MPS11_CLK_AP = 0,
 	S2MPS11_CLK_CP,
@@ -145,6 +142,7 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 {
 	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct s2mps11_clk *s2mps11_clks, *s2mps11_clk;
+	struct clk_onecell_data *clk_data;
 	unsigned int s2mps11_reg;
 	int i, ret = 0;
 	enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data;
@@ -156,9 +154,13 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 
 	s2mps11_clk = s2mps11_clks;
 
-	clk_table = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
+	clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
 				sizeof(struct clk *), GFP_KERNEL);
-	if (!clk_table)
+	if (!clk_data->clks)
 		return -ENOMEM;
 
 	switch (hwid) {
@@ -207,13 +209,12 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 			ret = -ENOMEM;
 			goto err_reg;
 		}
-		clk_table[i] = s2mps11_clks[i].clk;
+		clk_data->clks[i] = s2mps11_clks[i].clk;
 	}
 
-	clk_data.clks = clk_table;
-	clk_data.clk_num = S2MPS11_CLKS_NUM;
+	clk_data->clk_num = S2MPS11_CLKS_NUM;
 	of_clk_add_provider(s2mps11_clks->clk_np, of_clk_src_onecell_get,
-			&clk_data);
+			clk_data);
 
 	platform_set_drvdata(pdev, s2mps11_clks);
 
-- 
2.7.0.rc3

[toc] | [prev] | [next] | [standalone]

#1313721 — Re: [PATCH v3 3/4] clk: s2mps11: remove redundant static variables declaration

On 20.01.2016 19:14, Andi Shyti wrote:
> The clk_table and clk_data are declared static. The clk_table
> contains the three clock data stractures belonging to the s2mps11

s/stractures/structures/

Rest looks good:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[toc] | [prev] | [next] | [standalone]

#1313720

On 20.01.2016 19:14, Andi Shyti wrote:
> Hi,
> 
> This patchset contains some code refactoring on the s2mps11 clock
> device driver. The main goal is to remove some dead code and
> improve its readability.
> 
> At any iteration the patchset is getting a patch more, so that
> this time there are four patches :)

Which means that if I have more comments now, you will send a v5 with 5
patches inside? Oh...

BR,
Krzysztof

> 
> Thanks to Krzysztof for his review, the changset is coming from
> his suggestions:
> 
> V1 -> V2
>  - the order of the patches has changed
>  - the second patch has been added and it merges a for loop
>    inside
>    a previous one
>  - the 3rd patch (which in the first version was the 1st)
>    contains some more redundant variables removal.
> 
> V2 -> V3
>  - The "merge two for loops in one" has been moved as first patch
>  - removed the global variable clk_data
> 
> The patches have been applied on next-20160120 and tested on
> Odroid Xu4.
> 
> Andi Shyti (4):
>   clk: s2mps11: merge two for loops in one
>   clk: s2mps11: allocate only one structure for clock init
>   clk: s2mps11: remove redundant static variables declaration
>   clk: s2mps11: remove redundant code
> 
>  drivers/clk/clk-s2mps11.c | 108 +++++++++++++---------------------------------
>  1 file changed, 31 insertions(+), 77 deletions(-)
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web