Date:	Wed, 14 Mar 2012 15:13:28 -0700
From:	"Turquette, Mike" <mturquette@...com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Sascha Hauer <s.hauer@...gutronix.de>,
	Russell King <linux@....linux.org.uk>,
	Andrew Lunn <andrew@...n.ch>, linaro-dev@...ts.linaro.org,
	Grant Likely <grant.likely@...retlab.ca>,
	Saravana Kannan <skannan@...eaurora.org>,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	Magnus Damm <magnus.damm@...il.com>,
	Deepak Saxena <dsaxena@...aro.org>,
	linux-arm-kernel@...ts.infradead.org,
	Arnd Bergman <arnd.bergmann@...aro.org>, patches@...aro.org,
	Rob Herring <rob.herring@...xeda.com>,
	Richard Zhao <richard.zhao@...aro.org>,
	Shawn Guo <shawn.guo@...escale.com>,
	Paul Walmsley <paul@...an.com>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-kernel@...r.kernel.org,
	Amit Kucheria <amit.kucheria@...aro.org>
Subject: Re: [PATCH v5 3/4] clk: introduce the common clock framework

On Wed, Mar 14, 2012 at 2:28 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> On Wed, 14 Mar 2012, Turquette, Mike wrote:
>
> Could you folks please trim your replies? It's annoying to page down a
> gazillion of lines to find the gist.

Sure.  My mailer does this for me so I forget to do it sometimes...

>> On Wed, Mar 14, 2012 at 1:48 AM, Sascha Hauer <s.hauer@...gutronix.de> wrote:
>> >> Also, do you forsee needing hole in parent_names for any reason other
>> >> than described above?
>> >
>> > I need it only for the case where a some values in the mux are marked as
>> > reserved in the datasheet or we simply do not have the corresponding
>> > clock in our tree (yet). We could also say that NULL pointers are not
>> > allowed in parent arrays, but instead "orphan" or "dummy" should be
>> > used. Then __clk_init should check for NULL pointers to make this clear.
>>
>> I've added a WARN in __clk_init if any entries in .parent_names are
>> NULL.  I think it best to populate it with "dummy", or maybe a
>> platform-specific name if it helps you during development.
>
> There is no guarantee that the selection of a parent can be mapped
> linear to register values.

Agreed.  I have always viewed .parent_names as only a list of the
names of all possible parents, nothing more.  And of course its array
indexing should line up with struct clk **parents.

> So the right way to deal with it is to have an array of valid names
> with no holes and NULL pointers allowed and have a mapping from the
> array index to the register value.

This is essentially what the .set_rate callback does.  It takes as
input "u8 index" and peforms the hardware specific magic to select the
correct parent clock.  This might be a register write using that exact
same index, or it might be a single-bit register write using that
index as the shift value, or it might translate that index into the
data sent to an i2c device (where the address would be stored in
struct clk_foo), etc etc.

We both agree that .parent_names must contain valid names and should
not have holes.  What I don't understand is if you are saying that we
should allow NULL ptrs as names; that seems contradictory but I want
to make sure I'm reading you correctly.

Thanks,
Mike

> That makes the core code robust and allows to handle all corner cases
> including reserved bits, not implemented clocks and weird register
> layouts.
>
> Thanks,
>
>        tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists