On Thu, Jul 18, 2013 at 09:04:02AM +0200, Gerhard Sittig wrote: > On Mon, Jul 15, 2013 at 21:38 +0200, Sascha Hauer wrote: > > > > On Mon, Jul 15, 2013 at 08:47:34PM +0200, Gerhard Sittig wrote: > > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > > > index 6d55eb2..2c07061 100644 > > > --- a/drivers/clk/clk-divider.c > > > +++ b/drivers/clk/clk-divider.c > > > @@ -104,7 +104,7 @@ static unsigned long clk_divider_recalc_rate(struct > > > clk_hw *hw, > > > struct clk_divider *divider = to_clk_divider(hw); > > > unsigned int div, val; > > > > > > - val = readl(divider->reg) >> divider->shift; > > > + val = clk_readl(divider->reg) >> divider->shift; > > > val &= div_mask(divider); > > > > Would it be an option to use regmap for the generic dividers/muxes > > instead? This should be suitable for ppc and also for people who want to > > use the generic clocks on i2c devices. > > Some other thought crossed my mind regarding access to clock > control registers that reside behind some communication channel > like I2C: > > The common clock API assumes (it's part of the contract) that > there are potentially expensive operations like get, put, prepare > and unprepare, as well as swift and non-blocking operations like > enable and disable. > > Would the regmap abstraction hide the potentially blocking nature > of a register access (I understand that you can implement "local" > as well as "remote" register sets by this mechanism)? Or could > you still meet the assumptions or requirements of the common > clock API? > > It might as well be the responsibility of the clock driver's > implementor to arrange for the availability of non-blocking > enable/disable operations, just as it is today. Such that > expensive register access need not be forbidden in general.
regmap for mmio uses a spinlock for read/modify/write operations, just like you have to use a spinlock in the common clk dividers/gates. This part wouldn't change with regmap. For i2c connected clocks where a spinlock doesn't work due to the nonatomic nature of i2c devices we would have to move the enable/disble stuff to prepare/unprepare in the common gate code. This can be left for someone who works on i2c clocks though. I think regmap has the potential to solve a number of issues like the hardcoded readl/writel in the common clock blocks, issues with i2c clocks and your endianess issue. The biggest question probably is how to get there without putting too much of a burden on you. It's probably not an option to convert all users to regmap, so it seems additional functions like clk_register_gate_regmap are better to handle. 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 | _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
