[PATCH v3 2/7] mfd: omap: control: core system control driver

Hello again,

On Thu, Jun 28, 2012 at 1:51 PM, Valentin, Eduardo
[off-list ref] wrote:

Hello,

On Thu, Jun 28, 2012 at 1:26 PM, Konstantin Baydarov
[off-list ref] wrote:

quoted

On 06/28/2012 02:12 PM, Konstantin Baydarov wrote:

quoted

The interface(design) of omap-control-core.c has already been discussed many times :(
Eduardo, in his patch set, suggested following design:
- omap-control-core.c ioremaps SCM window and provide functions to read/write SCP register for bandgap and usb.

IIRC, this approach didn't satisfy and it was suggested to have private read/write in bandgap and usb.

So, my patch set introduces following design:
- omap-control-core.c don't provide read/write functions for bandgap and usb.
- bandgap and usb use their own private read/write functions
- Each omap-control-core.c, bandgap and usb drivers remap SCM window. It's OK because SCM window is statically mapped to the same virtual address. But the problem is that SMP memory window isn't protected. I'm not sure whether it's possible to protect SCM window using this approach.

I mean:

- Each omap-control-core.c, bandgap and usb drivers remap SCM window. It's OK because SCM window is statically mapped. So each call of ioremap in omap-control-core.c, bandgap and usb drivers returns the same virtual address. But the problem is that SCM memory window isn't protected. I'm not sure whether it's possible to protect SCM window using this approach(when each driver remaps the same IOMEM).

quoted

Another possible design is:
- omap-control-core.c ioremaps and reserves SCM IOMEM window
- omap-control-core.c exports omap_control_get_base(virtual base address is returned) to use in bandgap and usb_phy driver.
- Bandgap and usb phy uses their own private read/write function.
IIUC, this way was suggested by Tony.

Well I understood slightly different :-)

I think the point is not really where to put the access functions, but
to have each driver handling a separate part of the the io window. As
I said before, so that they don't access each other io area.

If you have 1 io window, for the above mentioned constraint, you won't
protect anything. So, in that sense, it doesn't make much difference
if you have access functions in core, or in the children, as they are
all sharing the same io window. Of course, in case we put only 1 io
window, for me it is safer if that window is managed in only one
place, instead of several places.

The question is then, can we split the io area into smaller windows
for each children? Considering the children registers are not
contiguous :-(. In theory we can put several entries in the 'reg' DT
property, but that becomes a bit messy as it will change depending on
OMAP version. Anyways, if we split the scm io window into several io
smaller areas/chunks, then it makes sense to have access functions in
each children.

quoted

quoted

I guess It's better to settle the design(interface) of omap-control-core.c, bandgap and usb phy and then submit the next version of patch set.

Agreed. Here. We need to decide how to have this design and stick to it.

Once the design is agreed, the series can probably be split into
parts, so we can have the scm core and its children worked separately

--

Eduardo Valentin

-- 

Eduardo Valentin