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?
> > - 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.
>
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.
-Courtney
--
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
