Message-ID: <20120305092948.GV3852@pengutronix.de>
Date:	Mon, 5 Mar 2012 10:29:48 +0100
From:	Sascha Hauer <s.hauer@...gutronix.de>
To:	Andrew Lunn <andrew@...n.ch>
Cc:	"Turquette, Mike" <mturquette@...com>,
	Russell King <linux@....linux.org.uk>, patches@...aro.org,
	linaro-dev@...ts.linaro.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arnd Bergman <arnd.bergmann@...aro.org>,
	Paul Walmsley <paul@...an.com>,
	Shawn Guo <shawn.guo@...escale.com>,
	Jamie Iles <jamie@...ieiles.com>,
	Richard Zhao <richard.zhao@...aro.org>,
	Saravana Kannan <skannan@...eaurora.org>,
	Magnus Damm <magnus.damm@...il.com>,
	Rob Herring <rob.herring@...xeda.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Amit Kucheria <amit.kucheria@...aro.org>,
	Deepak Saxena <dsaxena@...aro.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: [PATCH v5 4/4] clk: basic clock hardware types

On Mon, Mar 05, 2012 at 09:48:23AM +0100, Andrew Lunn wrote:
> On Sun, Mar 04, 2012 at 04:30:08PM -0800, Turquette, Mike wrote:
> > On Sun, Mar 4, 2012 at 9:42 AM, Andrew Lunn <andrew@...n.ch> wrote:
> > > On Sat, Mar 03, 2012 at 12:29:01AM -0800, Mike Turquette wrote:
> > >> Many platforms support simple gateable clocks, fixed-rate clocks,
> > >> adjustable divider clocks and multi-parent multiplexer clocks.
> > >>
> > >> This patch introduces basic clock types for the above-mentioned hardware
> > >> which share some common characteristics.
> > >
> > > Hi Mike
> > >
> > > These basic clocks don't allow the use of prepare/unprepare, from the
> > > side of the clock provider. I think i need this.
> > 
> > This is an interesting point and might help us nail down exactly what
> > we expect from clk_prepare/clk_unprepare.
> > 
> > >
> > > The Orion Kirkwood SoC has clock gating for most of its on chip
> > > peripherals, which the other older Orion devices don't have. The SATA
> > > and PCIe also allows the PHY to be shut down, again which older Orion
> > > devices don't have. The current code links the clock and the PHY
> > > together, shutting both down are the same time. So i would like to
> > > perform the PHY shutdown in the unprepare() function of the clk
> > > driver.
> > 
> > Do you feel it is The Right Thing to enable/disable the phy from
> > clk_prepare/clk_unprepare?  
> 
> Humm, not sure yet. I don't know all the different possibilities,
> which is why i tried to describe my use case, rather than just assume
> i need prepare/unprepare.
> 
> I also realized i did not explain my use case properly. 
> 
> At boot, uboot is turning on various clocks and SATA/PCIe PHYs etc, in
> order to get the device booted. Linux takes over, and the
> Orion/kirkwood board files, ask the kirkwood or generic Orion code to
> create platform_data structures for the different devices that the
> board uses. The kirkwood code keeps a bitmap of devices for which it
> creates platform data for which there is a gated clock. Then in a
> lateinit call, it turns on clocks which are needed, and also turns off
> clocks which are no longer needed, because the board did not ask for a
> driver binding for that device. If it turns off a SATA or PCIe clock,
> it also turns off the PHY associated with it.
> 
> So we are talking about turning off hardware for which there is no
> driver. This seems to exclude pm_runtime_get(_sync), which is about
> hardware with drivers.
> 
> We touched on this subject a couple of months ago, at least with
> respect to clocks. You said that is what the flag CLK_IGNORE_UNUSED
> will be used for. In a lateinit call, you plan to iterate over all
> clocks and turn off any which don't have CLK_IGNORE_UNUSED and have
> not been enabled. I assume you will call both disable() and
> unprepare(), and if i've can put code into the unprepare to turn the
> PHY off, all is good.
> 
> > > Is allowing to pass prepare/unprepare functions to basic clocks
> > > something you want to support? If i prepare a patch would you consider
> > > it?
> > 
> > My original instinct was "no".  The simple gate clock was always
> > supposed to be "simple" and if you need more than it provides then it
> > might be best for your platform to implement a specific clock type.
> > Especially since the purpose of clk_prepare is still up in the air.
> 
> I think i can wrap your simple gate clock, to make my "complex" gate
> clock. What would help is if you would EXPORT_SYMBOL_GPL
> clk_gate_enable() and clk_gate_disable(), since they do exactly what i
> want. I can then build my own clk_ops structure, with my own
> unprepare() function. I would probably use DEFINE_CLK_GATE as is, and
> then at run time, before calling __clk_init() overwrite the .ops with
> my own version.

Maybe I don't get your point, but clk_unprepare should be used when
you have to sleep to disable your clock. When clk_gate_disable() is
exactly why do you want to use clk_unprepare instead of clk_disable?

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 |
--
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