[U-Boot] unused-const-variable warnings in FSL DDR driver
Thomas Schaefer
Thomas.Schaefer at kontron.com
Fri Feb 10 10:00:21 UTC 2017
> On Thu, Feb 09, 2017 at 05:51:36PM +0000, york sun wrote:
>> On 02/09/2017 09:46 AM, Thomas Schaefer wrote:
>> >>>
>> >>>> On 02/09/2017 02:32 AM, Thomas Schaefer wrote:
>> >>>>> Hi York,
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> When compiling latest u-boot with gcc 6.3 compiler, I get
>> >>>>> several 'unused-const-variable' warnings in options.c file of FSL DDR driver.
>> >>>>> Affected variables are for (DIMM_SLOTS_PER_CTLR==2) configuration (e.g.
>> >>>>> dual_0S[4]) and warnings could be fixed with the patch applied.
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> What do you think?
>> >>>>
>> >>>> Thomas,
>> >>>>
>> >>>> Thanks for bringing it to my attention. I understand GCC 6 may
>> >>>> have more warnings. The proposed patch is OK in logic but it
>> >>>> increases the size of code unnecessarily. Can you come up with a different fix?
>> >>>>
>> >>>> I can come back to check after I finish my work on hand.
>> >>>>
>> >>>> York
>> >>>
>> >>> Hi York,
>> >>>
>> >>> not sure if I understand this correctly, but why is code size
>> >>> increased when these variables are not defined? I think code size
>> >>> is decreased instead as these variables are no longer defined if not needed.
>> >>>
>> >>> I also don't see a way to achieve this in a different way as the
>> >>> variables are defined differently for DDR2, DDR3 and DDR4.
>> >>>
>> >>>
>>> >
>> >> Wait a minute, did you generate the patch backward? Your patch
>> >> shows removing "#if CONFIG_DIMM_SLOTS_PER_CTLR==2" which doesn't
>> >> exist in current code. If the logic is reversed, then yes, you are
>> >> reducing the size. Can GCC 6 optimize out the unused data? If yes,
>> >> maybe we can use __maybe_unused to get rid of the warning?
>> >>
>> >> York
>> >
>> > Oops, you are right, sorry for the confusion. Here's the corrected version:
>> >
>> > diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c
>> > index d6a8fcb216..d90ed0b6cc 100644
>> > --- a/drivers/ddr/fsl/options.c
>> > +++ b/drivers/ddr/fsl/options.c
>> > @@ -89,6 +89,7 @@ static const struct dynamic_odt single_S[4] = {
>> > {0, 0, 0, 0},
>> > };
>> >
>> > +#if (CONFIG_DIMM_SLOTS_PER_CTLR==2)
>> > static const struct dynamic_odt dual_DD[4] = {
>> > { /* cs0 */
>> > FSL_DDR_ODT_NEVER,
>> > @@ -235,6 +236,7 @@ static const struct dynamic_odt dual_0S[4] = {
>> > {0, 0, 0, 0}
>> >
>> > };
>> > +#endif
>> >
>> > static const struct dynamic_odt odt_unknown[4] = {
>> > { /* cs0 */
>> > @@ -319,6 +321,7 @@ static const struct dynamic_odt single_S[4] = {
>> > {0, 0, 0, 0},
>> > };
>> >
>> > +#if (CONFIG_DIMM_SLOTS_PER_CTLR==2)
>> > static const struct dynamic_odt dual_DD[4] = {
>> > { /* cs0 */
>> > FSL_DDR_ODT_NEVER,
>> > @@ -465,6 +468,7 @@ static const struct dynamic_odt dual_0S[4] = {
>> > {0, 0, 0, 0}
>> >
>> > };
>> > +#endif
>> >
>> > static const struct dynamic_odt odt_unknown[4] = {
>> > { /* cs0 */
>> > @@ -529,6 +533,7 @@ static const struct dynamic_odt single_S[4] = {
>> > {0, 0, 0, 0},
>> > };
>> >
>> > +#if (CONFIG_DIMM_SLOTS_PER_CTLR==2)
>> > static const struct dynamic_odt dual_DD[4] = {
>> > { /* cs0 */
>> > FSL_DDR_ODT_OTHER_DIMM, @@ -676,6 +681,7 @@ static
>> > const struct dynamic_odt dual_0S[4] = {
>> > {0, 0, 0, 0}
>> >
>> > };
>> > +#endif
>> >
>> > static const struct dynamic_odt odt_unknown[4] = {
>> > { /* cs0 */
>> >
>> >
>>
>> This looks better. Can you check the size before and after this change?
>> I wonder if GCC 6 can optimize out unused const. If it can, we may be
>> able to get away by using __maybe_used and save a lot of #if.
>>
>> By the way, please put space around "==" if you want to go this way.
>
>The above isn't quite enough for all cases. I'm testing a different and larger patch currently.
>
>--
>Tom
I've built u-boot for T1042RDB with and without the above patch applied. Actually, binary images
created are the same (with the exception of build timestamp of course), so GCC actually removes
the unused variables.
When adding __maybe_unsued attribute to the variable definitions, warnings don't show up anymore:
diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c
index d6a8fcb216..64aaa77047 100644
--- a/drivers/ddr/fsl/options.c
+++ b/drivers/ddr/fsl/options.c
@@ -33,7 +33,7 @@ struct dynamic_odt {
/* Quad rank is not verified yet due availability.
* Replacing 20 OHM with 34 OHM since DDR4 doesn't have 20 OHM option
*/
-static const struct dynamic_odt single_Q[4] = {
+__maybe_unused static const struct dynamic_odt single_Q[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS_AND_OTHER_DIMM,
@@ -60,7 +60,7 @@ static const struct dynamic_odt single_Q[4] = {
}
};
-static const struct dynamic_odt single_D[4] = {
+__maybe_unused static const struct dynamic_odt single_D[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -77,7 +77,7 @@ static const struct dynamic_odt single_D[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt single_S[4] = {
+__maybe_unused static const struct dynamic_odt single_S[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -89,7 +89,7 @@ static const struct dynamic_odt single_S[4] = {
{0, 0, 0, 0},
};
-static const struct dynamic_odt dual_DD[4] = {
+__maybe_unused static const struct dynamic_odt dual_DD[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -116,7 +116,7 @@ static const struct dynamic_odt dual_DD[4] = {
}
};
-static const struct dynamic_odt dual_DS[4] = {
+__maybe_unused static const struct dynamic_odt dual_DS[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -137,7 +137,7 @@ static const struct dynamic_odt dual_DS[4] = {
},
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_SD[4] = {
+__maybe_unused static const struct dynamic_odt dual_SD[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_ALL,
@@ -159,7 +159,7 @@ static const struct dynamic_odt dual_SD[4] = {
}
};
-static const struct dynamic_odt dual_SS[4] = {
+__maybe_unused static const struct dynamic_odt dual_SS[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_ALL,
@@ -176,7 +176,7 @@ static const struct dynamic_odt dual_SS[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_D0[4] = {
+__maybe_unused static const struct dynamic_odt dual_D0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -193,7 +193,7 @@ static const struct dynamic_odt dual_D0[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_0D[4] = {
+__maybe_unused static const struct dynamic_odt dual_0D[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -210,7 +210,7 @@ static const struct dynamic_odt dual_0D[4] = {
}
};
-static const struct dynamic_odt dual_S0[4] = {
+__maybe_unused static const struct dynamic_odt dual_S0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
@@ -223,7 +223,7 @@ static const struct dynamic_odt dual_S0[4] = {
};
-static const struct dynamic_odt dual_0S[4] = {
+__maybe_unused static const struct dynamic_odt dual_0S[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -236,7 +236,7 @@ static const struct dynamic_odt dual_0S[4] = {
};
-static const struct dynamic_odt odt_unknown[4] = {
+__maybe_unused static const struct dynamic_odt odt_unknown[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
@@ -263,7 +263,7 @@ static const struct dynamic_odt odt_unknown[4] = {
}
};
#elif defined(CONFIG_SYS_FSL_DDR3)
-static const struct dynamic_odt single_Q[4] = {
+__maybe_unused static const struct dynamic_odt single_Q[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS_AND_OTHER_DIMM,
@@ -290,7 +290,7 @@ static const struct dynamic_odt single_Q[4] = {
}
};
-static const struct dynamic_odt single_D[4] = {
+__maybe_unused static const struct dynamic_odt single_D[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -307,7 +307,7 @@ static const struct dynamic_odt single_D[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt single_S[4] = {
+__maybe_unused static const struct dynamic_odt single_S[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -319,7 +319,7 @@ static const struct dynamic_odt single_S[4] = {
{0, 0, 0, 0},
};
-static const struct dynamic_odt dual_DD[4] = {
+__maybe_unused static const struct dynamic_odt dual_DD[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -346,7 +346,7 @@ static const struct dynamic_odt dual_DD[4] = {
}
};
-static const struct dynamic_odt dual_DS[4] = {
+__maybe_unused static const struct dynamic_odt dual_DS[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -367,7 +367,7 @@ static const struct dynamic_odt dual_DS[4] = {
},
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_SD[4] = {
+__maybe_unused static const struct dynamic_odt dual_SD[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_ALL,
@@ -389,7 +389,7 @@ static const struct dynamic_odt dual_SD[4] = {
}
};
-static const struct dynamic_odt dual_SS[4] = {
+__maybe_unused static const struct dynamic_odt dual_SS[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_ALL,
@@ -406,7 +406,7 @@ static const struct dynamic_odt dual_SS[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_D0[4] = {
+__maybe_unused static const struct dynamic_odt dual_D0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_SAME_DIMM,
@@ -423,7 +423,7 @@ static const struct dynamic_odt dual_D0[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_0D[4] = {
+__maybe_unused static const struct dynamic_odt dual_0D[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -440,7 +440,7 @@ static const struct dynamic_odt dual_0D[4] = {
}
};
-static const struct dynamic_odt dual_S0[4] = {
+__maybe_unused static const struct dynamic_odt dual_S0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
@@ -453,7 +453,7 @@ static const struct dynamic_odt dual_S0[4] = {
};
-static const struct dynamic_odt dual_0S[4] = {
+__maybe_unused static const struct dynamic_odt dual_0S[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -466,7 +466,7 @@ static const struct dynamic_odt dual_0S[4] = {
};
-static const struct dynamic_odt odt_unknown[4] = {
+__maybe_unused static const struct dynamic_odt odt_unknown[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
@@ -493,14 +493,14 @@ static const struct dynamic_odt odt_unknown[4] = {
}
};
#else /* CONFIG_SYS_FSL_DDR3 */
-static const struct dynamic_odt single_Q[4] = {
+__maybe_unused static const struct dynamic_odt single_Q[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{0, 0, 0, 0},
{0, 0, 0, 0}
};
-static const struct dynamic_odt single_D[4] = {
+__maybe_unused static const struct dynamic_odt single_D[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -517,7 +517,7 @@ static const struct dynamic_odt single_D[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt single_S[4] = {
+__maybe_unused static const struct dynamic_odt single_S[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -529,7 +529,7 @@ static const struct dynamic_odt single_S[4] = {
{0, 0, 0, 0},
};
-static const struct dynamic_odt dual_DD[4] = {
+__maybe_unused static const struct dynamic_odt dual_DD[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_OTHER_DIMM,
@@ -556,7 +556,7 @@ static const struct dynamic_odt dual_DD[4] = {
}
};
-static const struct dynamic_odt dual_DS[4] = {
+__maybe_unused static const struct dynamic_odt dual_DS[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_OTHER_DIMM,
@@ -578,7 +578,7 @@ static const struct dynamic_odt dual_DS[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_SD[4] = {
+__maybe_unused static const struct dynamic_odt dual_SD[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_OTHER_DIMM,
@@ -600,7 +600,7 @@ static const struct dynamic_odt dual_SD[4] = {
}
};
-static const struct dynamic_odt dual_SS[4] = {
+__maybe_unused static const struct dynamic_odt dual_SS[4] = {
{ /* cs0 */
FSL_DDR_ODT_OTHER_DIMM,
FSL_DDR_ODT_OTHER_DIMM,
@@ -617,7 +617,7 @@ static const struct dynamic_odt dual_SS[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_D0[4] = {
+__maybe_unused static const struct dynamic_odt dual_D0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_ALL,
@@ -634,7 +634,7 @@ static const struct dynamic_odt dual_D0[4] = {
{0, 0, 0, 0}
};
-static const struct dynamic_odt dual_0D[4] = {
+__maybe_unused static const struct dynamic_odt dual_0D[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -651,7 +651,7 @@ static const struct dynamic_odt dual_0D[4] = {
}
};
-static const struct dynamic_odt dual_S0[4] = {
+__maybe_unused static const struct dynamic_odt dual_S0[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
@@ -664,7 +664,7 @@ static const struct dynamic_odt dual_S0[4] = {
};
-static const struct dynamic_odt dual_0S[4] = {
+__maybe_unused static const struct dynamic_odt dual_0S[4] = {
{0, 0, 0, 0},
{0, 0, 0, 0},
{ /* cs2 */
@@ -677,7 +677,7 @@ static const struct dynamic_odt dual_0S[4] = {
};
-static const struct dynamic_odt odt_unknown[4] = {
+__maybe_unused static const struct dynamic_odt odt_unknown[4] = {
{ /* cs0 */
FSL_DDR_ODT_NEVER,
FSL_DDR_ODT_CS,
Best regards,
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: drivers_ddr_fsl_options.patch
Type: application/octet-stream
Size: 9324 bytes
Desc: drivers_ddr_fsl_options.patch
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170210/d3c02a7c/attachment.obj>
More information about the U-Boot
mailing list