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