clk: sunxi: error checking on clock setup

PATCH Superseded Andre Przywara 3 participants 9 messages Feb 12, 2016 – Feb 16, 2016

Subsystems (3)

Common clk Arm/Allwinner sunXi SoC support Arm/Allwinner SoC Clock Support

lore

Share & tools

lore

Andre Przywara

author

2016-02-12 15:11 UTC #0 lore raw

cover clk: sunxi: error checking on clock setup

Since I promised to do this some days ago:

Setting up the clocks properly is quite critical to the system's operation, but currently our error handling is not very verbose. This series adds error handling and reporting to the sunxi clocks, so that any errors are correctly detected and reported. Also previous actions are rolled back in case something went wrong.
This proves to be helpful in debugging clock tree issues, especially when adding support for new SoCs.
I tested this on a BananaPi by deliberately misspelling "clock-output-names". The resulting kernel crash in sun4i_timer_interrupt is totally misleading, but now there is a line in the dmesg before saying:
=======
sunxi_divider_clk_setup: could not read clock-output-names for "apb0"
=======

This applies on top of Maxime's sunxi/for-next branch.

Please have a look (and apply, if you like it).

Cheers,
Andre.

Andre Przywara (3):
clk: sunxi: improve mux_clk error handling and reporting
clk: sunxi: improve divider_clk error handling and reporting
clk: sunxi: Improve divs_clk error handling and reporting

 drivers/clk/sunxi/clk-sunxi.c | 68 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 12 deletions(-)

Signature · 1 line

2.6.4

Andre Przywara

author

2016-02-12 15:11 UTC #1 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.

Signed-off-by Andre Przywara <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

drivers/clk/sunxi/clk-sunxi.c +13 -6

index 49ce283..361f396 100644
@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 	int i;
 
 	reg = of_iomap(node, 0);
+	if (!reg) {
+		pr_err("Could not map registers for mux-clk: %s\n", node->name);
+		return NULL;
+	}
 
 	i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
 	if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-		pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-			__func__, clk_name);
+		pr_err("%s: could not read clock-output-names for \"%s\"\n",
+		       __func__, clk_name);
 		goto out_unmap;
 	}

@@ -704,14 +708,17 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
 			       0, &clk_lock);
 
 	if (IS_ERR(clk)) {
-		pr_warn("%s: failed to register mux clock %s: %ld\n", __func__,
-			clk_name, PTR_ERR(clk));
+		pr_err("%s: failed to register mux clock %s: %ld\n", __func__,
+		       clk_name, PTR_ERR(clk));
 		goto out_unmap;
 	}
 
-	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (!of_clk_add_provider(node, of_clk_src_simple_get, clk))
+		return clk;
+
+	pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);
 
-	return clk;
+	clk_unregister_divider(clk);
 
 out_unmap:
 	iounmap(reg);

Signature · 1 line

2.6.4

Chen-Yu Tsai

2016-02-13 02:44 UTC #2 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

Hi,

Quoted 14 lines · Andre Przywara

We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.
Signed-off-by Andre Przywara <[email protected]>
---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/clk/sunxi/clk-sunxi.c +0 -0
index 49ce283..361f396 100644

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 694

Quoted 5 lines

drivers/clk/sunxi/clk-sunxi.c +2 -0

@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
        int i;
 
        reg = of_iomap(node, 0);
+       if (!reg) {
+               pr_err("Could not map registers for mux-clk: %s\n", node->name);

of_node_full_name(node->name) is better. node->name is almost always "clk", which is useless. (Unless someone goes through the dts files to replace all of them.)

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 701

Quoted 9 lines

drivers/clk/sunxi/clk-sunxi.c +4 -2

+               return NULL;
+       }
 
        i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
        if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-               pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-                       __func__, clk_name);
+               pr_err("%s: could not read clock-output-names for \"%s\"\n",
+                      __func__, clk_name);

Same here. clk_name defaults to node->name. If you could, please replace it.

Quoted 11 lines

drivers/clk/sunxi/clk-sunxi.c +2 -2

                goto out_unmap;
        }

