[PATCH v3 RESEND] clk: sifive: Fix W=1 kernel build warning

[Zong Li]

On Mon, Jan 10, 2022 at 5:50 PM Lee Jones <lee.jones at linaro.org> wrote:
>
> Please improve the subject line.
>
> If this is a straight revert, the subject line should reflect that.
>
> If not, you need to give us specific information regarding the purpose
> of this patch.  Please read the Git log for better, more forthcoming
> examples.
>

It seems to me that this patch is not a straight revert, it provides
another way to fix the original build warnings, just like
'487dc7bb6a0c' tried to do. I guess the commit message has described
what the original warnings is and what the root cause is, it also
mentioned what is changed in this patch. I'm a bit confused whether we
need to add fixes tag, it looks like that it might cause some
misunderstanding?

> On Fri, 07 Jan 2022, Zong Li wrote:
>
> > This commit reverts commit 487dc7bb6a0c ("clk: sifive: fu540-prci:
> > Declare static const variable 'prci_clk_fu540' where it's used").
> > For fixing W=1 kernel build warning(s) about ‘prci_clk_fu540’ defined
> > but not used [-Wunused-const-variable=], the problem is that the C file
> > of fu540 and fu740 doesn't use these variables, but they includes the
> > header files.
>
> What exactly does this patch fix?  Does it fix a build warning?
>
> If so, please provide the line you are seeing.
>
> > We could refine the code by moving the definition of these
> > variables into fu540 and fu740 implementation respectively instead of
> > common core code, then we could still separate the SoCs-dependent data
> > in their own implementation.
> >
> > Fixes: 487dc7bb6a0c ("clk: sifive: fu540-prci: Declare static
> > const variable 'prci_clk_fu540' where it's used")
>
> This should be on one line.
>
> What exactly does it fix though?  Please provide more details.
>
> What about the warning that this patch was designed to fix?  Doesn't
> that return after this patch has been applied?
>
> > Signed-off-by: Zong Li <zong.li at sifive.com>
> >
> > ---
> > Changed in v3:
> >  - Rebase on v5.16-rc8
> >  - Add fixes tag
> >
> > Changed in v2:
> >  - Move definition of variable to C file from header
> > ---
> >  drivers/clk/sifive/fu540-prci.c  |  6 +++++-
> >  drivers/clk/sifive/fu540-prci.h  |  6 +-----
> >  drivers/clk/sifive/fu740-prci.c  |  6 +++++-
> >  drivers/clk/sifive/fu740-prci.h  | 11 +----------
> >  drivers/clk/sifive/sifive-prci.c |  5 -----
> >  5 files changed, 12 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
> > index 29bab915003c..5568bc26e36f 100644
> > --- a/drivers/clk/sifive/fu540-prci.c
> > +++ b/drivers/clk/sifive/fu540-prci.c
> > @@ -20,7 +20,6 @@
> >
> >  #include <dt-bindings/clock/sifive-fu540-prci.h>
> >
> > -#include "fu540-prci.h"
> >  #include "sifive-prci.h"
> >
> >  /* PRCI integration data for each WRPLL instance */
> > @@ -87,3 +86,8 @@ struct __prci_clock __prci_init_clocks_fu540[] = {
> >               .ops = &sifive_fu540_prci_tlclksel_clk_ops,
> >       },
> >  };
> > +
> > +struct prci_clk_desc prci_clk_fu540 = {
> > +     .clks = __prci_init_clocks_fu540,
> > +     .num_clks = ARRAY_SIZE(__prci_init_clocks_fu540),
> > +};
> > diff --git a/drivers/clk/sifive/fu540-prci.h b/drivers/clk/sifive/fu540-prci.h
> > index c220677dc010..931d6cad8c1c 100644
> > --- a/drivers/clk/sifive/fu540-prci.h
> > +++ b/drivers/clk/sifive/fu540-prci.h
> > @@ -7,10 +7,6 @@
> >  #ifndef __SIFIVE_CLK_FU540_PRCI_H
> >  #define __SIFIVE_CLK_FU540_PRCI_H
> >
> > -#include "sifive-prci.h"
> > -
> > -#define NUM_CLOCK_FU540      4
> > -
> > -extern struct __prci_clock __prci_init_clocks_fu540[NUM_CLOCK_FU540];
> > +extern struct prci_clk_desc prci_clk_fu540;
> >
> >  #endif /* __SIFIVE_CLK_FU540_PRCI_H */
> > diff --git a/drivers/clk/sifive/fu740-prci.c b/drivers/clk/sifive/fu740-prci.c
> > index 53f6e00a03b9..0ade3dcd24ed 100644
> > --- a/drivers/clk/sifive/fu740-prci.c
> > +++ b/drivers/clk/sifive/fu740-prci.c
> > @@ -8,7 +8,6 @@
> >
> >  #include <dt-bindings/clock/sifive-fu740-prci.h>
> >
> > -#include "fu540-prci.h"
> >  #include "sifive-prci.h"
> >
> >  /* PRCI integration data for each WRPLL instance */
> > @@ -132,3 +131,8 @@ struct __prci_clock __prci_init_clocks_fu740[] = {
> >               .ops = &sifive_fu740_prci_pcie_aux_clk_ops,
> >       },
> >  };
> > +
> > +struct prci_clk_desc prci_clk_fu740 = {
> > +     .clks = __prci_init_clocks_fu740,
> > +     .num_clks = ARRAY_SIZE(__prci_init_clocks_fu740),
> > +};
> > diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h
> > index 511a0bf7ba2b..5bc0e18f058c 100644
> > --- a/drivers/clk/sifive/fu740-prci.h
> > +++ b/drivers/clk/sifive/fu740-prci.h
> > @@ -7,15 +7,6 @@
> >  #ifndef __SIFIVE_CLK_FU740_PRCI_H
> >  #define __SIFIVE_CLK_FU740_PRCI_H
> >
> > -#include "sifive-prci.h"
> > -
> > -#define NUM_CLOCK_FU740      9
> > -
> > -extern struct __prci_clock __prci_init_clocks_fu740[NUM_CLOCK_FU740];
> > -
> > -static const struct prci_clk_desc prci_clk_fu740 = {
> > -     .clks = __prci_init_clocks_fu740,
> > -     .num_clks = ARRAY_SIZE(__prci_init_clocks_fu740),
> > -};
> > +extern struct prci_clk_desc prci_clk_fu740;
> >
> >  #endif /* __SIFIVE_CLK_FU740_PRCI_H */
> > diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c
> > index 80a288c59e56..916d2fc28b9c 100644
> > --- a/drivers/clk/sifive/sifive-prci.c
> > +++ b/drivers/clk/sifive/sifive-prci.c
> > @@ -12,11 +12,6 @@
> >  #include "fu540-prci.h"
> >  #include "fu740-prci.h"
> >
> > -static const struct prci_clk_desc prci_clk_fu540 = {
> > -     .clks = __prci_init_clocks_fu540,
> > -     .num_clks = ARRAY_SIZE(__prci_init_clocks_fu540),
> > -};
> > -
> >  /*
> >   * Private functions
> >   */
>
> --
> Lee Jones [李琼斯]
> Principal Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog