Re: [RFC PATCH 0/6] Support for QPNP PMIC's

[Stanimir Varbanov <svarbanov@mm-sol.com>]

Hi Courtney,

Thanks for the comments.

On 06/25/2014 01:36 AM, Courtney Cavin wrote:
> On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
>> Hello to all,
>>
>> The goal of this WIP set is to provide support for sub-functional
>> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
>> bus (device-driver modeled). On SPMI interface we attach a so called
>> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
>> every slaveid the PMIC chip has various peripherials. Every peripheral
>> driver should expose qpnp_driver structure and implement the .probe
>> and .remove callbacks. Every peripheral driver will reside on the
>> proper place in /drivers directory. 
>>
>> The fisrt patch describes the devicetree binding of the slave devices
>> attached to the SPMI interface. The second patch adds implemetation of
>> the qpnp-bus and a layer connection with the SPMI interface. The third
>> patch adds spmi arbiter and underlying slaveid devicetree nodes for
>> msm8974 SoC. The other patches are example of rtc-qpnp driver and
>> binding documentation.
>>
>> Since this set is a WIP, it will be used as a starting point for future
>> duscussions about implementing QPNP PMIC sub-function hw blocks.
> 
> Ok.  I have a few critical problems with this approach to implementing
> support for the PMICs:
>   - It's *way* more code than needed if done with of_platform_populate()

Agreed, but this will be the price we must pay to keep architectural
correctness. The PMIC peripherals are not directly accessible by the CPU
bus and thus they shouldn't be treated as platform devices. That fact
gives as an opportunity to create a qpnp bus on which we will attach the
whole bunch of PMIC peripherals and using DT we can describe register
regions (256B each) belonging to it.

>   - AFAICT, the only direct relation between this and 'QPNP' is the 256
>     byte block size, which I would argue can be clearly expressed in DT
>     instead with #size-cells == 1, and #address-cells == 1

The QPNP schema is a private case of Qualcomm PMIC's, so generalising
will over-engineering IMO.

>   - The resource code from drivers/{base,of}/platform.c is reimplemented
>     here, without any added benefit

The resource code could be simplified to satisfy our needs.

-- 
regards,
Stan