On Wed, Jun 25, 2014 at 1:04 PM, Courtney Cavin
<courtney.cavin@xxxxxxxxxxxxxx> wrote:
> On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
>> 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.
>>
>
> So the price we must pay for *your* vision of architectural correctness
> is to duplicate code without any actual benefit?
>
> According to Documentation/driver-model/platform.txt your view is
> correct:
>
> Platform devices are devices that typically appear as autonomous
> entities in the system. This includes legacy port-based devices and
> host bridges to peripheral buses, and most controllers integrated
> into system-on-chip platforms. What they usually have in common
> is direct addressing from a CPU bus. Rarely, a platform_device will
> be connected through a segment of some other kind of bus; but its
> registers will still be directly addressable.
>
> However, it is clear that with recent changes to the platform code, this
> view is getting distorted somewhere. I haven't specifically heard any
> decisions on what to do here though.
>
> Greg, Grant, Rob? What's the law?
Generally sub-blocks of a device are handled as platform devices. If
there is a good enough reason then creating a new device type may be
okay, but we certainly wouldn't want every PMIC or MFD driver to go
off and define their own bus. Probably not each vendor doing a bus
either.
>> > - 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
I agree that I don't see anything unique here.
>> The QPNP schema is a private case of Qualcomm PMIC's, so generalising
>> will over-engineering IMO.
But it is already generalized for you with platform devices.
> I made a similar attempt to write something like this, but did it
> entirely based on regmap "partitions", abstracting out the need for
> knowing anything in the client drivers but regmap.... It was about the
> same amount of code/complexity as this, but I tossed it out for the same
> reasons I mentioned here. Over/under-engineering has little to do with
> it.
>
> Using 'QPNP' as an excuse to have a bus in the kernel that Qualcomm can
> define may not be your goal, but it does appear that way. From what I
> know of QPNP (which is only what Josh has described on the list) the
> entirety of what matters regarding the QPNP spec is already implemented
> in drivers/spmi/spmi-pmic-arb.c
>
>> > - 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.
>
> Because we would then have our own playground on which to play?
>
> On a related note, it would probably be a good idea to move much of the
> platform resource stuff out of the platform code... so we don't
> re-implement it over-and-over again.
Which part is implemented over-and-over?
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
