diff mbox

[v10,05/17] clk: tegra: Introduce ability for SoC-specific reset control callbacks

Message ID 1432035567-19008-1-git-send-email-mikko.perttunen@kapsi.fi (mailing list archive)
State New, archived
Headers show

Commit Message

Mikko Perttunen May 19, 2015, 11:39 a.m. UTC
This patch allows SoC-specific CAR initialization routines to register
their own reset_assert and reset_deassert callbacks with the common Tegra
CAR code. If defined, the common code will call these callbacks when a
reset control with number >= num_periph_banks * 32 is attempted to be asserted 
or deasserted respectively. Numbers greater than or equal to num_periph_banks * 32
are used to avoid clashes with low numbers that are automatically mapped to
standard CAR reset lines.

Each SoC with these special resets should specify the defined reset control
numbers in a device tree header file.

Signed-off-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
Acked-by: Michael Turquette <mturquette@linaro.org>
---
 drivers/clk/tegra/clk.c | 39 +++++++++++++++++++++++++++++++--------
 drivers/clk/tegra/clk.h |  3 +++
 2 files changed, 34 insertions(+), 8 deletions(-)

Comments

Mikko Perttunen May 19, 2015, 11:46 a.m. UTC | #1
Forgot to write it in the patch description, but v10 for patches 5 and 6 
changes the base for custom resets from 0x40000000 to periph_banks * 32 
and also makes the SoC-specific code tell the core how many special 
resets it has. This is quite a bit cleaner than the previous version.

Thanks to Thierry for the idea.

Mikko

