Patchwork ARM: imx: clk-imx6sl: Suppress duplicate const sparse warning

login
register
mail settings
Submitter Liu Ying
Date Jan. 15, 2014, 6:19 a.m.
Message ID <1389766774-16661-1-git-send-email-Ying.Liu@freescale.com>
Download mbox | patch
Permalink /patch/310964/
State New
Headers show

Comments

Liu Ying - Jan. 15, 2014, 6:19 a.m.
There should be no duplicate const specifiers for those static
constant character string arrays defined for clock mux options.
Also, the arrays are only taken as the 5th argument for the
imx_clk_mux() function, which is in the type of 'const char
**parents'.  So, let's remove the 2nd const specifier right
after 'char'.

This patch fixes these sparse warnings:
arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:22:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:23:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:24:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:25:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:26:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:27:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:28:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:29:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:30:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:31:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:32:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:33:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:34:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:35:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:36:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:37:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:38:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:39:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:40:25: warning: duplicate const
arch/arm/mach-imx/clk-imx6sl.c:41:25: warning: duplicate const

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 arch/arm/mach-imx/clk-imx6sl.c |   42 ++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 21 deletions(-)
Shawn Guo - Jan. 15, 2014, 6:58 a.m.
On Wed, Jan 15, 2014 at 02:19:34PM +0800, Liu Ying wrote:
> There should be no duplicate const specifiers for those static
> constant character string arrays defined for clock mux options.
> Also, the arrays are only taken as the 5th argument for the
> imx_clk_mux() function, which is in the type of 'const char
> **parents'.  So, let's remove the 2nd const specifier right
> after 'char'.
> 
> This patch fixes these sparse warnings:
> arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:22:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:23:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:24:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:25:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:26:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:27:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:28:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:29:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:30:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:31:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:32:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:33:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:34:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:35:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:36:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:37:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:38:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:39:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:40:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:41:25: warning: duplicate const
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  arch/arm/mach-imx/clk-imx6sl.c |   42 ++++++++++++++++++++--------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> index c0c4ef5..9fad29a9 100644
> --- a/arch/arm/mach-imx/clk-imx6sl.c
> +++ b/arch/arm/mach-imx/clk-imx6sl.c
> @@ -18,27 +18,27 @@
>  #include "clk.h"
>  #include "common.h"
>  
> -static const char const *step_sels[]		= { "osc", "pll2_pfd2", };
...
> +static const char *step_sels[]		= { "osc", "pll2_pfd2", };

So now we're getting the following checkpatch warning:

WARNING: static const char * array should probably be static const char * const

It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
add warnings for static char that could be static const char).  I'm not
sure which warning we should ignore, the sparse or the checkpatch one.

Joe, comments?

Shawn
Joe Perches - Jan. 15, 2014, 9:21 a.m.
On Wed, 2014-01-15 at 14:58 +0800, Shawn Guo wrote:
> On Wed, Jan 15, 2014 at 02:19:34PM +0800, Liu Ying wrote:
> > There should be no duplicate const specifiers for those static
> > constant character string arrays defined for clock mux options.
> > Also, the arrays are only taken as the 5th argument for the
> > imx_clk_mux() function, which is in the type of 'const char
> > **parents'.  So, let's remove the 2nd const specifier right
> > after 'char'.
> > 
> > This patch fixes these sparse warnings:
> > arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const
[]
> > diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
[]
> > @@ -18,27 +18,27 @@
> >  #include "clk.h"
> >  #include "common.h"
> >  
> > -static const char const *step_sels[]		= { "osc", "pll2_pfd2", };
> ...
> > +static const char *step_sels[]		= { "osc", "pll2_pfd2", };
> 
> So now we're getting the following checkpatch warning:
> 
> WARNING: static const char * array should probably be static const char * const
> 
> It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
> add warnings for static char that could be static const char).  I'm not
> sure which warning we should ignore, the sparse or the checkpatch one.
> 
> Joe, comments?

