Re: [RFC 3/8] mfd:syscon: Introduce claim/read/write/release APIs

[Srinivas KANDAGATLA <srinivas.kandagatla@st.com>]

Hi Arnd,
Thankyou for the comments.

On 17/05/13 15:36, Arnd Bergmann wrote:
> On Thursday 09 May 2013, Srinivas KANDAGATLA wrote:
>> On 08/05/13 20:48, Arnd Bergmann wrote:
>> I agree, my initial approach was having a dedicated driver specific to
>> ST syscon, however syscon seems to do things very much similar to what
>> we want, so I have integrated those 3 functions in syscon.
>> Am happy to go back with my first approach of adding ST specific syscon
>> driver if no one is actually going to benefit with such a change to
>> syscon driver.
> 
> That would at least be less controversial.

yes I agree.
> 
>> Total layout of the sysconf changes per SOC, and the bit arrangements
>> aswell, however the core IP(pinctrl, etherenet ...) and logic to drive
>> those bits remains exactly same.
> 
> It sounds like you really need a driver with high-level interfaces
> for the bits that change by each core and are needed by otherwise
> identical drivers, like the Ethernet driver you mention.
> 
> I would not go as far as you did describing the individual bits in
> the device node using these however. That driver can be layered on
> top of the existing syscon driver, but hardcode the bits for each
> SoC it knows of.
Some of the drivers like Ethernet already provide higher level
interfaces via callbacks. We did implement such a callbacks per each SOC
in non-DT case, and ended up having code duplicated for each SOC.

On the other hand using device trees to describe the HW
configuration(sysconfs) made more sense and got rid of SOC specific
callbacks.

> 
> For drivers that are essentially just wrappers around sysconf,
> I would make them one driver per SoC and use a low-level interface
> but still hardcode the offsets in the driver instead of using DT
> to find the registers.
> 
> The pinctrl and reset drivers are examples of this.

In pinctrl bindings case, I think we could do better job by replacing
the existing bindings of sysconfs for a group of banks with just two
integer offsets. This would mean that, we can still use the a common
driver across the SOCs.

And w.r.t to reset, we are planning on using sysconf based
reset-controller API sitting underneath the reset-controller subsystem.
Passing the information from device trees would be much clear and
flexible than adding new driver per/SOC.

>> 2> The infrastructure should protect the claimed registers from
>> over-writing by other drivers. We do this by claim-read/write-release
>> style API.
> 
> I don't understand this part. Is it about atomicity of accesses to
> 32-bit registers when you only want to change a bit? That is something
> the regmap interface handles already.

I forget to mention a important point here, the protection is at bit
level for the sysconfs. Some of sysconfs have bits shared across IPs, so
protecting them while the IP is still using it is what we are trying to
achieve with sysconf-claim/release apis.
While it may be overkill, but In past it has found bugs and been helpful
with a meaning full debug output.

>> 3> The driver should be able to set a group of sysconf registers bits to
>> a particular values before initialises the IP. I was thinking of doing
>> this in a same way as pinctrl state.
> 
> That does not fit well with the model we use for other subsystems. If possible,
> try to use the existing abstractions for clock, regulator, pinctrl, reset,
> etc. and call generic interfaces from the driver. When that does not work,
> create a high-level function call from your sysconf driver to do a particular
> thing (e.g. stih_sysconf_ethernet_set_phy_mode()) rather than set up random
> bits from the driver.
I agree going for a higher level api in this case makes sense.

Thanks,
srini

> 
> 	Arnd
> 
>