From: Zong Li <zong.li@sifive.com> To: Stephen Boyd <sboyd@kernel.org> Cc: Albert Ou <aou@eecs.berkeley.edu>, linux-clk@vger.kernel.org, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org>, Michael Turquette <mturquette@baylibre.com>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Yash Shah <yash.shah@sifive.com> Subject: Re: [PATCH 4/4] clk: sifive: Refactor __prci_clock array by using macro Date: Thu, 5 Nov 2020 15:16:30 +0800 [thread overview] Message-ID: <CANXhq0rbkRc3kKAPsbR=Tg5uqvazOqSZwzEEOp7std_Tf5LhHA@mail.gmail.com> (raw) In-Reply-To: <160454464591.3965362.9361884545184336266@swboyd.mtv.corp.google.com> On Thu, Nov 5, 2020 at 10:50 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Zong Li (2020-10-16 02:18:26) > > Refactor code by using DEFINE_PRCI_CLOCK to define each clock > > and reduce duplicate code. > > What is duplicate? Sorry for unclear description, actually, I want to say that we can remove the repeating code about initializing the data member of structure. > > > > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > drivers/clk/sifive/fu540-prci.c | 38 ++++++---------- > > drivers/clk/sifive/fu540-prci.h | 2 +- > > drivers/clk/sifive/fu740-prci.c | 74 ++++++++++++-------------------- > > drivers/clk/sifive/fu740-prci.h | 2 +- > > drivers/clk/sifive/sifive-prci.c | 2 +- > > drivers/clk/sifive/sifive-prci.h | 10 ++++- > > 6 files changed, 53 insertions(+), 75 deletions(-) > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > index 840b97bfff85..d43b9a9984f6 100644 > > --- a/drivers/clk/sifive/fu540-prci.c > > +++ b/drivers/clk/sifive/fu540-prci.c > > @@ -54,29 +54,19 @@ static const struct clk_ops sifive_fu540_prci_tlclksel_clk_ops = { > > .recalc_rate = sifive_prci_tlclksel_recalc_rate, > > }; > > > > +DEFINE_PRCI_CLOCK(fu540, corepll, hfclk, > > + &sifive_fu540_prci_wrpll_clk_ops, &__prci_corepll_data); > > +DEFINE_PRCI_CLOCK(fu540, ddrpll, hfclk, > > + &sifive_fu540_prci_wrpll_ro_clk_ops, &__prci_ddrpll_data); > > +DEFINE_PRCI_CLOCK(fu540, gemgxlpll, hfclk, > > + &sifive_fu540_prci_wrpll_clk_ops, &__prci_gemgxlpll_data); > > +DEFINE_PRCI_CLOCK(fu540, tlclk, corepll, > > + &sifive_fu540_prci_tlclksel_clk_ops, NULL); > > Readability looks to decrease with this change. Why should all us code > reviewers suffer because the code author wants to type a few less > characters? Named initializers are useful so we don't have to hold each > macro argument in our head and map it to the struct member that is being > initialized. Ok, as you mentioned, macro reduce the readability, let me remove this change in the next version. > > > + > > /* List of clock controls provided by the PRCI */ > > -struct __prci_clock __prci_init_clocks_fu540[] = { > > - [PRCI_CLK_COREPLL] = { > > - .name = "corepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_clk_ops, > > - .pwd = &__prci_corepll_data, > > - }, > > - [PRCI_CLK_DDRPLL] = { > > - .name = "ddrpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_ro_clk_ops, > > - .pwd = &__prci_ddrpll_data, > > - }, > > - [PRCI_CLK_GEMGXLPLL] = { > > - .name = "gemgxlpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_clk_ops, > > - .pwd = &__prci_gemgxlpll_data, > > - }, > > - [PRCI_CLK_TLCLK] = { > > - .name = "tlclk", > > - .parent_name = "corepll", > > - .ops = &sifive_fu540_prci_tlclksel_clk_ops, > > - }, > > +struct __prci_clock *__prci_init_clocks_fu540[] = { > > + [PRCI_CLK_COREPLL] = &fu540_corepll, > > + [PRCI_CLK_DDRPLL] = &fu540_ddrpll, > > + [PRCI_CLK_GEMGXLPLL] = &fu540_gemgxlpll, > > + [PRCI_CLK_TLCLK] = &fu540_tlclk, > > }; > > diff --git a/drivers/clk/sifive/fu540-prci.h b/drivers/clk/sifive/fu540-prci.h > > index c8271efa7bdc..281200cd8848 100644 > > --- a/drivers/clk/sifive/fu540-prci.h > > +++ b/drivers/clk/sifive/fu540-prci.h > > @@ -11,7 +11,7 @@ > > > > #define NUM_CLOCK_FU540 4 > > > > -extern struct __prci_clock __prci_init_clocks_fu540[NUM_CLOCK_FU540]; > > +extern struct __prci_clock *__prci_init_clocks_fu540[NUM_CLOCK_FU540]; > > > > static const struct prci_clk_desc prci_clk_fu540 = { > > .clks = __prci_init_clocks_fu540, > > diff --git a/drivers/clk/sifive/fu740-prci.c b/drivers/clk/sifive/fu740-prci.c > > index 3b87e273c3eb..676cad2c3886 100644 > > --- a/drivers/clk/sifive/fu740-prci.c > > +++ b/drivers/clk/sifive/fu740-prci.c > > @@ -71,52 +71,32 @@ static const struct clk_ops sifive_fu740_prci_hfpclkplldiv_clk_ops = { > > .recalc_rate = sifive_prci_hfpclkplldiv_recalc_rate, > > }; > > > > + > > +DEFINE_PRCI_CLOCK(fu740, corepll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_corepll_data); > > +DEFINE_PRCI_CLOCK(fu740, ddrpll, hfclk, > > + &sifive_fu740_prci_wrpll_ro_clk_ops, &__prci_ddrpll_data); > > +DEFINE_PRCI_CLOCK(fu740, gemgxlpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_gemgxlpll_data); > > +DEFINE_PRCI_CLOCK(fu740, dvfscorepll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_dvfscorepll_data); > > +DEFINE_PRCI_CLOCK(fu740, hfpclkpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_hfpclkpll_data); > > +DEFINE_PRCI_CLOCK(fu740, cltxpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_cltxpll_data); > > +DEFINE_PRCI_CLOCK(fu740, tlclk, corepll, > > + &sifive_fu740_prci_tlclksel_clk_ops, NULL); > > +DEFINE_PRCI_CLOCK(fu740, pclk, hfpclkpll, > > + &sifive_fu740_prci_hfpclkplldiv_clk_ops, NULL); > > + > > /* List of clock controls provided by the PRCI */ > > -struct __prci_clock __prci_init_clocks_fu740[] = { > > - [PRCI_CLK_COREPLL] = { > > - .name = "corepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_corepll_data, > > - }, > > - [PRCI_CLK_DDRPLL] = { > > - .name = "ddrpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_ro_clk_ops, > > - .pwd = &__prci_ddrpll_data, > > - }, > > - [PRCI_CLK_GEMGXLPLL] = { > > - .name = "gemgxlpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_gemgxlpll_data, > > - }, > > - [PRCI_CLK_DVFSCOREPLL] = { > > - .name = "dvfscorepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_dvfscorepll_data, > > - }, > > - [PRCI_CLK_HFPCLKPLL] = { > > - .name = "hfpclkpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_hfpclkpll_data, > > - }, > > - [PRCI_CLK_CLTXPLL] = { > > - .name = "cltxpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_cltxpll_data, > > - }, > > - [PRCI_CLK_TLCLK] = { > > - .name = "tlclk", > > - .parent_name = "corepll", > > - .ops = &sifive_fu740_prci_tlclksel_clk_ops, > > - }, > > - [PRCI_CLK_PCLK] = { > > - .name = "pclk", > > - .parent_name = "hfpclkpll", > > - .ops = &sifive_fu740_prci_hfpclkplldiv_clk_ops, > > - }, > > +struct __prci_clock *__prci_init_clocks_fu740[] = { > > + [PRCI_CLK_COREPLL] = &fu740_corepll, > > + [PRCI_CLK_DDRPLL] = &fu740_ddrpll, > > + [PRCI_CLK_GEMGXLPLL] = &fu740_gemgxlpll, > > + [PRCI_CLK_DVFSCOREPLL] = &fu740_dvfscorepll, > > + [PRCI_CLK_HFPCLKPLL] = &fu740_hfpclkpll, > > + [PRCI_CLK_CLTXPLL] = &fu740_cltxpll, > > + [PRCI_CLK_TLCLK] = &fu740_tlclk, > > + [PRCI_CLK_PCLK] = &fu740_pclk, > > }; > > I suppose this is fine and then non-macro structs above this array of > pointers. > > > diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h > > index 13ef971f7764..3f03295f719a 100644 > > --- a/drivers/clk/sifive/fu740-prci.h > > +++ b/drivers/clk/sifive/fu740-prci.h > > @@ -11,7 +11,7 @@ > > > > #define NUM_CLOCK_FU740 8 > > > > -extern struct __prci_clock __prci_init_clocks_fu740[NUM_CLOCK_FU740]; > > +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, > > diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c > > index 4098dbc5881a..2ef3f9f91b33 100644 > > --- a/drivers/clk/sifive/sifive-prci.c > > +++ b/drivers/clk/sifive/sifive-prci.c > > @@ -431,7 +431,7 @@ static int __prci_register_clocks(struct device *dev, struct __prci_data *pd, > > > > /* Register PLLs */ > > for (i = 0; i < desc->num_clks; ++i) { > > - pic = &(desc->clks[i]); > > + pic = desc->clks[i]; > > This is related how? > > > > > init.name = pic->name; > > init.parent_names = &pic->parent_name; > > diff --git a/drivers/clk/sifive/sifive-prci.h b/drivers/clk/sifive/sifive-prci.h > > index bc0646bc9c3e..e6c9f72e20de 100644 > > --- a/drivers/clk/sifive/sifive-prci.h > > +++ b/drivers/clk/sifive/sifive-prci.h > > @@ -253,6 +253,14 @@ struct __prci_clock { > > struct __prci_data *pd; > > }; > > > > +#define DEFINE_PRCI_CLOCK(_platform, _name, _parent, _ops, _pwd) \ > > + static struct __prci_clock _platform##_##_name = { \ > > + .name = #_name, \ > > + .parent_name = #_parent, \ > > + .ops = _ops, \ > > + .pwd = _pwd, \ > > + } \ > > + > > #define clk_hw_to_prci_clock(pwd) container_of(pwd, struct __prci_clock, hw) > > > > /* > > @@ -261,7 +269,7 @@ struct __prci_clock { > > * @num_clks: the number of element of clks > > */ > > struct prci_clk_desc { > > - struct __prci_clock *clks; > > + struct __prci_clock **clks; > > Huh? Nothing in the commit text mentions this. > Because I introduce the macro in this patch, so the type of array __prci_init_clocks_fuXXX are changed to pointer which point to __prci_clock, the related use need to be changed as well. It would be recover due to discarding this patch. > > size_t num_clks; > > };
WARNING: multiple messages have this Message-ID (diff)
From: Zong Li <zong.li@sifive.com> To: Stephen Boyd <sboyd@kernel.org> Cc: Albert Ou <aou@eecs.berkeley.edu>, Michael Turquette <mturquette@baylibre.com>, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, Yash Shah <yash.shah@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, linux-riscv <linux-riscv@lists.infradead.org>, linux-clk@vger.kernel.org Subject: Re: [PATCH 4/4] clk: sifive: Refactor __prci_clock array by using macro Date: Thu, 5 Nov 2020 15:16:30 +0800 [thread overview] Message-ID: <CANXhq0rbkRc3kKAPsbR=Tg5uqvazOqSZwzEEOp7std_Tf5LhHA@mail.gmail.com> (raw) In-Reply-To: <160454464591.3965362.9361884545184336266@swboyd.mtv.corp.google.com> On Thu, Nov 5, 2020 at 10:50 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Zong Li (2020-10-16 02:18:26) > > Refactor code by using DEFINE_PRCI_CLOCK to define each clock > > and reduce duplicate code. > > What is duplicate? Sorry for unclear description, actually, I want to say that we can remove the repeating code about initializing the data member of structure. > > > > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > drivers/clk/sifive/fu540-prci.c | 38 ++++++---------- > > drivers/clk/sifive/fu540-prci.h | 2 +- > > drivers/clk/sifive/fu740-prci.c | 74 ++++++++++++-------------------- > > drivers/clk/sifive/fu740-prci.h | 2 +- > > drivers/clk/sifive/sifive-prci.c | 2 +- > > drivers/clk/sifive/sifive-prci.h | 10 ++++- > > 6 files changed, 53 insertions(+), 75 deletions(-) > > > > diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c > > index 840b97bfff85..d43b9a9984f6 100644 > > --- a/drivers/clk/sifive/fu540-prci.c > > +++ b/drivers/clk/sifive/fu540-prci.c > > @@ -54,29 +54,19 @@ static const struct clk_ops sifive_fu540_prci_tlclksel_clk_ops = { > > .recalc_rate = sifive_prci_tlclksel_recalc_rate, > > }; > > > > +DEFINE_PRCI_CLOCK(fu540, corepll, hfclk, > > + &sifive_fu540_prci_wrpll_clk_ops, &__prci_corepll_data); > > +DEFINE_PRCI_CLOCK(fu540, ddrpll, hfclk, > > + &sifive_fu540_prci_wrpll_ro_clk_ops, &__prci_ddrpll_data); > > +DEFINE_PRCI_CLOCK(fu540, gemgxlpll, hfclk, > > + &sifive_fu540_prci_wrpll_clk_ops, &__prci_gemgxlpll_data); > > +DEFINE_PRCI_CLOCK(fu540, tlclk, corepll, > > + &sifive_fu540_prci_tlclksel_clk_ops, NULL); > > Readability looks to decrease with this change. Why should all us code > reviewers suffer because the code author wants to type a few less > characters? Named initializers are useful so we don't have to hold each > macro argument in our head and map it to the struct member that is being > initialized. Ok, as you mentioned, macro reduce the readability, let me remove this change in the next version. > > > + > > /* List of clock controls provided by the PRCI */ > > -struct __prci_clock __prci_init_clocks_fu540[] = { > > - [PRCI_CLK_COREPLL] = { > > - .name = "corepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_clk_ops, > > - .pwd = &__prci_corepll_data, > > - }, > > - [PRCI_CLK_DDRPLL] = { > > - .name = "ddrpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_ro_clk_ops, > > - .pwd = &__prci_ddrpll_data, > > - }, > > - [PRCI_CLK_GEMGXLPLL] = { > > - .name = "gemgxlpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu540_prci_wrpll_clk_ops, > > - .pwd = &__prci_gemgxlpll_data, > > - }, > > - [PRCI_CLK_TLCLK] = { > > - .name = "tlclk", > > - .parent_name = "corepll", > > - .ops = &sifive_fu540_prci_tlclksel_clk_ops, > > - }, > > +struct __prci_clock *__prci_init_clocks_fu540[] = { > > + [PRCI_CLK_COREPLL] = &fu540_corepll, > > + [PRCI_CLK_DDRPLL] = &fu540_ddrpll, > > + [PRCI_CLK_GEMGXLPLL] = &fu540_gemgxlpll, > > + [PRCI_CLK_TLCLK] = &fu540_tlclk, > > }; > > diff --git a/drivers/clk/sifive/fu540-prci.h b/drivers/clk/sifive/fu540-prci.h > > index c8271efa7bdc..281200cd8848 100644 > > --- a/drivers/clk/sifive/fu540-prci.h > > +++ b/drivers/clk/sifive/fu540-prci.h > > @@ -11,7 +11,7 @@ > > > > #define NUM_CLOCK_FU540 4 > > > > -extern struct __prci_clock __prci_init_clocks_fu540[NUM_CLOCK_FU540]; > > +extern struct __prci_clock *__prci_init_clocks_fu540[NUM_CLOCK_FU540]; > > > > static const struct prci_clk_desc prci_clk_fu540 = { > > .clks = __prci_init_clocks_fu540, > > diff --git a/drivers/clk/sifive/fu740-prci.c b/drivers/clk/sifive/fu740-prci.c > > index 3b87e273c3eb..676cad2c3886 100644 > > --- a/drivers/clk/sifive/fu740-prci.c > > +++ b/drivers/clk/sifive/fu740-prci.c > > @@ -71,52 +71,32 @@ static const struct clk_ops sifive_fu740_prci_hfpclkplldiv_clk_ops = { > > .recalc_rate = sifive_prci_hfpclkplldiv_recalc_rate, > > }; > > > > + > > +DEFINE_PRCI_CLOCK(fu740, corepll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_corepll_data); > > +DEFINE_PRCI_CLOCK(fu740, ddrpll, hfclk, > > + &sifive_fu740_prci_wrpll_ro_clk_ops, &__prci_ddrpll_data); > > +DEFINE_PRCI_CLOCK(fu740, gemgxlpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_gemgxlpll_data); > > +DEFINE_PRCI_CLOCK(fu740, dvfscorepll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_dvfscorepll_data); > > +DEFINE_PRCI_CLOCK(fu740, hfpclkpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_hfpclkpll_data); > > +DEFINE_PRCI_CLOCK(fu740, cltxpll, hfclk, > > + &sifive_fu740_prci_wrpll_clk_ops, &__prci_cltxpll_data); > > +DEFINE_PRCI_CLOCK(fu740, tlclk, corepll, > > + &sifive_fu740_prci_tlclksel_clk_ops, NULL); > > +DEFINE_PRCI_CLOCK(fu740, pclk, hfpclkpll, > > + &sifive_fu740_prci_hfpclkplldiv_clk_ops, NULL); > > + > > /* List of clock controls provided by the PRCI */ > > -struct __prci_clock __prci_init_clocks_fu740[] = { > > - [PRCI_CLK_COREPLL] = { > > - .name = "corepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_corepll_data, > > - }, > > - [PRCI_CLK_DDRPLL] = { > > - .name = "ddrpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_ro_clk_ops, > > - .pwd = &__prci_ddrpll_data, > > - }, > > - [PRCI_CLK_GEMGXLPLL] = { > > - .name = "gemgxlpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_gemgxlpll_data, > > - }, > > - [PRCI_CLK_DVFSCOREPLL] = { > > - .name = "dvfscorepll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_dvfscorepll_data, > > - }, > > - [PRCI_CLK_HFPCLKPLL] = { > > - .name = "hfpclkpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_hfpclkpll_data, > > - }, > > - [PRCI_CLK_CLTXPLL] = { > > - .name = "cltxpll", > > - .parent_name = "hfclk", > > - .ops = &sifive_fu740_prci_wrpll_clk_ops, > > - .pwd = &__prci_cltxpll_data, > > - }, > > - [PRCI_CLK_TLCLK] = { > > - .name = "tlclk", > > - .parent_name = "corepll", > > - .ops = &sifive_fu740_prci_tlclksel_clk_ops, > > - }, > > - [PRCI_CLK_PCLK] = { > > - .name = "pclk", > > - .parent_name = "hfpclkpll", > > - .ops = &sifive_fu740_prci_hfpclkplldiv_clk_ops, > > - }, > > +struct __prci_clock *__prci_init_clocks_fu740[] = { > > + [PRCI_CLK_COREPLL] = &fu740_corepll, > > + [PRCI_CLK_DDRPLL] = &fu740_ddrpll, > > + [PRCI_CLK_GEMGXLPLL] = &fu740_gemgxlpll, > > + [PRCI_CLK_DVFSCOREPLL] = &fu740_dvfscorepll, > > + [PRCI_CLK_HFPCLKPLL] = &fu740_hfpclkpll, > > + [PRCI_CLK_CLTXPLL] = &fu740_cltxpll, > > + [PRCI_CLK_TLCLK] = &fu740_tlclk, > > + [PRCI_CLK_PCLK] = &fu740_pclk, > > }; > > I suppose this is fine and then non-macro structs above this array of > pointers. > > > diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h > > index 13ef971f7764..3f03295f719a 100644 > > --- a/drivers/clk/sifive/fu740-prci.h > > +++ b/drivers/clk/sifive/fu740-prci.h > > @@ -11,7 +11,7 @@ > > > > #define NUM_CLOCK_FU740 8 > > > > -extern struct __prci_clock __prci_init_clocks_fu740[NUM_CLOCK_FU740]; > > +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, > > diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c > > index 4098dbc5881a..2ef3f9f91b33 100644 > > --- a/drivers/clk/sifive/sifive-prci.c > > +++ b/drivers/clk/sifive/sifive-prci.c > > @@ -431,7 +431,7 @@ static int __prci_register_clocks(struct device *dev, struct __prci_data *pd, > > > > /* Register PLLs */ > > for (i = 0; i < desc->num_clks; ++i) { > > - pic = &(desc->clks[i]); > > + pic = desc->clks[i]; > > This is related how? > > > > > init.name = pic->name; > > init.parent_names = &pic->parent_name; > > diff --git a/drivers/clk/sifive/sifive-prci.h b/drivers/clk/sifive/sifive-prci.h > > index bc0646bc9c3e..e6c9f72e20de 100644 > > --- a/drivers/clk/sifive/sifive-prci.h > > +++ b/drivers/clk/sifive/sifive-prci.h > > @@ -253,6 +253,14 @@ struct __prci_clock { > > struct __prci_data *pd; > > }; > > > > +#define DEFINE_PRCI_CLOCK(_platform, _name, _parent, _ops, _pwd) \ > > + static struct __prci_clock _platform##_##_name = { \ > > + .name = #_name, \ > > + .parent_name = #_parent, \ > > + .ops = _ops, \ > > + .pwd = _pwd, \ > > + } \ > > + > > #define clk_hw_to_prci_clock(pwd) container_of(pwd, struct __prci_clock, hw) > > > > /* > > @@ -261,7 +269,7 @@ struct __prci_clock { > > * @num_clks: the number of element of clks > > */ > > struct prci_clk_desc { > > - struct __prci_clock *clks; > > + struct __prci_clock **clks; > > Huh? Nothing in the commit text mentions this. > Because I introduce the macro in this patch, so the type of array __prci_init_clocks_fuXXX are changed to pointer which point to __prci_clock, the related use need to be changed as well. It would be recover due to discarding this patch. > > size_t num_clks; > > }; _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2020-11-05 7:16 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-16 9:18 [PATCH 0/4] clk: add driver for the SiFive FU740 Zong Li 2020-10-16 9:18 ` Zong Li 2020-10-16 9:18 ` [PATCH 1/4] clk: sifive: Extract prci core to common base Zong Li 2020-10-16 9:18 ` Zong Li 2020-11-05 2:45 ` Stephen Boyd 2020-11-05 2:45 ` Stephen Boyd 2020-11-05 7:22 ` Zong Li 2020-11-05 7:22 ` Zong Li 2020-11-05 9:19 ` Andreas Schwab 2020-11-05 9:19 ` Andreas Schwab 2020-11-05 9:30 ` Zong Li 2020-11-05 9:30 ` Zong Li 2020-10-16 9:18 ` [PATCH 2/4] clk: sifive: Use common name for prci configuration Zong Li 2020-10-16 9:18 ` Zong Li 2020-11-05 2:46 ` Stephen Boyd 2020-11-05 2:46 ` Stephen Boyd 2020-11-05 6:56 ` Zong Li 2020-11-05 6:56 ` Zong Li 2020-11-05 18:54 ` Palmer Dabbelt 2020-11-05 18:54 ` Palmer Dabbelt 2020-10-16 9:18 ` [PATCH 3/4] clk: sifive: Add a driver for the SiFive FU740 PRCI IP block Zong Li 2020-10-16 9:18 ` Zong Li 2020-10-16 9:18 ` [PATCH 4/4] clk: sifive: Refactor __prci_clock array by using macro Zong Li 2020-10-16 9:18 ` Zong Li 2020-11-05 2:50 ` Stephen Boyd 2020-11-05 2:50 ` Stephen Boyd 2020-11-05 7:16 ` Zong Li [this message] 2020-11-05 7:16 ` Zong Li 2020-10-16 14:17 ` [PATCH 0/4] clk: add driver for the SiFive FU740 Sean Anderson 2020-10-16 14:17 ` Sean Anderson 2020-10-16 14:56 ` Zong Li 2020-10-16 14:56 ` Zong Li 2020-11-04 6:37 ` Zong Li 2020-11-04 6:37 ` Zong Li
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CANXhq0rbkRc3kKAPsbR=Tg5uqvazOqSZwzEEOp7std_Tf5LhHA@mail.gmail.com' \ --to=zong.li@sifive.com \ --cc=aou@eecs.berkeley.edu \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=mturquette@baylibre.com \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=sboyd@kernel.org \ --cc=yash.shah@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.