Re: [PATCH 1/2] mfd: pm8x41: add support for Qualcomm 8x41 PMICs

["Ivan T. Ivanov" <iivanov@mm-sol.com>]

Hi, 

On Thu, 2014-04-24 at 13:18 -0500, Josh Cartwright wrote:
> On Wed, Apr 23, 2014 at 04:36:22PM -0700, Courtney Cavin wrote:
> > On Wed, Apr 23, 2014 at 11:46:26PM +0200, Josh Cartwright wrote:
> > > On Tue, Apr 22, 2014 at 05:31:49PM -0700, Courtney Cavin wrote:
> [..]

<snip>
> 
> 	$ git grep spmi-slave-container arch/arm/boot/dts
> 	arch/arm/boot/dts/qcom/msm-pm8019.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8019.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8110.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8110.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8226.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8226.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8841.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8841.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8916.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8916.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8941.dtsi:	spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pm8941.dtsi:	spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pma8084.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pma8084.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmd9635.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmd9635.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmi8962.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmi8962.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmiplutonium.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmiplutonium.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmplutonium.dtsi:		spmi-slave-container;
> 	arch/arm/boot/dts/qcom/msm-pmplutonium.dtsi:		spmi-slave-container;
> 
> [..]
> > > > +static const struct of_device_id pm8x41_id_table[] = {
> > > > +	{ .compatible = "qcom,pm8841", },
> > > > +	{ .compatible = "qcom,pm8941", },
> > > > +	{},
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, pm8x41_id_table);
> > > 
> > > I'm thinking we should probably have a generic compatible entry as well,
> > > "qcom,pmic-qpnp" or similar.  We should still specify in the binding
> > > that PMIC slaves specify a version-specific string as well as the
> > > generic string.  That is, a slave should have:
> > > 
> > > 	compatible = "qcom,pm8841", "qcom,pmic-qpnp";
> > > 
> > > ...in case we would ever need to differentiate in the future.
> > > 
> > > (I recall that in a previous version I had done this, but I don't
> > > remember why I had changed it..)
> >
> > I gave this some thought but came to the conclusion that there is no
> > benefit of adding a generic compatible to a new binding.  Please clarify
> > a use-case where this would be ... useful.
> 
> Having a generic compatible entry allows for easily supporting new PMICs
> without having to add yet another vacuous entry in the ID table.  In
> this case I think it's perfectly acceptable given that this driver isn't
> really defining a programming model for a specific device, but rather
> acting much more like a bus.
> 
> Requiring a specific PMIC listed before a generic one allows us an
> escape hatch in the future if for some reason we need to add a quirk for
> a specific PMIC.

Is there a conclusion on this issue? I am voting for generic name :-)
"qcom,pm-qpnp".

Further complication is that several sub function drivers expect to
runtime detect the exact version of the controller ("qcom, qpnp-iadc",
"qcom, qpnp-vadc", "qcom, qpnp-linear-charger").  This is realized by the
exported function of the driver "qcom, qpnp-revid". Would it be good
idea to merge qpnp-revid and "qcom,pm-qpnp" driver?

Regards,
Ivan

> 
>   Josh
> 
> [1]: git://codeaurora.org/quic/la/kernel/msm-3.10#msm-3.10
>