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

From: Ivan T. Ivanov
Date: Fri May 09 2014 - 08:46:26 EST



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
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/