diff mbox

[v12,2/4] reset: mediatek: Add MT2701 reset driver

Message ID 1471854565-19810-3-git-send-email-erin.lo@mediatek.com (mailing list archive)
State Changes Requested, archived
Delegated to: Stephen Boyd
Headers show

Commit Message

Erin Lo Aug. 22, 2016, 8:29 a.m. UTC 
From: Shunli Wang <shunli.wang@mediatek.com>

In infrasys and perifsys, there are many reset
control bits for kinds of modules. These bits are
used as actual reset controllers to be registered
into kernel's generic reset controller framework.

Signed-off-by: Shunli Wang <shunli.wang@mediatek.com>
Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
Tested-by: John Crispin <blogic@openwrt.org>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/clk/mediatek/clk-mt2701-hif.c |  8 ++++++--
 drivers/clk/mediatek/clk-mt2701.c     | 16 ++++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

Comments

Stephen Boyd Aug. 24, 2016, 5:50 p.m. UTC | #1 
On 08/22, Erin Lo wrote:
> diff --git a/drivers/clk/mediatek/clk-mt2701-hif.c b/drivers/clk/mediatek/clk-mt2701-hif.c
> index 18b4ab5..702fd74 100644
> --- a/drivers/clk/mediatek/clk-mt2701-hif.c
> +++ b/drivers/clk/mediatek/clk-mt2701-hif.c
> @@ -52,11 +52,15 @@ static int mtk_hifsys_init(struct device_node *node)
>  						clk_data);
>  
>  	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> -	if (r)
> +	if (r) {
>  		pr_err("%s(): could not register clock provider: %d\n",
>  			__func__, r);
> +		return r;
> +	}
> +
> +	mtk_register_reset_controller(node, 1, 0x34);

The cleanup here isn't great. mtk_register_reset_controller()
should really return an error so that we can properly cleanup if
needed. Fixing that in a later patch would be a good idea.
James Liao Aug. 27, 2016, 4:21 a.m. UTC | #2 
On Wed, 2016-08-24 at 10:50 -0700, Stephen Boyd wrote:
> On 08/22, Erin Lo wrote:
> > diff --git a/drivers/clk/mediatek/clk-mt2701-hif.c b/drivers/clk/mediatek/clk-mt2701-hif.c
> > index 18b4ab5..702fd74 100644
> > --- a/drivers/clk/mediatek/clk-mt2701-hif.c
> > +++ b/drivers/clk/mediatek/clk-mt2701-hif.c
> > @@ -52,11 +52,15 @@ static int mtk_hifsys_init(struct device_node *node)
> >  						clk_data);
> >  
> >  	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> > -	if (r)
> > +	if (r) {
> >  		pr_err("%s(): could not register clock provider: %d\n",
> >  			__func__, r);
> > +		return r;
> > +	}
> > +
> > +	mtk_register_reset_controller(node, 1, 0x34);
> 
> The cleanup here isn't great. mtk_register_reset_controller()
> should really return an error so that we can properly cleanup if
> needed. Fixing that in a later patch would be a good idea.

Hi Stephen,

I think so. This function returns void because it was invoked in
CLK_OF_DECLARE() in previous SoC's drivers. I'll investigate how to make
it return an error code without breaking backward compatibility.


Best regards,

James


--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clk/mediatek/clk-mt2701-hif.c b/drivers/clk/mediatek/clk-mt2701-hif.c
index 18b4ab5..702fd74 100644
--- a/drivers/clk/mediatek/clk-mt2701-hif.c
+++ b/drivers/clk/mediatek/clk-mt2701-hif.c
@@ -52,11 +52,15 @@  static int mtk_hifsys_init(struct device_node *node)
 						clk_data);
 
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-	if (r)
+	if (r) {
 		pr_err("%s(): could not register clock provider: %d\n",
 			__func__, r);
+		return r;
+	}
+
+	mtk_register_reset_controller(node, 1, 0x34);
 
-	return r;
+	return 0;
 }
 
 static const struct of_device_id of_match_clk_mt2701_hif[] = {
diff --git a/drivers/clk/mediatek/clk-mt2701.c b/drivers/clk/mediatek/clk-mt2701.c
index f6df578..c8cc583 100644
--- a/drivers/clk/mediatek/clk-mt2701.c
+++ b/drivers/clk/mediatek/clk-mt2701.c
@@ -790,11 +790,15 @@  static int mtk_infrasys_init(struct device_node *node)
 						infra_clk_data);
 
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, infra_clk_data);
-	if (r)
+	if (r) {
 		pr_err("%s(): could not register clock provider: %d\n",
 			__func__, r);
+		return r;
+	}
 
-	return r;
+	mtk_register_reset_controller(node, 2, 0x30);
+
+	return 0;
 }
 
 static const struct mtk_gate_regs peri0_cg_regs = {
@@ -912,11 +916,15 @@  static int mtk_pericfg_init(struct device_node *node)
 			&lock, clk_data);
 
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-	if (r)
+	if (r) {
 		pr_err("%s(): could not register clock provider: %d\n",
 			__func__, r);
+		return r;
+	}
 
-	return r;
+	mtk_register_reset_controller(node, 2, 0x0);
+
+	return 0;
 }
 
 #define MT8590_PLL_FMAX		(2000 * MHZ)