@@ -704,14 +708,17 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
                               0, &clk_lock);
 
        if (IS_ERR(clk)) {
-               pr_warn("%s: failed to register mux clock %s: %ld\n", __func__,
-                       clk_name, PTR_ERR(clk));
+               pr_err("%s: failed to register mux clock %s: %ld\n", __func__,
+                      clk_name, PTR_ERR(clk));

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 714

Quoted 10 lines

drivers/clk/sunxi/clk-sunxi.c +4 -2

                goto out_unmap;
        }
 
-       of_clk_add_provider(node, of_clk_src_simple_get, clk);
+       if (!of_clk_add_provider(node, of_clk_src_simple_get, clk))
+               return clk;
+
+       pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);
 
-       return clk;

I'd have all the error path in ifs, and have the normal return path here for consistency.
But I guess this works as well.

Thanks!
ChenYu

Quoted 7 lines

drivers/clk/sunxi/clk-sunxi.c +1 -1

+       clk_unregister_divider(clk);
 
 out_unmap:
        iounmap(reg);
--

2.6.4

Andre Przywara

author

2016-02-16 09:45 UTC #3 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

Hi Chen-Yu,

Quoted 19 lines · Chen-Yu Tsai

Hi,
Quoted 16 lines · Andre Przywara
We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.
Signed-off-by Andre Przywara <[email protected]>
---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/clk/sunxi/clk-sunxi.c +0 -0
index 49ce283..361f396 100644
@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
        int i;
 

Quoted 7 lines

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 694
Quoted 3 lines
drivers/clk/sunxi/clk-sunxi.c +2 -0
        reg = of_iomap(node, 0);
+       if (!reg) {
+               pr_err("Could not map registers for mux-clk: %s\n", node->name);
of_node_full_name(node->name) is better. node->name is almost always "clk", which is useless. (Unless someone goes through the dts files to replace all of them.)

Good point. I both fixed the code here as well as amended the node names in the A64 DT.

Quoted 4 lines

Quoted 4 lines
drivers/clk/sunxi/clk-sunxi.c +2 -0
+               return NULL;
+       }
 

Quoted 8 lines

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 701
Quoted 6 lines
drivers/clk/sunxi/clk-sunxi.c +2 -2
        i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
        if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-               pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-                       __func__, clk_name);
+               pr_err("%s: could not read clock-output-names for \"%s\"\n",
+                      __func__, clk_name);
Same here. clk_name defaults to node->name. If you could, please replace it.

Really? Isn't clk_name directly from the DT clock-output-names strings here?

Quoted 20 lines

drivers/clk/sunxi/clk-sunxi.c +6 -3

                goto out_unmap;
        }

@@ -704,14 +708,17 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
                               0, &clk_lock);
 
        if (IS_ERR(clk)) {
-               pr_warn("%s: failed to register mux clock %s: %ld\n", __func__,
-                       clk_name, PTR_ERR(clk));
+               pr_err("%s: failed to register mux clock %s: %ld\n", __func__,
+                      clk_name, PTR_ERR(clk));
                goto out_unmap;
        }
 
-       of_clk_add_provider(node, of_clk_src_simple_get, clk);
+       if (!of_clk_add_provider(node, of_clk_src_simple_get, clk))
+               return clk;
+
+       pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);

Quoted 5 lines

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 714
Quoted 1 line
drivers/clk/sunxi/clk-sunxi.c +0 -1
-       return clk;
I'd have all the error path in ifs, and have the normal return path here for consistency.
But I guess this works as well.

Yeah, I wanted to skip that single goto ;-)
But I changed it now as you said and it indeed looks more reasonable.

I took the freedom of applying the comments to the other patches too.

Will send out a v2 in a minute.

Thanks for looking!
Andre.

Quoted 12 lines

Thanks!
ChenYu
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c +1 -1
+       clk_unregister_divider(clk);
 
 out_unmap:
        iounmap(reg);
--
2.6.4

Chen-Yu Tsai

2016-02-16 09:59 UTC #4 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

Quoted 44 lines · Andre Przywara

Hi Chen-Yu,
Quoted 19 lines · Chen-Yu Tsai
Hi,
Quoted 16 lines · Andre Przywara
We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.
Signed-off-by Andre Przywara <[email protected]>
---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/clk/sunxi/clk-sunxi.c +0 -0
index 49ce283..361f396 100644
@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
        int i;
 
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 694
Quoted 3 lines
drivers/clk/sunxi/clk-sunxi.c +2 -0
        reg = of_iomap(node, 0);
