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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html