On 05/19/15 14:39, Mikko Perttunen wrote:
> This patch allows SoC-specific CAR initialization routines to register
> their own reset_assert and reset_deassert callbacks with the common Tegra
> CAR code. If defined, the common code will call these callbacks when a
> reset control with number >= num_periph_banks * 32 is attempted to be asserted
> or deasserted respectively. Numbers greater than or equal to num_periph_banks * 32
> are used to avoid clashes with low numbers that are automatically mapped to
> standard CAR reset lines.
>
> Each SoC with these special resets should specify the defined reset control
> numbers in a device tree header file.
>
> Signed-off-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
> Acked-by: Michael Turquette <mturquette@linaro.org>
> ---
>   drivers/clk/tegra/clk.c | 39 +++++++++++++++++++++++++++++++--------
>   drivers/clk/tegra/clk.h |  3 +++
>   2 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index 41cd87c..c093ed9 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -49,7 +49,6 @@
>   #define RST_DEVICES_L			0x004
>   #define RST_DEVICES_H			0x008
>   #define RST_DEVICES_U			0x00C
> -#define RST_DFLL_DVCO			0x2F4
>   #define RST_DEVICES_V			0x358
>   #define RST_DEVICES_W			0x35C
>   #define RST_DEVICES_X			0x28C
> @@ -79,6 +78,11 @@ static struct clk **clks;
>   static int clk_num;
>   static struct clk_onecell_data clk_data;
>
> +/* Handlers for SoC-specific reset lines */
> +static int (*special_reset_assert)(unsigned long);
> +static int (*special_reset_deassert)(unsigned long);
> +static int special_reset_num;
> +
>   static struct tegra_clk_periph_regs periph_regs[] = {
>   	[0] = {
>   		.enb_reg = CLK_OUT_ENB_L,
> @@ -152,19 +156,29 @@ static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
>   	 */
>   	tegra_read_chipid();
>
> -	writel_relaxed(BIT(id % 32),
> -			clk_base + periph_regs[id / 32].rst_set_reg);
> +	if (id < periph_banks * 32) {
> +		writel_relaxed(BIT(id % 32),
> +			       clk_base + periph_regs[id / 32].rst_set_reg);
> +		return 0;
> +	} else if (id < periph_banks * 32 + special_reset_num) {
> +		return special_reset_assert(id);
> +	}
>
> -	return 0;
> +	return -EINVAL;
>   }
>
>   static int tegra_clk_rst_deassert(struct reset_controller_dev *rcdev,
>   		unsigned long id)
>   {
> -	writel_relaxed(BIT(id % 32),
> -			clk_base + periph_regs[id / 32].rst_clr_reg);
> +	if (id < periph_banks * 32) {
> +		writel_relaxed(BIT(id % 32),
> +			       clk_base + periph_regs[id / 32].rst_clr_reg);
> +		return 0;
> +	} else if (id < periph_banks * 32 + special_reset_num) {
> +		return special_reset_deassert(id);
> +	}
>
> -	return 0;
> +	return -EINVAL;
>   }
>
>   struct tegra_clk_periph_regs *get_reg_bank(int clkid)
> @@ -286,10 +300,19 @@ void __init tegra_add_of_provider(struct device_node *np)
>   	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>
>   	rst_ctlr.of_node = np;
> -	rst_ctlr.nr_resets = periph_banks * 32;
> +	rst_ctlr.nr_resets = periph_banks * 32 + special_reset_num;
>   	reset_controller_register(&rst_ctlr);
>   }
>
> +void __init tegra_init_special_resets(int num,
> +				      int (*assert)(unsigned long),
> +				      int (*deassert)(unsigned long))
> +{
> +	special_reset_num = num;
> +	special_reset_assert = assert;
> +	special_reset_deassert = deassert;
> +}
> +
>   void __init tegra_register_devclks(struct tegra_devclk *dev_clks, int num)
>   {
>   	int i;
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 75ddc8ff..f42cbbe 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -591,6 +591,9 @@ struct tegra_devclk {
>   	char		*con_id;
>   };
>
> +void tegra_init_special_resets(int num, int (*assert)(unsigned long),
> +			       int (*deassert)(unsigned long));
> +
>   void tegra_init_from_table(struct tegra_clk_init_table *tbl,
>   		struct clk *clks[], int clk_max);
>
>
Thierry Reding May 19, 2015, 2:59 p.m. UTC | #2
On Tue, May 19, 2015 at 02:39:27PM +0300, Mikko Perttunen wrote:
> This patch allows SoC-specific CAR initialization routines to register
> their own reset_assert and reset_deassert callbacks with the common Tegra
> CAR code. If defined, the common code will call these callbacks when a
> reset control with number >= num_periph_banks * 32 is attempted to be asserted 
> or deasserted respectively. Numbers greater than or equal to num_periph_banks * 32
> are used to avoid clashes with low numbers that are automatically mapped to
> standard CAR reset lines.
> 
> Each SoC with these special resets should specify the defined reset control
> numbers in a device tree header file.

This is looking pretty good, but I think we can simplify a wee bit
more...

> Signed-off-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
> Acked-by: Michael Turquette <mturquette@linaro.org>
> ---
>  drivers/clk/tegra/clk.c | 39 +++++++++++++++++++++++++++++++--------
>  drivers/clk/tegra/clk.h |  3 +++
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index 41cd87c..c093ed9 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -49,7 +49,6 @@
>  #define RST_DEVICES_L			0x004
>  #define RST_DEVICES_H			0x008
>  #define RST_DEVICES_U			0x00C
> -#define RST_DFLL_DVCO			0x2F4
>  #define RST_DEVICES_V			0x358
>  #define RST_DEVICES_W			0x35C
>  #define RST_DEVICES_X			0x28C
> @@ -79,6 +78,11 @@ static struct clk **clks;
>  static int clk_num;
>  static struct clk_onecell_data clk_data;
>  
> +/* Handlers for SoC-specific reset lines */
> +static int (*special_reset_assert)(unsigned long);
> +static int (*special_reset_deassert)(unsigned long);
> +static int special_reset_num;

I think we can get rid of this if we...

> +
>  static struct tegra_clk_periph_regs periph_regs[] = {
>  	[0] = {
>  		.enb_reg = CLK_OUT_ENB_L,
> @@ -152,19 +156,29 @@ static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
>  	 */
>  	tegra_read_chipid();
>  
> -	writel_relaxed(BIT(id % 32),
> -			clk_base + periph_regs[id / 32].rst_set_reg);
> +	if (id < periph_banks * 32) {
> +		writel_relaxed(BIT(id % 32),
> +			       clk_base + periph_regs[id / 32].rst_set_reg);
> +		return 0;
> +	} else if (id < periph_banks * 32 + special_reset_num) {
> +		return special_reset_assert(id);
> +	}

... pass id - periph_banks * 32 into special_reset_assert(). Oh, but
then...

> @@ -286,10 +300,19 @@ void __init tegra_add_of_provider(struct device_node *np)
>  	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>  
>  	rst_ctlr.of_node = np;
> -	rst_ctlr.nr_resets = periph_banks * 32;
> +	rst_ctlr.nr_resets = periph_banks * 32 + special_reset_num;

... this is no longer going to work. We could keep special_reset_num,
though obviously it should be an unsigned int and renamed to
num_special_reset, yet still pass the relative ID into the SoC-specific
callbacks, after all that's what they care about and each implementation
would have to subtract periph_banks * 32 anyway.

>  	reset_controller_register(&rst_ctlr);
>  }
>  
> +void __init tegra_init_special_resets(int num,
> +				      int (*assert)(unsigned long),
> +				      int (*deassert)(unsigned long))
> +{
> +	special_reset_num = num;
> +	special_reset_assert = assert;
> +	special_reset_deassert = deassert;
> +}
> +

I think we could possibly improve this interface somewhat by turning it
upside-down, that is, make SoC-specific drivers call this in a helper
fashion. But this is good enough for now, I can always take a stab at
refactoring if I get bored.

Thierry
Mikko Perttunen May 19, 2015, 3:06 p.m. UTC | #3
On 05/19/2015 05:59 PM, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:39:27PM +0300, Mikko Perttunen wrote:
>> This patch allows SoC-specific CAR initialization routines to register
>> their own reset_assert and reset_deassert callbacks with the common Tegra
>> CAR code. If defined, the common code will call these callbacks when a
>> reset control with number >= num_periph_banks * 32 is attempted to be asserted
>> or deasserted respectively. Numbers greater than or equal to num_periph_banks * 32
>> are used to avoid clashes with low numbers that are automatically mapped to
>> standard CAR reset lines.
>>
>> Each SoC with these special resets should specify the defined reset control
>> numbers in a device tree header file.
>
> This is looking pretty good, but I think we can simplify a wee bit
> more...
>
>> Signed-off-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
>> Acked-by: Michael Turquette <mturquette@linaro.org>
>> ---
>>   drivers/clk/tegra/clk.c | 39 +++++++++++++++++++++++++++++++--------
>>   drivers/clk/tegra/clk.h |  3 +++
>>   2 files changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
>> index 41cd87c..c093ed9 100644
>> --- a/drivers/clk/tegra/clk.c
>> +++ b/drivers/clk/tegra/clk.c
>> @@ -49,7 +49,6 @@
>>   #define RST_DEVICES_L			0x004
>>   #define RST_DEVICES_H			0x008
>>   #define RST_DEVICES_U			0x00C
>> -#define RST_DFLL_DVCO			0x2F4
>>   #define RST_DEVICES_V			0x358
>>   #define RST_DEVICES_W			0x35C
>>   #define RST_DEVICES_X			0x28C
>> @@ -79,6 +78,11 @@ static struct clk **clks;
>>   static int clk_num;
>>   static struct clk_onecell_data clk_data;
>>
>> +/* Handlers for SoC-specific reset lines */
>> +static int (*special_reset_assert)(unsigned long);
>> +static int (*special_reset_deassert)(unsigned long);
>> +static int special_reset_num;
>
> I think we can get rid of this if we...
>
>> +
>>   static struct tegra_clk_periph_regs periph_regs[] = {
>>   	[0] = {
>>   		.enb_reg = CLK_OUT_ENB_L,
>> @@ -152,19 +156,29 @@ static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
>>   	 */
>>   	tegra_read_chipid();
>>
>> -	writel_relaxed(BIT(id % 32),
>> -			clk_base + periph_regs[id / 32].rst_set_reg);
>> +	if (id < periph_banks * 32) {
>> +		writel_relaxed(BIT(id % 32),
>> +			       clk_base + periph_regs[id / 32].rst_set_reg);
>> +		return 0;
>> +	} else if (id < periph_banks * 32 + special_reset_num) {
>> +		return special_reset_assert(id);
>> +	}
>
> ... pass id - periph_banks * 32 into special_reset_assert(). Oh, but
> then...

The reason I don't subtract periph_banks * 32 is because this way the 
code in the SoC-specific callback can just include the dt-bindings 
header and use the same defines used in the device tree.

>
>> @@ -286,10 +300,19 @@ void __init tegra_add_of_provider(struct device_node *np)
>>   	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>>
>>   	rst_ctlr.of_node = np;
>> -	rst_ctlr.nr_resets = periph_banks * 32;
>> +	rst_ctlr.nr_resets = periph_banks * 32 + special_reset_num;
>
> ... this is no longer going to work. We could keep special_reset_num,
> though obviously it should be an unsigned int and renamed to
> num_special_reset, yet still pass the relative ID into the SoC-specific
> callbacks, after all that's what they care about and each implementation
> would have to subtract periph_banks * 32 anyway.

Mostly agreed, but see above :)

>
>>   	reset_controller_register(&rst_ctlr);
>>   }
>>
>> +void __init tegra_init_special_resets(int num,
>> +				      int (*assert)(unsigned long),
>> +				      int (*deassert)(unsigned long))
>> +{
>> +	special_reset_num = num;
>> +	special_reset_assert = assert;
>> +	special_reset_deassert = deassert;
>> +}
>> +
>
> I think we could possibly improve this interface somewhat by turning it
> upside-down, that is, make SoC-specific drivers call this in a helper
> fashion. But this is good enough for now, I can always take a stab at
> refactoring if I get bored.
>
> Thierry
>

Thanks,
Mikko
Thierry Reding May 19, 2015, 3:22 p.m. UTC | #4
On Tue, May 19, 2015 at 06:06:39PM +0300, Mikko Perttunen wrote:
> On 05/19/2015 05:59 PM, Thierry Reding wrote:
> >On Tue, May 19, 2015 at 02:39:27PM +0300, Mikko Perttunen wrote:
> >>This patch allows SoC-specific CAR initialization routines to register
> >>their own reset_assert and reset_deassert callbacks with the common Tegra
> >>CAR code. If defined, the common code will call these callbacks when a
> >>reset control with number >= num_periph_banks * 32 is attempted to be asserted
> >>or deasserted respectively. Numbers greater than or equal to num_periph_banks * 32
> >>are used to avoid clashes with low numbers that are automatically mapped to
> >>standard CAR reset lines.
> >>
> >>Each SoC with these special resets should specify the defined reset control
> >>numbers in a device tree header file.
> >
> >This is looking pretty good, but I think we can simplify a wee bit
> >more...
> >
> >>Signed-off-by: Mikko Perttunen <mikko.perttunen@kapsi.fi>
> >>Acked-by: Michael Turquette <mturquette@linaro.org>
> >>---
> >>  drivers/clk/tegra/clk.c | 39 +++++++++++++++++++++++++++++++--------
> >>  drivers/clk/tegra/clk.h |  3 +++
> >>  2 files changed, 34 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> >>index 41cd87c..c093ed9 100644
> >>--- a/drivers/clk/tegra/clk.c
> >>+++ b/drivers/clk/tegra/clk.c
> >>@@ -49,7 +49,6 @@
> >>  #define RST_DEVICES_L			0x004
> >>  #define RST_DEVICES_H			0x008
> >>  #define RST_DEVICES_U			0x00C
> >>-#define RST_DFLL_DVCO			0x2F4
> >>  #define RST_DEVICES_V			0x358
> >>  #define RST_DEVICES_W			0x35C
> >>  #define RST_DEVICES_X			0x28C
> >>@@ -79,6 +78,11 @@ static struct clk **clks;
> >>  static int clk_num;
> >>  static struct clk_onecell_data clk_data;
> >>
> >>+/* Handlers for SoC-specific reset lines */
> >>+static int (*special_reset_assert)(unsigned long);
> >>+static int (*special_reset_deassert)(unsigned long);
> >>+static int special_reset_num;
> >
> >I think we can get rid of this if we...
> >
> >>+
> >>  static struct tegra_clk_periph_regs periph_regs[] = {
> >>  	[0] = {
> >>  		.enb_reg = CLK_OUT_ENB_L,
> >>@@ -152,19 +156,29 @@ static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
> >>  	 */
> >>  	tegra_read_chipid();
> >>
> >>-	writel_relaxed(BIT(id % 32),
> >>-			clk_base + periph_regs[id / 32].rst_set_reg);
> >>+	if (id < periph_banks * 32) {
> >>+		writel_relaxed(BIT(id % 32),
> >>+			       clk_base + periph_regs[id / 32].rst_set_reg);
> >>+		return 0;
> >>+	} else if (id < periph_banks * 32 + special_reset_num) {
> >>+		return special_reset_assert(id);
> >>+	}
> >
> >... pass id - periph_banks * 32 into special_reset_assert(). Oh, but
> >then...
> 
> The reason I don't subtract periph_banks * 32 is because this way the code
> in the SoC-specific callback can just include the dt-bindings header and use
> the same defines used in the device tree.

Indeed, you're right. Now if

	static int special_reset_num;

could be turned into

	static unsigned int num_special_reset;

I'd be happy to take this into the Tegra clock tree.

Thierry
diff mbox

Patch

diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index 41cd87c..c093ed9 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -49,7 +49,6 @@ 
 #define RST_DEVICES_L			0x004
 #define RST_DEVICES_H			0x008
 #define RST_DEVICES_U			0x00C
-#define RST_DFLL_DVCO			0x2F4
 #define RST_DEVICES_V			0x358
 #define RST_DEVICES_W			0x35C
 #define RST_DEVICES_X			0x28C
@@ -79,6 +78,11 @@  static struct clk **clks;
 static int clk_num;
 static struct clk_onecell_data clk_data;
 
+/* Handlers for SoC-specific reset lines */
+static int (*special_reset_assert)(unsigned long);
+static int (*special_reset_deassert)(unsigned long);
+static int special_reset_num;
+
 static struct tegra_clk_periph_regs periph_regs[] = {
 	[0] = {
 		.enb_reg = CLK_OUT_ENB_L,
@@ -152,19 +156,29 @@  static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
 	 */
 	tegra_read_chipid();
 
-	writel_relaxed(BIT(id % 32),
-			clk_base + periph_regs[id / 32].rst_set_reg);
+	if (id < periph_banks * 32) {
+		writel_relaxed(BIT(id % 32),
+			       clk_base + periph_regs[id / 32].rst_set_reg);
+		return 0;
+	} else if (id < periph_banks * 32 + special_reset_num) {
+		return special_reset_assert(id);
+	}
 
-	return 0;
+	return -EINVAL;
 }
 
 static int tegra_clk_rst_deassert(struct reset_controller_dev *rcdev,
 		unsigned long id)
 {
-	writel_relaxed(BIT(id % 32),
-			clk_base + periph_regs[id / 32].rst_clr_reg);
+	if (id < periph_banks * 32) {
+		writel_relaxed(BIT(id % 32),
+			       clk_base + periph_regs[id / 32].rst_clr_reg);
+		return 0;
+	} else if (id < periph_banks * 32 + special_reset_num) {
+		return special_reset_deassert(id);
+	}
 
-	return 0;
+	return -EINVAL;
 }
 
 struct tegra_clk_periph_regs *get_reg_bank(int clkid)
@@ -286,10 +300,19 @@  void __init tegra_add_of_provider(struct device_node *np)
 	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
 	rst_ctlr.of_node = np;
-	rst_ctlr.nr_resets = periph_banks * 32;
+	rst_ctlr.nr_resets = periph_banks * 32 + special_reset_num;
 	reset_controller_register(&rst_ctlr);
 }
 
+void __init tegra_init_special_resets(int num,
+				      int (*assert)(unsigned long),
+				      int (*deassert)(unsigned long))
+{
+	special_reset_num = num;
+	special_reset_assert = assert;
+	special_reset_deassert = deassert;
+}
+
 void __init tegra_register_devclks(struct tegra_devclk *dev_clks, int num)
 {
 	int i;
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 75ddc8ff..f42cbbe 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -591,6 +591,9 @@  struct tegra_devclk {
 	char		*con_id;
 };
 
+void tegra_init_special_resets(int num, int (*assert)(unsigned long),
+			       int (*deassert)(unsigned long));
+
 void tegra_init_from_table(struct tegra_clk_init_table *tbl,
 		struct clk *clks[], int clk_max);