+       if (!reg) {
+               pr_err("Could not map registers for mux-clk: %s\n", node->name);
of_node_full_name(node->name) is better. node->name is almost always "clk", which is useless. (Unless someone goes through the dts files to replace all of them.)
Good point. I both fixed the code here as well as amended the node names in the A64 DT.
Quoted 11 lines
Quoted 10 lines
drivers/clk/sunxi/clk-sunxi.c +4 -2
+               return NULL;
+       }
 
        i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
        if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-               pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-                       __func__, clk_name);
+               pr_err("%s: could not read clock-output-names for \"%s\"\n",
+                      __func__, clk_name);

Quoted 3 lines

Quoted 1 line
Same here. clk_name defaults to node->name. If you could, please replace it.
Really? Isn't clk_name directly from the DT clock-output-names strings here?

It fills it from the DT clock-output-names in the of_property_read_string() call in the if here. If that failed, obviously something was wrong with the DT clock-output-names strings. :)

Quoted 27 lines

Quoted 21 lines

drivers/clk/sunxi/clk-sunxi.c +6 -3

                goto out_unmap;
        }

@@ -704,14 +708,17 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
                               0, &clk_lock);
 
        if (IS_ERR(clk)) {
-               pr_warn("%s: failed to register mux clock %s: %ld\n", __func__,
-                       clk_name, PTR_ERR(clk));
+               pr_err("%s: failed to register mux clock %s: %ld\n", __func__,
+                      clk_name, PTR_ERR(clk));
                goto out_unmap;
        }
 
-       of_clk_add_provider(node, of_clk_src_simple_get, clk);
+       if (!of_clk_add_provider(node, of_clk_src_simple_get, clk))
+               return clk;
+
+       pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);

Quoted 5 lines

drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 714
Quoted 1 line
drivers/clk/sunxi/clk-sunxi.c +0 -1
-       return clk;
I'd have all the error path in ifs, and have the normal return path here for consistency.
But I guess this works as well.

Quoted 4 lines

Yeah, I wanted to skip that single goto ;-)
But I changed it now as you said and it indeed looks more reasonable.
I took the freedom of applying the comments to the other patches too.

Thanks!
ChenYu

Quoted 18 lines

Will send out a v2 in a minute.
Thanks for looking!
Andre.
Quoted 12 lines
Thanks!
ChenYu
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c +1 -1
+       clk_unregister_divider(clk);
 
 out_unmap:
        iounmap(reg);
--
2.6.4

Andre Przywara

author

2016-02-16 10:04 UTC #5 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

Hi,

Quoted 45 lines · Chen-Yu Tsai

Quoted 44 lines · Andre Przywara
Hi Chen-Yu,
Quoted 19 lines · Chen-Yu Tsai
Hi,
Quoted 16 lines · Andre Przywara
We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.
Signed-off-by Andre Przywara <[email protected]>
---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/clk/sunxi/clk-sunxi.c +0 -0
index 49ce283..361f396 100644
@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
        int i;
 
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 694
Quoted 3 lines
drivers/clk/sunxi/clk-sunxi.c +2 -0
        reg = of_iomap(node, 0);
+       if (!reg) {
+               pr_err("Could not map registers for mux-clk: %s\n", node->name);
of_node_full_name(node->name) is better. node->name is almost always "clk", which is useless. (Unless someone goes through the dts files to replace all of them.)
Good point. I both fixed the code here as well as amended the node names in the A64 DT.
Quoted 11 lines
Quoted 10 lines
drivers/clk/sunxi/clk-sunxi.c +4 -2
+               return NULL;
+       }
 
        i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
        if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-               pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-                       __func__, clk_name);
+               pr_err("%s: could not read clock-output-names for \"%s\"\n",
+                      __func__, clk_name);

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Same here. clk_name defaults to node->name. If you could, please replace it.
Really? Isn't clk_name directly from the DT clock-output-names strings here?
It fills it from the DT clock-output-names in the of_property_read_string() call in the if here. If that failed, obviously something was wrong with the DT clock-output-names strings. :)

