All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Saravana Kannan <skannan@codeaurora.org>, h@pengutronix.de
Cc: Mike Turquette <mturquette@linaro.org>,
	Arnd Bergman <arnd.bergmann@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	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,
	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>
Subject: Re: [PATCH] clk: Use a separate struct for holding init data.
Date: Thu, 26 Apr 2012 11:51:35 +0200	[thread overview]
Message-ID: <20120426095135.GG17184@pengutronix.de> (raw)
In-Reply-To: <4d67387a86d99cbb4e2acf68d3588b1c.squirrel@www.codeaurora.org>

On Thu, Apr 26, 2012 at 02:36:37AM -0700, Saravana Kannan wrote:
> 
> On Thu, April 26, 2012 1:42 am, Sascha Hauer wrote:
> > On Wed, Apr 25, 2012 at 11:28:32PM -0700, Saravana Kannan wrote:
> >>
> >> >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.
> >
> > I think this goes in my direction. Still exactly this abuse how you call
> > it makes me relatively independent of for example the changes you
> > introduce with your patch. Had I static initializers I would now have
> > to start a rebase orgy.
> 
> In the other email you say you have to change. Here you say, you don't
> have to change. Hopefully, you didn't have to change much

I don't have to change much, still there are changes since I also use
clk_register. I will do the changes if we agree on your patch, but I
really hope this is the last of such changes.

> -- I was aiming for that.

Thank you for this.

> If there was agreement about removing these functions, I was
> planning on helping move the current users after this patch merged.

Please understand that I begin to get frustrated. I was really happy to
see the clock framework merged in the last window. Now it's -rc4 already
and there are still patches flowing that delay the merge of SoC support.

> 
> I think in the long run this will result in less changes for you and more
> readable code. If clk_register() adds another optional param, you can't
> get around that without having to write more wrapper functions or changing
> any existing ones you might have. But with this struct, the common clock
> code can be written in a way so that the a value of 0 for the new param
> defaults to the behavior that was there before the param was added.
> 
> Something to think about: With these wrapper calls, one would do a lot of
> kalloc and copying of small items when one knows at compile time what the
> clocks are going to be.

We do not know during compile what clocks will be since we do not know
at compile time which SoC we are going to run on. Statically allocated
clocks (which are going to be used directly instead of making copies)
may be fine for someone do a build for one SoC only, but I think our
goal is to build a multi SoC and eventually even a multi architecture
kernel.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

WARNING: multiple messages have this Message-ID (diff)
From: s.hauer@pengutronix.de (Sascha Hauer)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: Use a separate struct for holding init data.
Date: Thu, 26 Apr 2012 11:51:35 +0200	[thread overview]
Message-ID: <20120426095135.GG17184@pengutronix.de> (raw)
In-Reply-To: <4d67387a86d99cbb4e2acf68d3588b1c.squirrel@www.codeaurora.org>

On Thu, Apr 26, 2012 at 02:36:37AM -0700, Saravana Kannan wrote:
> 
> On Thu, April 26, 2012 1:42 am, Sascha Hauer wrote:
> > On Wed, Apr 25, 2012 at 11:28:32PM -0700, Saravana Kannan wrote:
> >>
> >> >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.
> >
> > I think this goes in my direction. Still exactly this abuse how you call
> > it makes me relatively independent of for example the changes you
> > introduce with your patch. Had I static initializers I would now have
> > to start a rebase orgy.
> 
> In the other email you say you have to change. Here you say, you don't
> have to change. Hopefully, you didn't have to change much

I don't have to change much, still there are changes since I also use
clk_register. I will do the changes if we agree on your patch, but I
really hope this is the last of such changes.

> -- I was aiming for that.

Thank you for this.

> If there was agreement about removing these functions, I was
> planning on helping move the current users after this patch merged.

Please understand that I begin to get frustrated. I was really happy to
see the clock framework merged in the last window. Now it's -rc4 already
and there are still patches flowing that delay the merge of SoC support.

> 
> I think in the long run this will result in less changes for you and more
> readable code. If clk_register() adds another optional param, you can't
> get around that without having to write more wrapper functions or changing
> any existing ones you might have. But with this struct, the common clock
> code can be written in a way so that the a value of 0 for the new param
> defaults to the behavior that was there before the param was added.
> 
> Something to think about: With these wrapper calls, one would do a lot of
> kalloc and copying of small items when one knows at compile time what the
> clocks are going to be.

We do not know during compile what clocks will be since we do not know
at compile time which SoC we are going to run on. Statically allocated
clocks (which are going to be used directly instead of making copies)
may be fine for someone do a build for one SoC only, but I think our
goal is to build a multi SoC and eventually even a multi architecture
kernel.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2012-04-26  9:51 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
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 [this message]
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=20120426095135.GG17184@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --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=h@pengutronix.de \
    --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=sboyd@codeaurora.org \
    --cc=shawn.guo@freescale.com \
    --cc=skannan@codeaurora.org \
    --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: link
Be 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.