From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tero Kristo Subject: Re: [PATCHv3 08/35] clk: ti: fix ti_clk_get_reg_addr error handling Date: Fri, 20 Mar 2015 09:00:46 +0200 Message-ID: <550BC59E.9010408@ti.com> References: <1424891085-10392-1-git-send-email-t-kristo@ti.com> <1424891085-10392-9-git-send-email-t-kristo@ti.com> <20150306191821.11109.2610@quantum> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from bear.ext.ti.com ([192.94.94.41]:59592 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750951AbbCTHBO (ORCPT ); Fri, 20 Mar 2015 03:01:14 -0400 In-Reply-To: <20150306191821.11109.2610@quantum> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Mike Turquette , tony@atomide.com, paul@pwsan.com, linux-omap@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org On 03/06/2015 09:18 PM, Mike Turquette wrote: > Quoting Tero Kristo (2015-02-25 11:04:18) >> There is a case where NULL can be a valid return value for >> ti_clk_get_reg_addr, specifically the case where both the provider index >> and register offsets are zero. In this case, the current error checking >> against a NULL pointer will fail. Thus, change the API to return a >> ERR_PTR value in an error case, and change all the users of this API to >> check against IS_ERR instead. >> >> Signed-off-by: Tero Kristo >> Cc: Michael Turquette > > Looks good to me. I'll interpret this as an Acked-by, will add this tag to the latest version of the set I will post later today, thanks. -Tero > > Regards, > Mike > >> --- >> drivers/clk/ti/apll.c | 5 +++-- >> drivers/clk/ti/autoidle.c | 2 +- >> drivers/clk/ti/clk.c | 7 ++++--- >> drivers/clk/ti/divider.c | 4 ++-- >> drivers/clk/ti/dpll.c | 6 +++--- >> drivers/clk/ti/gate.c | 4 ++-- >> drivers/clk/ti/interface.c | 2 +- >> drivers/clk/ti/mux.c | 4 ++-- >> 8 files changed, 18 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c >> index 72d9727..49baf38 100644 >> --- a/drivers/clk/ti/apll.c >> +++ b/drivers/clk/ti/apll.c >> @@ -203,7 +203,7 @@ static void __init of_dra7_apll_setup(struct device_node *node) >> ad->control_reg = ti_clk_get_reg_addr(node, 0); >> ad->idlest_reg = ti_clk_get_reg_addr(node, 1); >> >> - if (!ad->control_reg || !ad->idlest_reg) >> + if (IS_ERR(ad->control_reg) || IS_ERR(ad->idlest_reg)) >> goto cleanup; >> >> ad->idlest_mask = 0x1; >> @@ -384,7 +384,8 @@ static void __init of_omap2_apll_setup(struct device_node *node) >> ad->autoidle_reg = ti_clk_get_reg_addr(node, 1); >> ad->idlest_reg = ti_clk_get_reg_addr(node, 2); >> >> - if (!ad->control_reg || !ad->autoidle_reg || !ad->idlest_reg) >> + if (IS_ERR(ad->control_reg) || IS_ERR(ad->autoidle_reg) || >> + IS_ERR(ad->idlest_reg)) >> goto cleanup; >> >> clk = clk_register(NULL, &clk_hw->hw); >> diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c >> index 8912ff8..e75c64c 100644 >> --- a/drivers/clk/ti/autoidle.c >> +++ b/drivers/clk/ti/autoidle.c >> @@ -119,7 +119,7 @@ int __init of_ti_clk_autoidle_setup(struct device_node *node) >> clk->name = node->name; >> clk->reg = ti_clk_get_reg_addr(node, 0); >> >> - if (!clk->reg) { >> + if (IS_ERR(clk->reg)) { >> kfree(clk); >> return -EINVAL; >> } >> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c >> index e22b956..0ebe5c5 100644 >> --- a/drivers/clk/ti/clk.c >> +++ b/drivers/clk/ti/clk.c >> @@ -103,7 +103,8 @@ int __init ti_clk_retry_init(struct device_node *node, struct clk_hw *hw, >> * @index: register index from the clock node >> * >> * Builds clock register address from device tree information. This >> - * is a struct of type clk_omap_reg. >> + * is a struct of type clk_omap_reg. Returns a pointer to the register >> + * address, or a pointer error value in failure. >> */ >> void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index) >> { >> @@ -121,14 +122,14 @@ void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index) >> >> if (i == CLK_MAX_MEMMAPS) { >> pr_err("clk-provider not found for %s!\n", node->name); >> - return NULL; >> + return ERR_PTR(-ENOENT); >> } >> >> reg->index = i; >> >> if (of_property_read_u32_index(node, "reg", index, &val)) { >> pr_err("%s must have reg[%d]!\n", node->name, index); >> - return NULL; >> + return ERR_PTR(-EINVAL); >> } >> >> reg->offset = val; >> diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c >> index 6211893..ff5f117 100644 >> --- a/drivers/clk/ti/divider.c >> +++ b/drivers/clk/ti/divider.c >> @@ -530,8 +530,8 @@ static int __init ti_clk_divider_populate(struct device_node *node, >> u32 val; >> >> *reg = ti_clk_get_reg_addr(node, 0); >> - if (!*reg) >> - return -EINVAL; >> + if (IS_ERR(*reg)) >> + return PTR_ERR(*reg); >> >> if (!of_property_read_u32(node, "ti,bit-shift", &val)) >> *shift = val; >> diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c >> index 81dc469..11478a5 100644 >> --- a/drivers/clk/ti/dpll.c >> +++ b/drivers/clk/ti/dpll.c >> @@ -390,18 +390,18 @@ static void __init of_ti_dpll_setup(struct device_node *node, >> #endif >> } else { >> dd->idlest_reg = ti_clk_get_reg_addr(node, 1); >> - if (!dd->idlest_reg) >> + if (IS_ERR(dd->idlest_reg)) >> goto cleanup; >> >> dd->mult_div1_reg = ti_clk_get_reg_addr(node, 2); >> } >> >> - if (!dd->control_reg || !dd->mult_div1_reg) >> + if (IS_ERR(dd->control_reg) || IS_ERR(dd->mult_div1_reg)) >> goto cleanup; >> >> if (dd->autoidle_mask) { >> dd->autoidle_reg = ti_clk_get_reg_addr(node, 3); >> - if (!dd->autoidle_reg) >> + if (IS_ERR(dd->autoidle_reg)) >> goto cleanup; >> } >> >> diff --git a/drivers/clk/ti/gate.c b/drivers/clk/ti/gate.c >> index d493307..0c6fdfc 100644 >> --- a/drivers/clk/ti/gate.c >> +++ b/drivers/clk/ti/gate.c >> @@ -225,7 +225,7 @@ static void __init _of_ti_gate_clk_setup(struct device_node *node, >> >> if (ops != &omap_gate_clkdm_clk_ops) { >> reg = ti_clk_get_reg_addr(node, 0); >> - if (!reg) >> + if (IS_ERR(reg)) >> return; >> >> if (!of_property_read_u32(node, "ti,bit-shift", &val)) >> @@ -264,7 +264,7 @@ _of_ti_composite_gate_clk_setup(struct device_node *node, >> return; >> >> gate->enable_reg = ti_clk_get_reg_addr(node, 0); >> - if (!gate->enable_reg) >> + if (IS_ERR(gate->enable_reg)) >> goto cleanup; >> >> of_property_read_u32(node, "ti,bit-shift", &val); >> diff --git a/drivers/clk/ti/interface.c b/drivers/clk/ti/interface.c >> index 265d91f..c76230d 100644 >> --- a/drivers/clk/ti/interface.c >> +++ b/drivers/clk/ti/interface.c >> @@ -111,7 +111,7 @@ static void __init _of_ti_interface_clk_setup(struct device_node *node, >> u32 val; >> >> reg = ti_clk_get_reg_addr(node, 0); >> - if (!reg) >> + if (IS_ERR(reg)) >> return; >> >> if (!of_property_read_u32(node, "ti,bit-shift", &val)) >> diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c >> index 728e253..5cdeed5 100644 >> --- a/drivers/clk/ti/mux.c >> +++ b/drivers/clk/ti/mux.c >> @@ -210,7 +210,7 @@ static void of_mux_clk_setup(struct device_node *node) >> >> reg = ti_clk_get_reg_addr(node, 0); >> >> - if (!reg) >> + if (IS_ERR(reg)) >> goto cleanup; >> >> of_property_read_u32(node, "ti,bit-shift", &shift); >> @@ -283,7 +283,7 @@ static void __init of_ti_composite_mux_clk_setup(struct device_node *node) >> >> mux->reg = ti_clk_get_reg_addr(node, 0); >> >> - if (!mux->reg) >> + if (IS_ERR(mux->reg)) >> goto cleanup; >> >> if (!of_property_read_u32(node, "ti,bit-shift", &val)) >> -- >> 1.7.9.5 >>