| Message ID | 1389766774-16661-1-git-send-email-Ying.Liu@freescale.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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.
> 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.
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.
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
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, },
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(-)