All of lore.kernel.org
 help / color / mirror / Atom feed
From: mturquette@ti.com (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] clk: Add support for rate table based dividers
Date: Wed, 27 Jun 2012 11:06:35 -0700	[thread overview]
Message-ID: <20120627180635.GH22766@gmail.com> (raw)
In-Reply-To: <20120627162851.GT1623@pengutronix.de>

On 20120627-18:28, Sascha Hauer wrote:
> On Wed, Jun 27, 2012 at 04:31:33PM +0530, Rajendra Nayak wrote:
> > Some divider clks do not have any obvious relationship
> > between the divider and the value programmed in the
> > register. For instance, say a value of 1 could signify divide
> > by 6 and a value of 2 could signify divide by 4 etc.
> > Also there are dividers where not all values possible
> > based on the bitfield width are valid. For instance
> > a 3 bit wide bitfield can be used to program a value
> > from 0 to 7. However its possible that only 0 to 4
> > are valid values.
> > 
> > All these cases need the platform code to pass a simple
> > table of divider/value tuple, so the framework knows
> > the exact value to be written based on the divider
> > calculation and can also do better error checking.
> > 
> > This patch adds support for such rate table based
> > dividers.
> > 
> > Also since this means adding a new parameter to the
> > clk_register_divider(), update all existing users of
> > it.
> 
> I'm not sure whether we should overload the divider code with another
> type of divider. Maybe it would be better to add a new
> clk-divider-table.c for this? Just an idea, the result may or may not be
> better.
> 

Sascha,

I had the same concerns originally, but the code reuse in clk-divider.c
is pretty good.  Before this patch I have it about 200 lines, most of
which would have to be reproduced for a separate clk-rate-table.c.  So I
think marginally added complexity is OK compared to code duplication
(and duplicate bugfixes, etc).

Rajendra,

After thinking about it a bit more I still think a separate
clk_register_divider_table is needed.  Primarily this would reduce
needless churn in having to update all existing users of
clk_register_divider.  I also think that clearly separating the two
functions will make it a bit easier on folks trying to port their clocks
trees over.

Unless there is a technical reason why having two registration functions
is a bad idea, can you send a V4 with that new registration function?
I'll take it into clk-next.

Thanks,
Mike

> 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: Mike Turquette <mturquette@ti.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Rajendra Nayak <rnayak@ti.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Sascha Hauer <kernel@pengutronix.de>,
	Viresh Kumar <viresh.linux@gmail.com>
Subject: Re: [PATCH v2] clk: Add support for rate table based dividers
Date: Wed, 27 Jun 2012 11:06:35 -0700	[thread overview]
Message-ID: <20120627180635.GH22766@gmail.com> (raw)
In-Reply-To: <20120627162851.GT1623@pengutronix.de>

On 20120627-18:28, Sascha Hauer wrote:
> On Wed, Jun 27, 2012 at 04:31:33PM +0530, Rajendra Nayak wrote:
> > Some divider clks do not have any obvious relationship
> > between the divider and the value programmed in the
> > register. For instance, say a value of 1 could signify divide
> > by 6 and a value of 2 could signify divide by 4 etc.
> > Also there are dividers where not all values possible
> > based on the bitfield width are valid. For instance
> > a 3 bit wide bitfield can be used to program a value
> > from 0 to 7. However its possible that only 0 to 4
> > are valid values.
> > 
> > All these cases need the platform code to pass a simple
> > table of divider/value tuple, so the framework knows
> > the exact value to be written based on the divider
> > calculation and can also do better error checking.
> > 
> > This patch adds support for such rate table based
> > dividers.
> > 
> > Also since this means adding a new parameter to the
> > clk_register_divider(), update all existing users of
> > it.
> 
> I'm not sure whether we should overload the divider code with another
> type of divider. Maybe it would be better to add a new
> clk-divider-table.c for this? Just an idea, the result may or may not be
> better.
> 

Sascha,

I had the same concerns originally, but the code reuse in clk-divider.c
is pretty good.  Before this patch I have it about 200 lines, most of
which would have to be reproduced for a separate clk-rate-table.c.  So I
think marginally added complexity is OK compared to code duplication
(and duplicate bugfixes, etc).

Rajendra,

After thinking about it a bit more I still think a separate
clk_register_divider_table is needed.  Primarily this would reduce
needless churn in having to update all existing users of
clk_register_divider.  I also think that clearly separating the two
functions will make it a bit easier on folks trying to port their clocks
trees over.

Unless there is a technical reason why having two registration functions
is a bad idea, can you send a V4 with that new registration function?
I'll take it into clk-next.

Thanks,
Mike

> 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-06-27 18:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-27 11:01 [PATCH v2] clk: Add support for rate table based dividers Rajendra Nayak
2012-06-27 11:01 ` Rajendra Nayak
2012-06-27 11:03 ` Marc Kleine-Budde
2012-06-27 11:03   ` Marc Kleine-Budde
2012-06-27 11:10   ` Rajendra Nayak
2012-06-27 11:10     ` Rajendra Nayak
2012-06-27 16:28 ` Sascha Hauer
2012-06-27 16:28   ` Sascha Hauer
2012-06-27 18:06   ` Mike Turquette [this message]
2012-06-27 18:06     ` Mike Turquette
2012-06-29  6:31     ` Rajendra Nayak
2012-06-29  6:31       ` Rajendra Nayak

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=20120627180635.GH22766@gmail.com \
    --to=mturquette@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.