Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
On Mon, Mar 27, 2017 at 09:32:26AM +, Thomas Schaefer wrote: > > From: york sun [mailto:york@nxp.com] > > Sent: Samstag, 25. März 2017 16:35 > > To: Thomas Schaefer; Tom Rini > > Cc: u-boot@lists.denx.de > > Re: Re: AW: [U-Boot] unused-const-variable warnings in FSL DDR driver > > > > Thomas, > > > > Can you put your patch together with proper commit message and signature? I > > prefer your solution than this one http://patchwork.ozlabs.org/patch/726353/ > > > > York > > > > Hi York, > > please find the patch in the attachment. > > Best regards, > Thomas > > > commit 66dcf316bf2649e576f1b79aad3a5bb950d178d6 > Author: Thomas Schaefer <thomas.schae...@kontron.com> > Date: Mon Mar 27 10:17:09 2017 +0200 > > drivers: ddr: fix unused-const-variable warnings > > Depending on DDR configuration, gcc-6.x will show up unused-const- > variable messages. Use __maybe_unused specifier for all dynamic_odt > variable definitions to remove these warnings. > > Memory footprint will not increase as gcc will optimize out unused > constants. > > Signed-off-by: Thomas Schaefer <thomas.schae...@kontron.com> > > diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c > index d6a8fcb216..64aaa77047 100644 Reviewed-by: Tom Rini <tr...@konsulko.com> -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
> From: york sun [mailto:york@nxp.com]
> Sent: Samstag, 25. März 2017 16:35
> To: Thomas Schaefer; Tom Rini
> Cc: u-boot@lists.denx.de
> Re: Re: AW: [U-Boot] unused-const-variable warnings in FSL DDR driver
>
> Thomas,
>
> Can you put your patch together with proper commit message and signature? I
> prefer your solution than this one http://patchwork.ozlabs.org/patch/726353/
>
> York
>
Hi York,
please find the patch in the attachment.
Best regards,
Thomas
commit 66dcf316bf2649e576f1b79aad3a5bb950d178d6
Author: Thomas Schaefer <thomas.schae...@kontron.com>
Date: Mon Mar 27 10:17:09 2017 +0200
drivers: ddr: fix unused-const-variable warnings
Depending on DDR configuration, gcc-6.x will show up unused-const-
variable messages. Use __maybe_unused specifier for all dynamic_odt
variable definitions to remove these warnings.
Memory footprint will not increase as gcc will optimize out unused
constants.
Signed-off-by: Thomas Schaefer <thomas.schae...@kontron.com>
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] = {
+__mayb
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
On 02/10/2017 02:00 AM, Thomas Schaefer wrote: >> On Thu, Feb 09, 2017 at 05:51:36PM +, 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, > > 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 Thomas, Can you put your patch together with proper commit message and signature? I prefer your solution than this one http://patchwork.ozlabs.org/patch/726353/ York ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
> On Thu, Feb 09, 2017 at 05:51:36PM +, 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
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
On 02/09/2017 09:17 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
On Thu, Feb 09, 2017 at 05:51:36PM +, 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
signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
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.
York
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
>>
>>> 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 */
Best regards,
Thomas
drivers_ddr_fsl_options.patch
Description: drivers_ddr_fsl_options.patch
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] unused-const-variable warnings in FSL DDR driver
> 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. Best regards, Thomas ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] unused-const-variable warnings in FSL DDR driver
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? Best regards, Thomas drivers_ddr_fsl_options.patch Description: drivers_ddr_fsl_options.patch ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