Of course! I shouldn't write stuff that early in the morning ...

Cheers,
Andre.

Quoted 54 lines

Quoted 27 lines
Quoted 21 lines
Quoted 21 lines
drivers/clk/sunxi/clk-sunxi.c +6 -3
                goto out_unmap;
        }
 
@@ -704,14 +708,17 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
                               0, &clk_lock);
 
        if (IS_ERR(clk)) {
-               pr_warn("%s: failed to register mux clock %s: %ld\n", __func__,
-                       clk_name, PTR_ERR(clk));
+               pr_err("%s: failed to register mux clock %s: %ld\n", __func__,
+                      clk_name, PTR_ERR(clk));
                goto out_unmap;
        }
 
-       of_clk_add_provider(node, of_clk_src_simple_get, clk);
+       if (!of_clk_add_provider(node, of_clk_src_simple_get, clk))
+               return clk;
+
+       pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);
 
Quoted 5 lines
drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 714
Quoted 1 line
drivers/clk/sunxi/clk-sunxi.c +0 -1
-       return clk;
I'd have all the error path in ifs, and have the normal return path here for consistency.
But I guess this works as well.
Quoted 4 lines
Yeah, I wanted to skip that single goto ;-)
But I changed it now as you said and it indeed looks more reasonable.
I took the freedom of applying the comments to the other patches too.
Thanks!
ChenYu
Quoted 18 lines
Will send out a v2 in a minute.
Thanks for looking!
Andre.
Quoted 12 lines
Thanks!
ChenYu
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c +1 -1
+       clk_unregister_divider(clk);
 
 out_unmap:
        iounmap(reg);
--
2.6.4

Maxime Ripard

2016-02-16 10:02 UTC #6 lore raw

PATCH 1/3 clk: sunxi: improve mux_clk error handling and reporting

In reply to Andre Przywara

Quoted 30 lines · Andre Przywara

Hi Chen-Yu,
Quoted 19 lines · Chen-Yu Tsai
Hi,
Quoted 16 lines · Andre Przywara
We now catch and report a failing ioremap, also a failure in the final step of the clock registration is now handled and reported. Also warnings are turned into errors.
Signed-off-by Andre Przywara <[email protected]>
---
 drivers/clk/sunxi/clk-sunxi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
