Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-14 Thread Olof Johansson
On Fri, Jun 14, 2013 at 02:04:00PM +0100, Pawel Moll wrote: > On Thu, 2013-06-13 at 23:52 +0100, Olof Johansson wrote: > > > + reg = <0 0x7FFF 0 0x1000>; > > > > #size-cells 2 on the parent bus? That's somewhat unusual. > > LPAE == 40 bit physical addresses == potential > 32 bit sizes (m

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-14 Thread Pawel Moll
On Thu, 2013-06-13 at 23:52 +0100, Olof Johansson wrote: > > + reg = <0 0x7FFF 0 0x1000>; > > #size-cells 2 on the parent bus? That's somewhat unusual. LPAE == 40 bit physical addresses == potential > 32 bit sizes (memory blocks > 4GB) Paweł ___

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-14 Thread Lorenzo Pieralisi
Hi Olof, thank you very much for having a look. On Thu, Jun 13, 2013 at 11:52:33PM +0100, Olof Johansson wrote: > Hi, > > Overall this driver looks like it just needs more cooking > time. It's... gritty. Complicated when it should be simple and > layered. Naming is nonobvious, and overall it's

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-13 Thread Nicolas Pitre
On Thu, 13 Jun 2013, Olof Johansson wrote: > > + u32 status = readl_relaxed(info->baseaddr + PWC_STATUS); > > Why readl_relaxed() here? Can't you use a normal readl()? Unfortunately, on ARM readl_relaxed() _is_ the normal readl() because the actual readl() may have side effects. See commit 7

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-13 Thread Olof Johansson
Hi, Overall this driver looks like it just needs more cooking time. It's... gritty. Complicated when it should be simple and layered. Naming is nonobvious, and overall it's hard to glance at a function and say "ah, it does ". I think some of this might be because of naming conventions. Lots of l

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-13 Thread Lorenzo Pieralisi
Hi Samuel, first things first, thanks a lot for having a look. On Thu, Jun 13, 2013 at 01:01:43AM +0100, Samuel Ortiz wrote: > Hi Lorenzo, > > I don't particularily like this code, but I guess most of my dislike > comes from the whole bridge interface API and how that forces you into > implement

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-12 Thread Nicolas Pitre
On Thu, 13 Jun 2013, Samuel Ortiz wrote: > > +static struct vexpress_spc_drvdata *info; > > +static u32 *vexpress_spc_config_data; > > +static struct vexpress_config_bridge *vexpress_spc_config_bridge; > > +static struct vexpress_config_func *opp_func, *perf_func; > > + > > +static int vexpress_sp

Re: [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-12 Thread Samuel Ortiz
Hi Lorenzo, I don't particularily like this code, but I guess most of my dislike comes from the whole bridge interface API and how that forces you into implementing pretty much static code. A few nitpicks: On Thu, Jun 06, 2013 at 10:59:23AM +0100, Lorenzo Pieralisi wrote: > diff --git a/drivers/m

[RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support

2013-06-06 Thread Lorenzo Pieralisi
The TC2 versatile express core tile integrates a logic block that provides the interface between the dual cluster test-chip and the M3 microcontroller that carries out power management. The logic block, called Serial Power Controller (SPC), contains several memory mapped registers to control among