From: Saravana Kannan <skannan@codeaurora.org> To: Mike Turquette <mturquette@linaro.org>, Arnd Bergman <arnd.bergmann@linaro.org>, linux-arm-kernel@lists.infradead.org Cc: Andrew Lunn <andrew@lunn.ch>, Paul Walmsley <paul@pwsan.com>, Russell King <linux@arm.linux.org.uk>, Linus Walleij <linus.walleij@stericsson.com>, Stephen Boyd <sboyd@codeaurora.org>, linux-arm-msm@vger.kernel.org, Sascha Hauer <s.hauer@pengutronix.de>, Mark Brown <broonie@opensource.wolfsonmicro.com>, Magnus Damm <magnus.damm@gmail.com>, linux-kernel@vger.kernel.org, Rob Herring <rob.herring@calxeda.com>, Richard Zhao <richard.zhao@linaro.org>, Grant Likely <grant.likely@secretlab.ca>, Deepak Saxena <dsaxena@linaro.org>, Amit Kucheria <amit.kucheria@linaro.org>, Jamie Iles <jamie@jamieiles.com>, Jeremy Kerr <jeremy.kerr@canonical.com>, Thomas Gleixner <tglx@linutronix.de>, Shawn Guo <shawn.guo@freescale.com>, Saravana Kannan <skannan@codeaurora.org> Subject: Re: [PATCH] clk: Use a separate struct for holding init data. Date: Wed, 25 Apr 2012 23:28:32 -0700 [thread overview] Message-ID: <4F98EB10.2000505@codeaurora.org> (raw) In-Reply-To: <1335419936-10881-1-git-send-email-skannan@codeaurora.org> On 04/25/2012 10:58 PM, Saravana Kannan wrote: > Create a struct clk_init_data to hold all data that needs to be passed from > the platfrom specific driver to the common clock framework during clock > registration. Add a pointer to this struct inside clk_hw. > > This has several advantages: > * Completely hides struct clk from many clock platform drivers and static > clock initialization code that don't care for static initialization of > the struct clks. > * For platforms that want to do complete static initialization, it removed > the need to directly mess with the struct clk's fields while still > allowing to statically allocate struct clk. This keeps the code more > future proof even if they include clk-private.h. > * Simplifies the generic clk_register() function and allows adding optional > fields in the future without modifying the function signature. > * Simplifies the static initialization of clocks on all platforms by > removing the need for forward delcarations or convoluted macros. <SNIP> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > index 90627e4..8ea11b4 100644 > --- a/drivers/clk/clk-divider.c > +++ b/drivers/clk/clk-divider.c > @@ -167,6 +167,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c > index b555a04..cbd2462 100644 > --- a/drivers/clk/clk-fixed-rate.c > +++ b/drivers/clk/clk-fixed-rate.c > @@ -52,6 +52,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 0021616..578465e 100644 > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -119,6 +119,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c > index 6e58f11..8e97491 100644 > --- a/drivers/clk/clk-mux.c > +++ b/drivers/clk/clk-mux.c > @@ -95,6 +95,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > { I would really like to remove these functions. At least until we add device tree support where each clock is listed in device tree. At present, these functions seem to be abused more than actually being used appropriately. IMHO, these should not be used to register clocks in your probe function by calling these functions one at a time. Just declare the clocks statically and call clk_register in a loop on them. The only time I see these functions as being appropriate is when the list of clocks in your device is detected by actually probing (reading registers) HW or by parsing device tree (haven't followed it closely, but still seems to be work in progress). Regards, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
WARNING: multiple messages have this Message-ID (diff)
From: skannan@codeaurora.org (Saravana Kannan) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] clk: Use a separate struct for holding init data. Date: Wed, 25 Apr 2012 23:28:32 -0700 [thread overview] Message-ID: <4F98EB10.2000505@codeaurora.org> (raw) In-Reply-To: <1335419936-10881-1-git-send-email-skannan@codeaurora.org> On 04/25/2012 10:58 PM, Saravana Kannan wrote: > Create a struct clk_init_data to hold all data that needs to be passed from > the platfrom specific driver to the common clock framework during clock > registration. Add a pointer to this struct inside clk_hw. > > This has several advantages: > * Completely hides struct clk from many clock platform drivers and static > clock initialization code that don't care for static initialization of > the struct clks. > * For platforms that want to do complete static initialization, it removed > the need to directly mess with the struct clk's fields while still > allowing to statically allocate struct clk. This keeps the code more > future proof even if they include clk-private.h. > * Simplifies the generic clk_register() function and allows adding optional > fields in the future without modifying the function signature. > * Simplifies the static initialization of clocks on all platforms by > removing the need for forward delcarations or convoluted macros. <SNIP> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > index 90627e4..8ea11b4 100644 > --- a/drivers/clk/clk-divider.c > +++ b/drivers/clk/clk-divider.c > @@ -167,6 +167,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c > index b555a04..cbd2462 100644 > --- a/drivers/clk/clk-fixed-rate.c > +++ b/drivers/clk/clk-fixed-rate.c > @@ -52,6 +52,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 0021616..578465e 100644 > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -119,6 +119,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name, > { <SNIP> > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c > index 6e58f11..8e97491 100644 > --- a/drivers/clk/clk-mux.c > +++ b/drivers/clk/clk-mux.c > @@ -95,6 +95,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, > { I would really like to remove these functions. At least until we add device tree support where each clock is listed in device tree. At present, these functions seem to be abused more than actually being used appropriately. IMHO, these should not be used to register clocks in your probe function by calling these functions one at a time. Just declare the clocks statically and call clk_register in a loop on them. The only time I see these functions as being appropriate is when the list of clocks in your device is detected by actually probing (reading registers) HW or by parsing device tree (haven't followed it closely, but still seems to be work in progress). Regards, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
next prev parent reply other threads:[~2012-04-26 6:28 UTC|newest] Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-04-26 5:58 [PATCH] clk: Use a separate struct for holding init data Saravana Kannan 2012-04-26 5:58 ` Saravana Kannan 2012-04-26 6:28 ` Saravana Kannan [this message] 2012-04-26 6:28 ` Saravana Kannan 2012-04-26 8:42 ` Sascha Hauer 2012-04-26 8:42 ` Sascha Hauer 2012-04-26 9:36 ` Saravana Kannan 2012-04-26 9:36 ` Saravana Kannan 2012-04-26 9:36 ` Saravana Kannan 2012-04-26 9:51 ` Sascha Hauer 2012-04-26 9:51 ` Sascha Hauer 2012-04-30 19:30 ` Saravana Kannan 2012-04-30 19:30 ` Saravana Kannan 2012-04-30 22:19 ` Turquette, Mike 2012-04-30 22:19 ` Turquette, Mike 2012-04-30 22:46 ` Saravana Kannan 2012-04-30 22:46 ` Saravana Kannan 2012-05-01 8:11 ` Shawn Guo 2012-05-01 8:11 ` Shawn Guo 2012-05-01 9:13 ` Andrew Lunn 2012-05-01 9:13 ` Andrew Lunn 2012-05-01 9:13 ` Andrew Lunn 2012-05-01 17:00 ` Mark Brown 2012-05-01 17:00 ` Mark Brown 2012-05-01 17:00 ` Mark Brown 2012-05-01 18:03 ` Saravana Kannan 2012-05-01 18:03 ` Saravana Kannan 2012-05-01 18:19 ` Mark Brown 2012-05-01 18:19 ` Mark Brown 2012-05-02 1:56 ` Mike Turquette 2012-05-02 1:56 ` Mike Turquette 2012-05-02 2:14 ` Shawn Guo 2012-05-02 2:14 ` Shawn Guo 2012-05-02 5:16 ` Andrew Lunn 2012-05-02 5:16 ` Andrew Lunn 2012-05-02 5:16 ` Andrew Lunn 2012-05-02 19:19 ` Mike Turquette 2012-05-02 19:19 ` Mike Turquette 2012-05-02 19:19 ` Mike Turquette 2012-05-02 13:32 ` Arnd Bergmann 2012-05-02 13:32 ` Arnd Bergmann 2012-05-02 15:28 ` Mark Brown 2012-05-02 15:28 ` Mark Brown 2012-05-01 18:04 ` Andrew Lunn 2012-05-01 18:04 ` Andrew Lunn 2012-05-01 18:04 ` Andrew Lunn 2012-04-26 8:39 ` Sascha Hauer 2012-04-26 8:39 ` Sascha Hauer 2012-04-26 9:15 ` Saravana Kannan 2012-04-26 9:15 ` Saravana Kannan 2012-04-26 9:15 ` Saravana Kannan 2012-04-26 9:49 ` Mark Brown 2012-04-26 9:49 ` Mark Brown 2012-05-02 2:04 ` Mike Turquette 2012-05-02 2:04 ` Mike Turquette 2012-05-02 4:42 ` Saravana Kannan 2012-05-02 4:42 ` Saravana Kannan 2012-05-02 19:07 ` Mike Turquette 2012-05-02 19:07 ` Mike Turquette 2012-05-02 9:58 ` Sascha Hauer 2012-05-02 9:58 ` Sascha Hauer 2012-05-02 10:02 ` Russell King - ARM Linux 2012-05-02 10:02 ` Russell King - ARM Linux 2012-05-02 10:11 ` Sascha Hauer 2012-05-02 10:11 ` Sascha Hauer 2012-05-03 23:03 ` Domenico Andreoli 2012-05-03 23:03 ` Domenico Andreoli 2012-05-04 1:11 ` Saravana Kannan 2012-05-04 1:11 ` Saravana Kannan 2012-05-04 6:50 ` Domenico Andreoli 2012-05-04 6:50 ` Domenico Andreoli
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=4F98EB10.2000505@codeaurora.org \ --to=skannan@codeaurora.org \ --cc=amit.kucheria@linaro.org \ --cc=andrew@lunn.ch \ --cc=arnd.bergmann@linaro.org \ --cc=broonie@opensource.wolfsonmicro.com \ --cc=dsaxena@linaro.org \ --cc=grant.likely@secretlab.ca \ --cc=jamie@jamieiles.com \ --cc=jeremy.kerr@canonical.com \ --cc=linus.walleij@stericsson.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=magnus.damm@gmail.com \ --cc=mturquette@linaro.org \ --cc=paul@pwsan.com \ --cc=richard.zhao@linaro.org \ --cc=rob.herring@calxeda.com \ --cc=s.hauer@pengutronix.de \ --cc=sboyd@codeaurora.org \ --cc=shawn.guo@freescale.com \ --cc=tglx@linutronix.de \ /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.