[PATCH 07/31] clk: tegra: implement a reset driver

Stephen Warren swarren at wwwdotorg.org
Tue Dec 3 14:07:21 EST 2013


On 11/29/2013 06:26 AM, Thierry Reding wrote:
> On Fri, Nov 15, 2013 at 01:54:02PM -0700, Stephen Warren wrote: 
> [...]
>> diff --git a/drivers/clk/tegra/clk-tegra114.c
>> b/drivers/clk/tegra/clk-tegra114.c
> [...]
>> -	clks = tegra_clk_init(TEGRA124_CLK_CLK_MAX, 6); +	clks =
>> tegra_clk_init(clk_base, TEGRA124_CLK_CLK_MAX, 6);
> 
> This doesn't really concern this patch, but this is inconsistent
> with the drivers for other generations. We should probably make
> that consistent in a separate patch.
> 
>> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> 
>> +static int tegra_clk_rst_assert(struct reset_controller_dev
>> *rcdev, +		unsigned long id); +static int
>> tegra_clk_rst_deassert(struct reset_controller_dev *rcdev, +
>> unsigned long id);
> 
> Can you reorder the code so that these forward-declarations can be 
> avoided?
> 
>> /* Global data of Tegra CPU CAR ops */ static struct
>> tegra_cpu_car_ops dummy_car_ops; struct tegra_cpu_car_ops
>> *tegra_cpu_car_ops = &dummy_car_ops; @@ -70,6 +77,17 @@ static
>> struct clk **clks; static int clk_num; static struct
>> clk_onecell_data clk_data;
>> 
>> +static struct reset_control_ops rst_ops = { +	.assert =
>> tegra_clk_rst_assert, +	.deassert = tegra_clk_rst_deassert, +}; 
>> + +static struct reset_controller_dev rst_ctlr = { +	.ops =
>> &rst_ops, +	.owner = THIS_MODULE, +	.of_reset_n_cells = 1, +};
> 
> It looks like these can be moved further down (below the
> implementation of tegra_clk_rst_assert() and
> tegra_clk_rst_deassert()). I rather like not having to modify two
> locations when the signature changes, but it's not that big a deal,
> so with or without that fixed up:
> 
> Reviewed-by: Thierry Reding <treding at nvidia.com>

OK, I moved the structs right before the function that uses them,
removed the function prototypes, and maintained your tag. Thanks.



More information about the linux-arm-kernel mailing list