Maybe the const **parents argument could be const * const *

That could change a lot of declarations through.
You could also ignore checkpatch.
Shawn Guo - Jan. 15, 2014, 11:24 a.m.
On Wed, Jan 15, 2014 at 01:21:36AM -0800, Joe Perches wrote:
> On Wed, 2014-01-15 at 14:58 +0800, Shawn Guo wrote:
> > On Wed, Jan 15, 2014 at 02:19:34PM +0800, Liu Ying wrote:
> > > There should be no duplicate const specifiers for those static
> > > constant character string arrays defined for clock mux options.
> > > Also, the arrays are only taken as the 5th argument for the
> > > imx_clk_mux() function, which is in the type of 'const char
> > > **parents'.  So, let's remove the 2nd const specifier right
> > > after 'char'.
> > > 
> > > This patch fixes these sparse warnings:
> > > arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const
> []
> > > diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> []
> > > @@ -18,27 +18,27 @@
> > >  #include "clk.h"
> > >  #include "common.h"
> > >  
> > > -static const char const *step_sels[]		= { "osc", "pll2_pfd2", };
> > ...
> > > +static const char *step_sels[]		= { "osc", "pll2_pfd2", };
> > 
> > So now we're getting the following checkpatch warning:
> > 
> > WARNING: static const char * array should probably be static const char * const
> > 
> > It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
> > add warnings for static char that could be static const char).  I'm not
> > sure which warning we should ignore, the sparse or the checkpatch one.
> > 
> > Joe, comments?
> 
> Maybe the const **parents argument could be const * const *
> 
> That could change a lot of declarations through.
> You could also ignore checkpatch.

Okay, for now, I will ignore the checkpatch warning for this case.
Thanks, Joe.

Shawn
Shawn Guo - Jan. 15, 2014, 11:27 a.m.
On Wed, Jan 15, 2014 at 02:19:34PM +0800, Liu Ying wrote:
> There should be no duplicate const specifiers for those static
> constant character string arrays defined for clock mux options.
> Also, the arrays are only taken as the 5th argument for the
> imx_clk_mux() function, which is in the type of 'const char
> **parents'.  So, let's remove the 2nd const specifier right
> after 'char'.
> 
> This patch fixes these sparse warnings:
> arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:22:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:23:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:24:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:25:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:26:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:27:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:28:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:29:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:30:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:31:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:32:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:33:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:34:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:35:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:36:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:37:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:38:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:39:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:40:25: warning: duplicate const
> arch/arm/mach-imx/clk-imx6sl.c:41:25: warning: duplicate const
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>

Applied both, thanks.
Bill Pringlemeir - Jan. 16, 2014, 5:06 p.m.
> On Wed, Jan 15, 2014 at 02:19:34PM +0800, Liu Ying wrote:
>> There should be no duplicate const specifiers for those static
>> constant character string arrays defined for clock mux options.
>> Also, the arrays are only taken as the 5th argument for the
>> imx_clk_mux() function, which is in the type of 'const char
>> **parents'.  So, let's remove the 2nd const specifier right
>> after 'char'.
>>
>> This patch fixes these sparse warnings:
>> arch/arm/mach-imx/clk-imx6sl.c:21:25: warning: duplicate const

[snip]

>> Signed-off-by: Liu Ying <Ying.Liu at freescale.com>
>> ---
>> arch/arm/mach-imx/clk-imx6sl.c | 42 ++++++++++++++++++++--------------------
>> 1 file changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
>> index c0c4ef5..9fad29a9 100644
>> --- a/arch/arm/mach-imx/clk-imx6sl.c
>> +++ b/arch/arm/mach-imx/clk-imx6sl.c
>>>> -18,27 +18,27 @@
>> #include "clk.h"
>> #include "common.h"
>>
>> -static const char const *step_sels[] = { "osc", "pll2_pfd2", };
> ...
>> +static const char *step_sels[] = { "osc", "pll2_pfd2", };

