[PATCH v2 0/3] refactoring on the s2mps11 device driver

[PATCH v2 0/3] refactoring on the s2mps11 device driver

[Andi Shyti]

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.

Compared to the first version there is a patch more:

 - 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.

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

Thanks,
Andi

Andi Shyti (3):
  clk: s2mps11: allocate only one structure for clock init
  clk: s2mps11: merge two for loop in one
  clk: s2mps11: remove redundant code

 drivers/clk/clk-s2mps11.c | 102 +++++++++++-----------------------------------
 1 file changed, 24 insertions(+), 78 deletions(-)

-- 
2.7.0.rc3

[PATCH v2 1/3] clk: s2mps11: allocate only one structure for clock init

[Andi Shyti]

The driver allocates three structures for three different clock
types. They are quite similar and in the clock init data they
differ only by the name. Only one of these structures is used,
while the others lie unused in the memory.

The clock's name, though, is not such a meaningful information
and by assigning the same name to the initial data we can avoid
over allocation. The common name chosen will be s2mps11,
coherently with the device driver name, instead of the clock
device.

Therefore, remove the structures associated to s2mps13 and
s2mps14 and use only the one referred to s2mps11 for all kind of
clocks.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/clk/clk-s2mps11.c | 56 ++++++-----------------------------------------
 1 file changed, 7 insertions(+), 49 deletions(-)

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index d266299..22d4df2 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -99,6 +99,7 @@ static struct clk_ops s2mps11_clk_ops = {
 	.recalc_rate	= s2mps11_clk_recalc_rate,
 };
 
+/* This s2mps11_clks_init tructure is common to s2mps11, s2mps13 and s2mps14 */
 static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = {
 	[S2MPS11_CLK_AP] = {
 		.name = "s2mps11_ap",
@@ -117,43 +118,11 @@ static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = {
 	},
 };
 
-static struct clk_init_data s2mps13_clks_init[S2MPS11_CLKS_NUM] = {
-	[S2MPS11_CLK_AP] = {
-		.name = "s2mps13_ap",
-		.ops = &s2mps11_clk_ops,
-		.flags = CLK_IS_ROOT,
-	},
-	[S2MPS11_CLK_CP] = {
-		.name = "s2mps13_cp",
-		.ops = &s2mps11_clk_ops,
-		.flags = CLK_IS_ROOT,
-	},
-	[S2MPS11_CLK_BT] = {
-		.name = "s2mps13_bt",
-		.ops = &s2mps11_clk_ops,
-		.flags = CLK_IS_ROOT,
-	},
-};
-
-static struct clk_init_data s2mps14_clks_init[S2MPS11_CLKS_NUM] = {
-	[S2MPS11_CLK_AP] = {
-		.name = "s2mps14_ap",
-		.ops = &s2mps11_clk_ops,
-		.flags = CLK_IS_ROOT,
-	},
-	[S2MPS11_CLK_BT] = {
-		.name = "s2mps14_bt",
-		.ops = &s2mps11_clk_ops,
-		.flags = CLK_IS_ROOT,
-	},
-};
-
 static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
 		struct clk_init_data *clks_init)
 {
 	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *clk_np;
-	int i;
 
 	if (!iodev->dev->of_node)
 		return ERR_PTR(-EINVAL);
@@ -164,13 +133,6 @@ static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
 		return ERR_PTR(-EINVAL);
 	}
 
-	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
-		if (!clks_init[i].name)
-			continue; /* Skip clocks not present in some devices */
-		of_property_read_string_index(clk_np, "clock-output-names", i,
-				&clks_init[i].name);
-	}
-
 	return clk_np;
 }
 
@@ -179,8 +141,8 @@ 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;
 	unsigned int s2mps11_reg;
-	struct clk_init_data *clks_init;
 	int i, ret = 0;
+	enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data;
 
 	s2mps11_clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
 				sizeof(*s2mps11_clk), GFP_KERNEL);
@@ -194,22 +156,18 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 	if (!clk_table)
 		return -ENOMEM;
 
