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.perttu...@kapsi.fi>
> Acked-by: Michael Turquette <mturque...@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
pgpLCqeHRK1x9.pgp
Description: PGP signature