On 15 Jan 2014, shawn.guo at linaro.org wrote:

> So now we're getting the following checkpatch warning:
>
> WARNING: static const char * array should probably be static const char *
> 	const
>
> It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
> add warnings for static char that could be static const char).  I'm not
> sure which warning we should ignore, the sparse or the checkpatch one.

I think both scripts/programs are right.  There is a difference.

 static const char const * step_sels[] = { "osc", "pll2_pfd2", }; /* dup */
 static const char * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */
 static char const * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */

I think that 'type const * const' is a const pointer to const data, but
'const type const *' is just a const pointer (with duplicate).  The
patches have made the data non-const?

Fwiw,
Bill Pringlemeir.
Bill Pringlemeir - Jan. 16, 2014, 5:28 p.m.
On 16 Jan 2014, bpringlemeir@nbsps.com wrote:

>> It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
>> add warnings for static char that could be static const char).  I'm not
>> sure which warning we should ignore, the sparse or the checkpatch one.

> I think both scripts/programs are right.  There is a difference.

> static const char const * step_sels[] = { "osc", "pll2_pfd2", }; /* dup */
> static const char * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */
> static char const * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */

> I think that 'type const * const' is a const pointer to const data, but
> 'const type const *' is just a const pointer (with duplicate).  The
> patches have made the data non-const?

Sorry, that patch is correct.  It just removed the duplicate 'const',
but checkpatch is right to recommend the 'const * const' as the strings
could be put in a read-only section.
Shawn Guo - Jan. 17, 2014, 5:44 a.m.
On Thu, Jan 16, 2014 at 12:28:19PM -0500, Bill Pringlemeir wrote:
> On 16 Jan 2014, bpringlemeir@nbsps.com wrote:
> 
> >> It was added into checkpatch.pl by commit cb710ec (scripts/checkpatch.pl:
> >> add warnings for static char that could be static const char).  I'm not
> >> sure which warning we should ignore, the sparse or the checkpatch one.
> 
> > I think both scripts/programs are right.  There is a difference.
> 
> > static const char const * step_sels[] = { "osc", "pll2_pfd2", }; /* dup */
> > static const char * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */
> > static char const * const step_sels[] = { "osc", "pll2_pfd2", }; /* ok */
> 
> > I think that 'type const * const' is a const pointer to const data, but
> > 'const type const *' is just a const pointer (with duplicate).  The
> > patches have made the data non-const?
> 
> Sorry, that patch is correct.  It just removed the duplicate 'const',
> but checkpatch is right to recommend the 'const * const' as the strings
> could be put in a read-only section.

Thanks for the comment, Bill.  To fix the checkpatch warning, we will
need to change a lot of function declaration.  That's why we ignore it
for now.

Shawn

Patch

diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
index c0c4ef5..9fad29a9 100644
--- a/arch/arm/mach-imx/clk-imx6sl.c
+++ b/arch/arm/mach-imx/clk-imx6sl.c
@@ -18,27 +18,27 @@ 
 #include "clk.h"
 #include "common.h"
 