-	switch(platform_get_device_id(pdev)->driver_data) {
+	switch (hwid) {
 	case S2MPS11X:
 		s2mps11_reg = S2MPS11_REG_RTC_CTRL;
-		clks_init = s2mps11_clks_init;
 		break;
 	case S2MPS13X:
 		s2mps11_reg = S2MPS13_REG_RTCCTRL;
-		clks_init = s2mps13_clks_init;
 		break;
 	case S2MPS14X:
 		s2mps11_reg = S2MPS14_REG_RTCCTRL;
-		clks_init = s2mps14_clks_init;
 		break;
 	case S5M8767X:
 		s2mps11_reg = S5M8767_REG_CTRL1;
-		clks_init = s2mps11_clks_init;
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device type\n");
@@ -217,15 +175,15 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 	}
 
 	/* Store clocks of_node in first element of s2mps11_clks array */
-	s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, clks_init);
+	s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, s2mps11_clks_init);
 	if (IS_ERR(s2mps11_clks->clk_np))
 		return PTR_ERR(s2mps11_clks->clk_np);
 
 	for (i = 0; i < S2MPS11_CLKS_NUM; i++, s2mps11_clk++) {
-		if (!clks_init[i].name)
+		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
 			continue; /* Skip clocks not present in some devices */
 		s2mps11_clk->iodev = iodev;
-		s2mps11_clk->hw.init = &clks_init[i];
+		s2mps11_clk->hw.init = &s2mps11_clks_init[i];
 		s2mps11_clk->mask = 1 << i;
 		s2mps11_clk->reg = s2mps11_reg;
 
@@ -248,7 +206,7 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 
 	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
 		/* Skip clocks not present on S2MPS14 */
-		if (!clks_init[i].name)
+		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
 			continue;
 		clk_table[i] = s2mps11_clks[i].clk;
 	}
-- 
2.7.0.rc3

Re: [PATCH v2 1/3] clk: s2mps11: allocate only one structure for clock init

[Krzysztof Kozlowski]

On 19.01.2016 17:52, Andi Shyti wrote:
> The driver allocates three structures for three different clock
> types. They are quite similar and in the clock init data they
> differ only by the name. Only one of these structures is used,
> while the others lie unused in the memory.
> 
> The clock's name, though, is not such a meaningful information
> and by assigning the same name to the initial data we can avoid
> over allocation. The common name chosen will be s2mps11,
> coherently with the device driver name, instead of the clock
> device.
> 
> Therefore, remove the structures associated to s2mps13 and
> s2mps14 and use only the one referred to s2mps11 for all kind of
> clocks.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/clk/clk-s2mps11.c | 56 ++++++-----------------------------------------
>  1 file changed, 7 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
> index d266299..22d4df2 100644
> --- a/drivers/clk/clk-s2mps11.c
> +++ b/drivers/clk/clk-s2mps11.c
> @@ -99,6 +99,7 @@ static struct clk_ops s2mps11_clk_ops = {
>  	.recalc_rate	= s2mps11_clk_recalc_rate,
>  };
>  
> +/* This s2mps11_clks_init tructure is common to s2mps11, s2mps13 and s2mps14 */
>  static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = {
>  	[S2MPS11_CLK_AP] = {
>  		.name = "s2mps11_ap",
> @@ -117,43 +118,11 @@ static struct clk_init_data s2mps11_clks_init[S2MPS11_CLKS_NUM] = {
>  	},
>  };
>  
> -static struct clk_init_data s2mps13_clks_init[S2MPS11_CLKS_NUM] = {
> -	[S2MPS11_CLK_AP] = {
> -		.name = "s2mps13_ap",
> -		.ops = &s2mps11_clk_ops,
> -		.flags = CLK_IS_ROOT,
> -	},
> -	[S2MPS11_CLK_CP] = {
> -		.name = "s2mps13_cp",
> -		.ops = &s2mps11_clk_ops,
> -		.flags = CLK_IS_ROOT,
> -	},
> -	[S2MPS11_CLK_BT] = {
> -		.name = "s2mps13_bt",
> -		.ops = &s2mps11_clk_ops,
> -		.flags = CLK_IS_ROOT,
> -	},
> -};
> -
> -static struct clk_init_data s2mps14_clks_init[S2MPS11_CLKS_NUM] = {
> -	[S2MPS11_CLK_AP] = {
> -		.name = "s2mps14_ap",
> -		.ops = &s2mps11_clk_ops,
> -		.flags = CLK_IS_ROOT,
> -	},
> -	[S2MPS11_CLK_BT] = {
> -		.name = "s2mps14_bt",
> -		.ops = &s2mps11_clk_ops,
> -		.flags = CLK_IS_ROOT,
> -	},
> -};
> -
>  static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
>  		struct clk_init_data *clks_init)
>  {
>  	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>  	struct device_node *clk_np;
> -	int i;
>  
>  	if (!iodev->dev->of_node)
>  		return ERR_PTR(-EINVAL);
> @@ -164,13 +133,6 @@ static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
> -		if (!clks_init[i].name)
> -			continue; /* Skip clocks not present in some devices */
> -		of_property_read_string_index(clk_np, "clock-output-names", i,
> -				&clks_init[i].name);
> -	}
> -