drivers/clk/sunxi/clk-sunxi.c +0 -0
index 49ce283..361f396 100644
@@ -690,11 +690,15 @@ static struct clk * __init sunxi_mux_clk_setup(struct device_node *node,
        int i;
 
Quoted 7 lines
drivers/clk/sunxi/clk-sunxi.c static struct clk * __init sunxi_mux_clk_setup(struct device_node *node, · line 694
Quoted 3 lines
drivers/clk/sunxi/clk-sunxi.c +2 -0
        reg = of_iomap(node, 0);
+       if (!reg) {
+               pr_err("Could not map registers for mux-clk: %s\n", node->name);
of_node_full_name(node->name) is better. node->name is almost always "clk", which is useless. (Unless someone goes through the dts files to replace all of them.)

Quoted 2 lines

Good point. I both fixed the code here as well as amended the node names in the A64 DT.

I'm not sure what there is to amend in the DTSI. The node name is defined as the class or function of the device, so all clocks should be called clk@<something>.

This doesn't work for clocks that don't have a meaningful unit address (like the oscillators), but it should be the exception, rather than the norm.

Quoted 12 lines

Quoted 11 lines

drivers/clk/sunxi/clk-sunxi.c +4 -2

+               return NULL;
+       }
 
        i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
        if (of_property_read_string(node, "clock-output-names", &clk_name)) {
-               pr_warn("%s: could not read clock-output-names for \"%s\"\n",
-                       __func__, clk_name);
+               pr_err("%s: could not read clock-output-names for \"%s\"\n",
+                      __func__, clk_name);

Quoted 3 lines

Quoted 1 line
Same here. clk_name defaults to node->name. If you could, please replace it.
Really? Isn't clk_name directly from the DT clock-output-names strings here?

Only if there is a clock-output-names property in the node. If not, the string will not be modified by of_property_read_string, and you'll default to the initial value of clk_name, in this case node->name or node->full_name.

Maxime

Signature · 10 lines

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160216/bda9826e/attachment.sig>

Andre Przywara

author

2016-02-12 15:11 UTC #7 lore raw

PATCH 2/3 clk: sunxi: improve divider_clk error handling and reporting

In reply to Andre Przywara

We now report a failing ioremap, failing output names parsing, failures in table registration and in the final step. Also there was a bug where clk_register_divider_table() would return an ERR_PTR value instead of NULL, which we were checking for. We now implement proper rollback in case of an error.

Signed-off-by Andre Przywara <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

drivers/clk/sunxi/clk-sunxi.c +30 -3

index 361f396..1ac6c9e0 100644
@@ -819,17 +819,44 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 	void __iomem *reg;
 
 	reg = of_iomap(node, 0);
+	if (!reg) {
+		pr_err("Could not map registers for mux-clk: %s\n", node->name);
+		return;
+	}
 
 	clk_parent = of_clk_get_parent_name(node, 0);
 
-	of_property_read_string(node, "clock-output-names", &clk_name);
+	if (of_property_read_string(node, "clock-output-names", &clk_name)) {
+		pr_err("%s: could not read clock-output-names for \"%s\"\n",
+		       __func__, clk_name);
+		goto out_unmap;
+	}
 
 	clk = clk_register_divider_table(NULL, clk_name, clk_parent, 0,
 					 reg, data->shift, data->width,
 					 data->pow ? CLK_DIVIDER_POWER_OF_TWO : 0,
 					 data->table, &clk_lock);
-	if (clk)
-		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register divider clock %s: %ld\n",
+		       __func__, clk_name, PTR_ERR(clk));
+		goto out_unmap;
+	}
+
+	if (of_clk_add_provider(node, of_clk_src_simple_get, clk)) {
+		pr_err("%s: failed to add clock provider for %s\n",
+		       __func__, clk_name);
+		goto out_unregister;
+	}
+
+	if (!clk_register_clkdev(clk, clk_name, NULL))
+		return;
+
+	of_clk_del_provider(node);
+out_unregister:
+	clk_unregister_divider(clk);
+
+out_unmap:
+	iounmap(reg);
 }
 
 static void __init sun4i_ahb_clk_setup(struct device_node *node)

Signature · 1 line

2.6.4

Andre Przywara

author

2016-02-12 15:11 UTC #8 lore raw

PATCH 3/3 clk: sunxi: Improve divs_clk error handling and reporting

In reply to Andre Przywara

We catch errors in the base clock registration, failure to ioremap and failures in the final of_clk_add_provider() call. Also we unmap the registers when we need to rollback.

Signed-off-by Andre Przywara <[email protected]>

---
 drivers/clk/sunxi/clk-sunxi.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

drivers/clk/sunxi/clk-sunxi.c +13 -3

index 1ac6c9e0..48ed81d 100644
@@ -987,13 +987,20 @@ static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
 
 	/* Set up factor clock that we will be dividing */
 	pclk = sunxi_factors_clk_setup(node, data->factors);
+	if (!pclk)
+		return NULL;
 	parent = __clk_get_name(pclk);
 
 	reg = of_iomap(node, 0);
+	if (!reg) {
+		pr_err("Could not map registers for divs-clk: %s\n",
+		       node->name);
+		return NULL;
+	}
 
 	clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
 	if (!clk_data)
-		return NULL;
+		goto out_unmap;
 
 	clks = kcalloc(ndivs, sizeof(*clks), GFP_KERNEL);
 	if (!clks)

@@ -1076,9 +1083,10 @@ static struct clk ** __init sunxi_divs_clk_setup(struct device_node *node,
 	/* Adjust to the real max */
 	clk_data->clk_num = i;
 
-	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (!of_clk_add_provider(node, of_clk_src_onecell_get, clk_data))
+		return clks;
 
-	return clks;
+	pr_err("%s: failed to add clock provider for %s\n", __func__, clk_name);
 
 free_gate:
 	kfree(gate);

@@ -1086,6 +1094,8 @@ free_clks:
 	kfree(clks);
 free_clkdata:
 	kfree(clk_data);
+out_unmap:
+	iounmap(reg);
 	return NULL;
 }

Signature · 1 line

2.6.4