-static const char const *step_sels[]		= { "osc", "pll2_pfd2", };
-static const char const *pll1_sw_sels[]		= { "pll1_sys", "step", };
-static const char const *ocram_alt_sels[]	= { "pll2_pfd2", "pll3_pfd1", };
-static const char const *ocram_sels[]		= { "periph", "ocram_alt_sels", };
-static const char const *pre_periph_sels[]	= { "pll2_bus", "pll2_pfd2", "pll2_pfd0", "pll2_198m", };
-static const char const *periph_clk2_sels[]	= { "pll3_usb_otg", "osc", "osc", "dummy", };
-static const char const *periph2_clk2_sels[]	= { "pll3_usb_otg", "pll2_bus", };
-static const char const *periph_sels[]		= { "pre_periph_sel", "periph_clk2_podf", };
-static const char const *periph2_sels[]		= { "pre_periph2_sel", "periph2_clk2_podf", };
-static const char const *csi_lcdif_sels[]	= { "mmdc", "pll2_pfd2", "pll3_120m", "pll3_pfd1", };
-static const char const *usdhc_sels[]		= { "pll2_pfd2", "pll2_pfd0", };
-static const char const *ssi_sels[]		= { "pll3_pfd2", "pll3_pfd3", "pll4_post_div", "dummy", };
-static const char const *perclk_sels[]		= { "ipg", "osc", };
-static const char const *epdc_pxp_sels[]	= { "mmdc", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll2_pfd2", "pll3_pfd1", };
-static const char const *gpu2d_ovg_sels[]	= { "pll3_pfd1", "pll3_usb_otg", "pll2_bus", "pll2_pfd2", };
-static const char const *gpu2d_sels[]		= { "pll2_pfd2", "pll3_usb_otg", "pll3_pfd1", "pll2_bus", };
-static const char const *lcdif_pix_sels[]	= { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll3_pfd0", "pll3_pfd1", };
-static const char const *epdc_pix_sels[]	= { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll2_pfd1", "pll3_pfd1", };
-static const char const *audio_sels[]		= { "pll4_post_div", "pll3_pfd2", "pll3_pfd3", "pll3_usb_otg", };
-static const char const *ecspi_sels[]		= { "pll3_60m", "osc", };
-static const char const *uart_sels[]		= { "pll3_80m", "osc", };
+static const char *step_sels[]		= { "osc", "pll2_pfd2", };
+static const char *pll1_sw_sels[]	= { "pll1_sys", "step", };
+static const char *ocram_alt_sels[]	= { "pll2_pfd2", "pll3_pfd1", };
+static const char *ocram_sels[]		= { "periph", "ocram_alt_sels", };
+static const char *pre_periph_sels[]	= { "pll2_bus", "pll2_pfd2", "pll2_pfd0", "pll2_198m", };
+static const char *periph_clk2_sels[]	= { "pll3_usb_otg", "osc", "osc", "dummy", };
+static const char *periph2_clk2_sels[]	= { "pll3_usb_otg", "pll2_bus", };
+static const char *periph_sels[]	= { "pre_periph_sel", "periph_clk2_podf", };
+static const char *periph2_sels[]	= { "pre_periph2_sel", "periph2_clk2_podf", };
+static const char *csi_lcdif_sels[]	= { "mmdc", "pll2_pfd2", "pll3_120m", "pll3_pfd1", };
+static const char *usdhc_sels[]		= { "pll2_pfd2", "pll2_pfd0", };
+static const char *ssi_sels[]		= { "pll3_pfd2", "pll3_pfd3", "pll4_post_div", "dummy", };
+static const char *perclk_sels[]	= { "ipg", "osc", };
+static const char *epdc_pxp_sels[]	= { "mmdc", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll2_pfd2", "pll3_pfd1", };
+static const char *gpu2d_ovg_sels[]	= { "pll3_pfd1", "pll3_usb_otg", "pll2_bus", "pll2_pfd2", };
+static const char *gpu2d_sels[]		= { "pll2_pfd2", "pll3_usb_otg", "pll3_pfd1", "pll2_bus", };
+static const char *lcdif_pix_sels[]	= { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll3_pfd0", "pll3_pfd1", };
+static const char *epdc_pix_sels[]	= { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0", "pll2_pfd1", "pll3_pfd1", };
+static const char *audio_sels[]		= { "pll4_post_div", "pll3_pfd2", "pll3_pfd3", "pll3_usb_otg", };
+static const char *ecspi_sels[]		= { "pll3_60m", "osc", };
+static const char *uart_sels[]		= { "pll3_80m", "osc", };
 
 static struct clk_div_table clk_enet_ref_table[] = {
 	{ .val = 0, .div = 20, },