The commit description does not mention anything about removal of
"clock-output-names". Why you are removing this?

Best regards,
Krzysztof

>  	return clk_np;
>  }
>  
> @@ -179,8 +141,8 @@ 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;
>  	unsigned int s2mps11_reg;
> -	struct clk_init_data *clks_init;
>  	int i, ret = 0;
> +	enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data;
>  
>  	s2mps11_clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
>  				sizeof(*s2mps11_clk), GFP_KERNEL);
> @@ -194,22 +156,18 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  	if (!clk_table)
>  		return -ENOMEM;
>  
> -	switch(platform_get_device_id(pdev)->driver_data) {
> +	switch (hwid) {
>  	case S2MPS11X:
>  		s2mps11_reg = S2MPS11_REG_RTC_CTRL;
> -		clks_init = s2mps11_clks_init;
>  		break;
>  	case S2MPS13X:
>  		s2mps11_reg = S2MPS13_REG_RTCCTRL;
> -		clks_init = s2mps13_clks_init;
>  		break;
>  	case S2MPS14X:
>  		s2mps11_reg = S2MPS14_REG_RTCCTRL;
> -		clks_init = s2mps14_clks_init;
>  		break;
>  	case S5M8767X:
>  		s2mps11_reg = S5M8767_REG_CTRL1;
> -		clks_init = s2mps11_clks_init;
>  		break;
>  	default:
>  		dev_err(&pdev->dev, "Invalid device type\n");
> @@ -217,15 +175,15 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Store clocks of_node in first element of s2mps11_clks array */
> -	s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, clks_init);
> +	s2mps11_clks->clk_np = s2mps11_clk_parse_dt(pdev, s2mps11_clks_init);
>  	if (IS_ERR(s2mps11_clks->clk_np))
>  		return PTR_ERR(s2mps11_clks->clk_np);
>  
>  	for (i = 0; i < S2MPS11_CLKS_NUM; i++, s2mps11_clk++) {
> -		if (!clks_init[i].name)
> +		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
>  			continue; /* Skip clocks not present in some devices */
>  		s2mps11_clk->iodev = iodev;
> -		s2mps11_clk->hw.init = &clks_init[i];
> +		s2mps11_clk->hw.init = &s2mps11_clks_init[i];
>  		s2mps11_clk->mask = 1 << i;
>  		s2mps11_clk->reg = s2mps11_reg;
>  
> @@ -248,7 +206,7 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
>  		/* Skip clocks not present on S2MPS14 */
> -		if (!clks_init[i].name)
> +		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
>  			continue;
>  		clk_table[i] = s2mps11_clks[i].clk;
>  	}
> 

[PATCH v2 2/3] clk: s2mps11: merge two for loop in one

[Andi Shyti]

the driver already loops once, there is no reason to loop again
for a different purpose. Merge the second loop into the first.

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

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index 22d4df2..cae92fc 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -202,12 +202,6 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 			ret = -ENOMEM;
 			goto err_reg;
 		}
-	}
-
-	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
-		/* Skip clocks not present on S2MPS14 */
-		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
-			continue;
 		clk_table[i] = s2mps11_clks[i].clk;
 	}
 
-- 
2.7.0.rc3

Re: [PATCH v2 2/3] clk: s2mps11: merge two for loop in one

[Krzysztof Kozlowski]

On 19.01.2016 17:52, Andi Shyti wrote:
> the driver already loops once, there is no reason to loop again

/s/the/The/

> for a different purpose. Merge the second loop into the first.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/clk/clk-s2mps11.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
> index 22d4df2..cae92fc 100644
> --- a/drivers/clk/clk-s2mps11.c
> +++ b/drivers/clk/clk-s2mps11.c
> @@ -202,12 +202,6 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  			ret = -ENOMEM;
>  			goto err_reg;
>  		}
> -	}
> -
> -	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
> -		/* Skip clocks not present on S2MPS14 */
> -		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)

