diff mbox

soc/tegra: pmc: Don't allocate struct tegra_powergate on stack

Message ID 3fe40fcd427e49cbeac31e14721fea569d230b6e.1490073884.git.viresh.kumar@linaro.org
State Accepted
Commit 495ac33a3b82f85ed4fbdd9b826c1d2fbc8e9b68
Headers show

Commit Message

Viresh Kumar March 21, 2017, 5:24 a.m. UTC
The size of the struct tegra_powergate is quite big and if any more
fields are added to the internal genpd structure, following warnings are
thrown:

drivers/soc/tegra/pmc.c:577:1: warning: the frame size of 1176 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Avoid such warnings by allocating the structure dynamically.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/soc/tegra/pmc.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

-- 
2.12.0.432.g71c3a4f4ba37

Comments

Jon Hunter March 21, 2017, 10:37 a.m. UTC | #1
On 21/03/17 05:24, Viresh Kumar wrote:
> The size of the struct tegra_powergate is quite big and if any more

> fields are added to the internal genpd structure, following warnings are

> thrown:

> 

> drivers/soc/tegra/pmc.c:577:1: warning: the frame size of 1176 bytes is larger than 1024 bytes [-Wframe-larger-than=]


Hmmm ... AFAICT the size of the tegra_powergate struct is 312 bytes
(based upon next-20170321) and so it looks like something massive needs
to be added to the genpd struct to blow this up to over 1024 bytes. Are
there some genpd changes in-flight that are causing this?

Cheers
Jon

-- 
nvpublic
Viresh Kumar March 21, 2017, 10:39 a.m. UTC | #2
On 21-03-17, 10:37, Jon Hunter wrote:
> 

> On 21/03/17 05:24, Viresh Kumar wrote:

> > The size of the struct tegra_powergate is quite big and if any more

> > fields are added to the internal genpd structure, following warnings are

> > thrown:

> > 

> > drivers/soc/tegra/pmc.c:577:1: warning: the frame size of 1176 bytes is larger than 1024 bytes [-Wframe-larger-than=]

> 

> Hmmm ... AFAICT the size of the tegra_powergate struct is 312 bytes

> (based upon next-20170321) and so it looks like something massive needs

> to be added to the genpd struct to blow this up to over 1024 bytes. Are

> there some genpd changes in-flight that are causing this?


https://marc.info/?l=linux-kernel&m=149000247329743&w=2

This is up for discussion right now though and we don't know if it
will surely get merged or not.

-- 
viresh
Viresh Kumar April 17, 2017, 5:50 a.m. UTC | #3
On 21-03-17, 16:09, Viresh Kumar wrote:
> On 21-03-17, 10:37, Jon Hunter wrote:

> > 

> > On 21/03/17 05:24, Viresh Kumar wrote:

> > > The size of the struct tegra_powergate is quite big and if any more

> > > fields are added to the internal genpd structure, following warnings are

> > > thrown:

> > > 

> > > drivers/soc/tegra/pmc.c:577:1: warning: the frame size of 1176 bytes is larger than 1024 bytes [-Wframe-larger-than=]

> > 

> > Hmmm ... AFAICT the size of the tegra_powergate struct is 312 bytes

> > (based upon next-20170321) and so it looks like something massive needs

> > to be added to the genpd struct to blow this up to over 1024 bytes. Are

> > there some genpd changes in-flight that are causing this?

> 

> https://marc.info/?l=linux-kernel&m=149000247329743&w=2

> 

> This is up for discussion right now though and we don't know if it

> will surely get merged or not.


@Jon: Regardless of the above series, do you want this patch to be merged as it
will still be better to avoid keeping large structures on stack.

Else I would be required to keep this in my above series from now on.

-- 
viresh
Jon Hunter April 24, 2017, 10:55 a.m. UTC | #4
On 17/04/17 06:50, Viresh Kumar wrote:
> On 21-03-17, 16:09, Viresh Kumar wrote:

>> On 21-03-17, 10:37, Jon Hunter wrote:

>>>

>>> On 21/03/17 05:24, Viresh Kumar wrote:

>>>> The size of the struct tegra_powergate is quite big and if any more

>>>> fields are added to the internal genpd structure, following warnings are

>>>> thrown:

>>>>

>>>> drivers/soc/tegra/pmc.c:577:1: warning: the frame size of 1176 bytes is larger than 1024 bytes [-Wframe-larger-than=]

>>>

>>> Hmmm ... AFAICT the size of the tegra_powergate struct is 312 bytes

>>> (based upon next-20170321) and so it looks like something massive needs

>>> to be added to the genpd struct to blow this up to over 1024 bytes. Are

>>> there some genpd changes in-flight that are causing this?

>>

>> https://marc.info/?l=linux-kernel&m=149000247329743&w=2

>>

>> This is up for discussion right now though and we don't know if it

>> will surely get merged or not.

> 

> @Jon: Regardless of the above series, do you want this patch to be merged as it

> will still be better to avoid keeping large structures on stack.


Given that it is currently much less than the default threshold, it
seems ok to me as-is. However, if it looks like you patch to add the
device struct to the gpd struct is going to be accepted, then it is fine
with me. Maybe we should wait for you patch to be accepted then this can
be applied as a fix.

Jon

-- 
nvpublic
diff mbox

Patch

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..c94196b939a4 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -557,22 +557,28 @@  EXPORT_SYMBOL(tegra_powergate_remove_clamping);
 int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 				      struct reset_control *rst)
 {
-	struct tegra_powergate pg;
+	struct tegra_powergate *pg;
 	int err;
 
 	if (!tegra_powergate_is_available(id))
 		return -EINVAL;
 
-	pg.id = id;
-	pg.clks = &clk;
-	pg.num_clks = 1;
-	pg.resets = &rst;
-	pg.num_resets = 1;
+	pg = kzalloc(sizeof(*pg), GFP_KERNEL);
+	if (!pg)
+		return -ENOMEM;
+
+	pg->id = id;
+	pg->clks = &clk;
+	pg->num_clks = 1;
+	pg->resets = &rst;
+	pg->num_resets = 1;
 
-	err = tegra_powergate_power_up(&pg, false);
+	err = tegra_powergate_power_up(pg, false);
 	if (err)
 		pr_err("failed to turn on partition %d: %d\n", id, err);
 
+	kfree(pg);
+
 	return err;
 }
 EXPORT_SYMBOL(tegra_powergate_sequence_power_up);