You changed this line in patch 1/3 - meaningless. This is a indication
that this patch should go before 1st.

BR,
Krzysztof

> -			continue;
>  		clk_table[i] = s2mps11_clks[i].clk;
>  	}
>  
> 

[PATCH v2 3/3] clk: s2mps11: remove redundant code

[Andi Shyti]

This patch performs some code removal as it uses some variables
for temporary use. Three changes:

* In the probe function the s2mps11_clk pointer is used only to
iterate through the s2mps11_clks. The naming itself brings
confusion and the readability is not improved.

* the s2mps11_name() define resolves the name of the a given
device. As it is used only once, we can expand it where it's used
and remove the defininition.

* The **clk_table variable is declared as a static variable and
used to reference some allocated memory, it's then assigned to
the clk_data.clks. This cycle can be skipped and the declaration
avoided.

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

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index cae92fc..09344bb 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -28,9 +28,6 @@
 #include <linux/mfd/samsung/s5m8767.h>
 #include <linux/mfd/samsung/core.h>
 
-#define s2mps11_name(a) (a->hw.init->name)
-
-static struct clk **clk_table;
 static struct clk_onecell_data clk_data;
 
 enum {
@@ -139,21 +136,19 @@ static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
 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 s2mps11_clk *s2mps11_clks;
 	unsigned int s2mps11_reg;
 	int i, ret = 0;
 	enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data;
 
 	s2mps11_clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
-				sizeof(*s2mps11_clk), GFP_KERNEL);
+				sizeof(*s2mps11_clks), GFP_KERNEL);
 	if (!s2mps11_clks)
 		return -ENOMEM;
 
-	s2mps11_clk = s2mps11_clks;
-
-	clk_table = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
+	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) {
@@ -179,33 +174,32 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(s2mps11_clks->clk_np))
 		return PTR_ERR(s2mps11_clks->clk_np);
 
-	for (i = 0; i < S2MPS11_CLKS_NUM; i++, s2mps11_clk++) {
+	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
 		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
 			continue; /* Skip clocks not present in some devices */
-		s2mps11_clk->iodev = iodev;
-		s2mps11_clk->hw.init = &s2mps11_clks_init[i];
-		s2mps11_clk->mask = 1 << i;
-		s2mps11_clk->reg = s2mps11_reg;
-
-		s2mps11_clk->clk = devm_clk_register(&pdev->dev,
-							&s2mps11_clk->hw);
-		if (IS_ERR(s2mps11_clk->clk)) {
+		s2mps11_clks[i].iodev = iodev;
+		s2mps11_clks[i].hw.init = &s2mps11_clks_init[i];
+		s2mps11_clks[i].mask = 1 << i;
+		s2mps11_clks[i].reg = s2mps11_reg;
+
+		s2mps11_clks[i].clk = devm_clk_register(&pdev->dev,
+							&s2mps11_clks[i].hw);
+		if (IS_ERR(s2mps11_clks[i].clk)) {
 			dev_err(&pdev->dev, "Fail to register : %s\n",
-						s2mps11_name(s2mps11_clk));
-			ret = PTR_ERR(s2mps11_clk->clk);
+						s2mps11_clks_init[i].name);
+			ret = PTR_ERR(s2mps11_clks[i].clk);
 			goto err_reg;
 		}
 
-		s2mps11_clk->lookup = clkdev_create(s2mps11_clk->clk,
-					s2mps11_name(s2mps11_clk), NULL);
-		if (!s2mps11_clk->lookup) {
+		s2mps11_clks[i].lookup = clkdev_create(s2mps11_clks[i].clk,
+					s2mps11_clks_init[i].name, NULL);
+		if (!s2mps11_clks[i].lookup) {
 			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;
 	of_clk_add_provider(s2mps11_clks->clk_np, of_clk_src_onecell_get,
 			&clk_data);
-- 
2.7.0.rc3

Re: [PATCH v2 3/3] clk: s2mps11: remove redundant code

[Krzysztof Kozlowski]

On 19.01.2016 17:52, Andi Shyti wrote:
> This patch performs some code removal as it uses some variables
> for temporary use. Three changes:
> 
> * In the probe function the s2mps11_clk pointer is used only to
> iterate through the s2mps11_clks. The naming itself brings
> confusion and the readability is not improved.
> 
> * the s2mps11_name() define resolves the name of the a given
> device. As it is used only once, we can expand it where it's used
> and remove the defininition.
> 
> * The **clk_table variable is declared as a static variable and
> used to reference some allocated memory, it's then assigned to
> the clk_data.clks. This cycle can be skipped and the declaration
> avoided.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/clk/clk-s2mps11.c | 44 +++++++++++++++++++-------------------------
>  1 file changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
> index cae92fc..09344bb 100644
> --- a/drivers/clk/clk-s2mps11.c
> +++ b/drivers/clk/clk-s2mps11.c
> @@ -28,9 +28,6 @@
>  #include <linux/mfd/samsung/s5m8767.h>
>  #include <linux/mfd/samsung/core.h>
>  
> -#define s2mps11_name(a) (a->hw.init->name)
> -
> -static struct clk **clk_table;
>  static struct clk_onecell_data clk_data;

We spoke about this. You don't need both of them. Remove them.

Best regards,
Krzysztof

>  
>  enum {
> @@ -139,21 +136,19 @@ static struct device_node *s2mps11_clk_parse_dt(struct platform_device *pdev,
>  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 s2mps11_clk *s2mps11_clks;
>  	unsigned int s2mps11_reg;
>  	int i, ret = 0;
>  	enum sec_device_type hwid = platform_get_device_id(pdev)->driver_data;
>  
>  	s2mps11_clks = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
> -				sizeof(*s2mps11_clk), GFP_KERNEL);
> +				sizeof(*s2mps11_clks), GFP_KERNEL);
>  	if (!s2mps11_clks)
>  		return -ENOMEM;
>  
> -	s2mps11_clk = s2mps11_clks;
> -
> -	clk_table = devm_kcalloc(&pdev->dev, S2MPS11_CLKS_NUM,
> +	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) {
> @@ -179,33 +174,32 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  	if (IS_ERR(s2mps11_clks->clk_np))
>  		return PTR_ERR(s2mps11_clks->clk_np);
>  
> -	for (i = 0; i < S2MPS11_CLKS_NUM; i++, s2mps11_clk++) {
> +	for (i = 0; i < S2MPS11_CLKS_NUM; i++) {
>  		if (i == S2MPS11_CLK_CP && hwid == S2MPS14X)
>  			continue; /* Skip clocks not present in some devices */
> -		s2mps11_clk->iodev = iodev;
> -		s2mps11_clk->hw.init = &s2mps11_clks_init[i];
> -		s2mps11_clk->mask = 1 << i;
> -		s2mps11_clk->reg = s2mps11_reg;
> -
> -		s2mps11_clk->clk = devm_clk_register(&pdev->dev,
> -							&s2mps11_clk->hw);
> -		if (IS_ERR(s2mps11_clk->clk)) {
> +		s2mps11_clks[i].iodev = iodev;
> +		s2mps11_clks[i].hw.init = &s2mps11_clks_init[i];
> +		s2mps11_clks[i].mask = 1 << i;
> +		s2mps11_clks[i].reg = s2mps11_reg;
> +
> +		s2mps11_clks[i].clk = devm_clk_register(&pdev->dev,
> +							&s2mps11_clks[i].hw);
> +		if (IS_ERR(s2mps11_clks[i].clk)) {
>  			dev_err(&pdev->dev, "Fail to register : %s\n",
> -						s2mps11_name(s2mps11_clk));
> -			ret = PTR_ERR(s2mps11_clk->clk);
> +						s2mps11_clks_init[i].name);
> +			ret = PTR_ERR(s2mps11_clks[i].clk);
>  			goto err_reg;
>  		}
>  
> -		s2mps11_clk->lookup = clkdev_create(s2mps11_clk->clk,
> -					s2mps11_name(s2mps11_clk), NULL);
> -		if (!s2mps11_clk->lookup) {
> +		s2mps11_clks[i].lookup = clkdev_create(s2mps11_clks[i].clk,
> +					s2mps11_clks_init[i].name, NULL);
> +		if (!s2mps11_clks[i].lookup) {
>  			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;
>  	of_clk_add_provider(s2mps11_clks->clk_np, of_clk_src_onecell_get,
>  			&clk_data);
>